24264: restrict effect of 24234 to parameter substitution code and document

7 files changed, 146 insertions, 124 deletions

diff --git a/ChangeLog b/ChangeLog
index 561144fe5..05755a1bd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2007-12-16  Peter Stephenson  <p.w.stephenson@ntlworld.com>
+
+	* 24264: Doc/Zsh/builtins.yo, Doc/Zsh/expn.yo, Src/params.c,
+	Src/subst.c, Src/zsh.h, Test/B02typeset.ztst: restrict the
+	effect of 24234 to expansion in the parameter substitution code
+	and attempt to document the effect.
+
 2007-12-15  Wayne Davison  <wayned@users.sourceforge.net>
 
 	* unposted: Completion/Unix/Command/_rsync: Improved the help
diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 169f4f881..ae828cd40 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -1429,6 +1429,11 @@ tt(exported), tt(integer), tt(readonly)).  If tt(+m) is used with attribute
 flags, and all those flags are introduced with tt(PLUS()), the matching
 parameter names are printed but their values are not.
 
+Attribute flags that transform the final value (tt(-L), tt(-R), tt(-Z),
+tt(-l), tt(u)) are only applied to the expanded value at the point
+of a parameter expansion expression using `tt($)'.  They are not applied
+when a parameter is retrieved internally by the shell for any purpose. 
+
 The following attribute flags may be specified:
 
 startitem()
diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo
index 9fb9f2628..1b43117c2 100644
--- a/Doc/Zsh/expn.yo
+++ b/Doc/Zsh/expn.yo
@@ -785,8 +785,12 @@ with `tt(a)', `tt(i)' or `tt(n)' to reverse the order of sorting.
 )
 item(tt(P))(
 This forces the value of the parameter var(name) to be interpreted as a
-further parameter name, whose value will be used where appropriate. If
-used with a nested parameter or command substitution, the result of that
+further parameter name, whose value will be used where appropriate.
+Note that flags set with one of the tt(typeset) family of commands
+(in particular case transformations) are not applied to the value of
+var(name) used in htis fashion.
+
+If used with a nested parameter or command substitution, the result of that
 will be taken as a parameter name in the same way.  For example, if you
 have `tt(foo=bar)' and `tt(bar=baz)', the strings tt(${(P)foo}),
 tt(${(P)${foo}}), and tt(${(P)$(echo bar)}) will be expanded to `tt(baz)'.
@@ -1062,7 +1066,7 @@ which is expanded by filename expansion to a full path; the outer
 substitution then applies the modifier tt(:h) and takes the directory part
 of the path.)
 )
-time(tt(2.) em(Internal Parameter Flags))(
+item(tt(2.) em(Internal Parameter Flags))(
 Any parameter flags set by one of the tt(typeset) family of commands,
 in particular the tt(L), tt(R), tt(Z), tt(u) and tt(l) flags for padding
 and capitalization, are applied directly to the parameter value.
diff --git a/Src/params.c b/Src/params.c
index 9ebb98dcf..0b372bf25 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -1889,129 +1889,131 @@ getstrvalue(Value v)
 	break;
     }
 
-    if (v->pm->node.flags & (PM_LEFT|PM_RIGHT_B|PM_RIGHT_Z)) {
-	int fwidth = v->pm->width ? v->pm->width : MB_METASTRLEN(s);
-	switch (v->pm->node.flags & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z)) {
-	    char *t, *tend;
-	    unsigned int t0;
-
-	case PM_LEFT:
-	case PM_LEFT | PM_RIGHT_Z:
-	    t = s;
-	    if (v->pm->node.flags & PM_RIGHT_Z)
-		while (*t == '0')
-		    t++;
-	    else
-		while (iblank(*t))
-		    t++;
-	    MB_METACHARINIT();
-	    for (tend = t, t0 = 0; t0 < fwidth && *tend; t0++)
-		tend += MB_METACHARLEN(tend);
-	    /*
-	     * t0 is the number of characters from t used,
-	     * hence (fwidth - t0) is the number of padding
-	     * characters.  fwidth is a misnomer: we use
-	     * character counts, not character widths.
-	     *
-	     * (tend - t) is the number of bytes we need
-	     * to get fwidth characters or the entire string;
-	     * the characters may be multiple bytes.
-	     */
-	    fwidth -= t0; /* padding chars remaining */
-	    t0 = tend - t; /* bytes to copy from string */
-	    s = (char *) hcalloc(t0 + fwidth + 1);
-	    memcpy(s, t, t0);
-	    if (fwidth)
-		memset(s + t0, ' ', fwidth);
-	    s[t0 + fwidth] = '\0';
-	    break;
-	case PM_RIGHT_B:
-	case PM_RIGHT_Z:
-	case PM_RIGHT_Z | PM_RIGHT_B:
-	    {
-		int zero = 1;
-		/* Calculate length in possibly multibyte chars */
-		unsigned int charlen = MB_METASTRLEN(s);
-
-		if (charlen < fwidth) {
-		    char *valprefend = s;
-		    int preflen;
-		    if (v->pm->node.flags & PM_RIGHT_Z) {
-			/*
-			 * This is a documented feature: when deciding
-			 * whether to pad with zeroes, ignore
-			 * leading blanks already in the value;
-			 * only look for numbers after that.
-			 * Not sure how useful this really is.
-			 * It's certainly confusing to code around.
-			 */
-			for (t = s; iblank(*t); t++)
-			    ;
-			/*
-			 * Allow padding after initial minus
-			 * for numeric variables.
-			 */
-			if ((v->pm->node.flags &
-			     (PM_INTEGER|PM_EFLOAT|PM_FFLOAT)) &&
-			    *t == '-')
-			    t++;
+    if (v->flags & VALFLAG_SUBST) {
+	if (v->pm->node.flags & (PM_LEFT|PM_RIGHT_B|PM_RIGHT_Z)) {
+	    int fwidth = v->pm->width ? v->pm->width : MB_METASTRLEN(s);
+	    switch (v->pm->node.flags & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z)) {
+		char *t, *tend;
+		unsigned int t0;
+
+	    case PM_LEFT:
+	    case PM_LEFT | PM_RIGHT_Z:
+		t = s;
+		if (v->pm->node.flags & PM_RIGHT_Z)
+		    while (*t == '0')
+			t++;
+		else
+		    while (iblank(*t))
+			t++;
+		MB_METACHARINIT();
+		for (tend = t, t0 = 0; t0 < fwidth && *tend; t0++)
+		    tend += MB_METACHARLEN(tend);
+		/*
+		 * t0 is the number of characters from t used,
+		 * hence (fwidth - t0) is the number of padding
+		 * characters.  fwidth is a misnomer: we use
+		 * character counts, not character widths.
+		 *
+		 * (tend - t) is the number of bytes we need
+		 * to get fwidth characters or the entire string;
+		 * the characters may be multiple bytes.
+		 */
+		fwidth -= t0; /* padding chars remaining */
+		t0 = tend - t; /* bytes to copy from string */
+		s = (char *) hcalloc(t0 + fwidth + 1);
+		memcpy(s, t, t0);
+		if (fwidth)
+		    memset(s + t0, ' ', fwidth);
+		s[t0 + fwidth] = '\0';
+		break;
+	    case PM_RIGHT_B:
+	    case PM_RIGHT_Z:
+	    case PM_RIGHT_Z | PM_RIGHT_B:
+		{
+		    int zero = 1;
+		    /* Calculate length in possibly multibyte chars */
+		    unsigned int charlen = MB_METASTRLEN(s);
+
+		    if (charlen < fwidth) {
+			char *valprefend = s;
+			int preflen;
+			if (v->pm->node.flags & PM_RIGHT_Z) {
+			    /*
+			     * This is a documented feature: when deciding
+			     * whether to pad with zeroes, ignore
+			     * leading blanks already in the value;
+			     * only look for numbers after that.
+			     * Not sure how useful this really is.
+			     * It's certainly confusing to code around.
+			     */
+			    for (t = s; iblank(*t); t++)
+				;
+			    /*
+			     * Allow padding after initial minus
+			     * for numeric variables.
+			     */
+			    if ((v->pm->node.flags &
+				 (PM_INTEGER|PM_EFLOAT|PM_FFLOAT)) &&
+				*t == '-')
+				t++;
+			    /*
+			     * Allow padding after initial 0x or
+			     * base# for integer variables.
+			     */
+			    if (v->pm->node.flags & PM_INTEGER) {
+				if (isset(CBASES) &&
+				    t[0] == '0' && t[1] == 'x')
+				    t += 2;
+				else if ((valprefend = strchr(t, '#')))
+				    t = valprefend + 1;
+			    }
+			    valprefend = t;
+			    if (!*t)
+				zero = 0;
+			    else if (v->pm->node.flags &
+				     (PM_INTEGER|PM_EFLOAT|PM_FFLOAT)) {
+				/* zero always OK */
+			    } else if (!idigit(*t))
+				zero = 0;
+			}
+			/* number of characters needed for padding */
+			fwidth -= charlen;
+			/* bytes from original string */
+			t0 = strlen(s);
+			t = (char *) hcalloc(fwidth + t0 + 1);
+			/* prefix guaranteed to be single byte chars */
+			preflen = valprefend - s;
+			memset(t + preflen, 
+			       (((v->pm->node.flags & PM_RIGHT_B)
+				 || !zero) ?       ' ' : '0'), fwidth);
 			/*
-			 * Allow padding after initial 0x or
-			 * base# for integer variables.
+			 * Copy - or 0x or base# before any padding
+			 * zeroes.
 			 */
-			if (v->pm->node.flags & PM_INTEGER) {
-			    if (isset(CBASES) &&
-				t[0] == '0' && t[1] == 'x')
-				t += 2;
-			    else if ((valprefend = strchr(t, '#')))
-				t = valprefend + 1;
-			}
-			valprefend = t;
-			if (!*t)
-			    zero = 0;
-			else if (v->pm->node.flags &
-				 (PM_INTEGER|PM_EFLOAT|PM_FFLOAT)) {
-			    /* zero always OK */
-			} else if (!idigit(*t))
-			    zero = 0;
+			if (preflen)
+			    memcpy(t, s, preflen);
+			memcpy(t + preflen + fwidth,
+			       valprefend, t0 - preflen);
+			t[fwidth + t0] = '\0';
+			s = t;
+		    } else {
+			/* Need to skip (charlen - fwidth) chars */
+			for (t0 = charlen - fwidth; t0; t0--)
+			    s += MB_METACHARLEN(s);
 		    }
-		    /* number of characters needed for padding */
-		    fwidth -= charlen;
-		    /* bytes from original string */
-		    t0 = strlen(s);
-		    t = (char *) hcalloc(fwidth + t0 + 1);
-		    /* prefix guaranteed to be single byte chars */
-		    preflen = valprefend - s;
-		    memset(t + preflen, 
-			   (((v->pm->node.flags & PM_RIGHT_B)
-			     || !zero) ?       ' ' : '0'), fwidth);
-		    /*
-		     * Copy - or 0x or base# before any padding
-		     * zeroes.
-		     */
-		    if (preflen)
-			memcpy(t, s, preflen);
-		    memcpy(t + preflen + fwidth,
-			   valprefend, t0 - preflen);
-		    t[fwidth + t0] = '\0';
-		    s = t;
-		} else {
-		    /* Need to skip (charlen - fwidth) chars */
-		    for (t0 = charlen - fwidth; t0; t0--)
-			s += MB_METACHARLEN(s);
 		}
+		break;
 	    }
+	}
+	switch (v->pm->node.flags & (PM_LOWER | PM_UPPER)) {
+	case PM_LOWER:
+	    s = casemodify(s, CASMOD_LOWER);
+	    break;
+	case PM_UPPER:
+	    s = casemodify(s, CASMOD_UPPER);
 	    break;
 	}
     }
-    switch (v->pm->node.flags & (PM_LOWER | PM_UPPER)) {
-    case PM_LOWER:
-	s = casemodify(s, CASMOD_LOWER);
-	break;
-    case PM_UPPER:
-	s = casemodify(s, CASMOD_UPPER);
-	break;
-    }
     if (v->start == 0 && v->end == -1)
 	return s;
 
diff --git a/Src/subst.c b/Src/subst.c
index e8257163b..22b4cfe08 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -2059,8 +2059,11 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int ssub)
 		 * There really is a value.  Padding and case
 		 * transformations used to be handled here, but
 		 * are now handled in getstrvalue() for greater
-		 * consistency.
+		 * consistency.  However, we get unexpected effects
+		 * if we allow them to applied on every call, so
+		 * set the flag that allows them to be substituted.
 		 */
+		v->flags |= VALFLAG_SUBST;
 		val = getstrvalue(v);
 	    }
 	}
diff --git a/Src/zsh.h b/Src/zsh.h
index 5354e9663..fb4d51ecd 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -599,7 +599,8 @@ struct value {
 
 enum {
     VALFLAG_INV =	0x0001,	/* We are performing inverse subscripting */
-    VALFLAG_EMPTY =	0x0002	/* Subscripted range is empty */
+    VALFLAG_EMPTY =	0x0002,	/* Subscripted range is empty */
+    VALFLAG_SUBST =     0x0004  /* Substitution, so apply padding, case flags */
 };
 
 #define MAX_ARRLEN    262144
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index e9837a1c7..34e2dab26 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -430,17 +430,17 @@
  local case1=upper
  typeset -u case1
  print $case1
- UPPER="VALUE OF \$UPPER"
+ upper="VALUE OF \$UPPER"
  print ${(P)case1}
-0:Upper case conversion
+0:Upper case conversion, does not apply to values used internally
 >UPPER
 >VALUE OF $UPPER
 
  local case2=LOWER
  typeset -l case2
  print $case2
- lower="value of \$lower"
+ LOWER="value of \$lower"
  print ${(P)case2}
-0:Lower case conversion
+0:Lower case conversion, does not apply to values used internally
 >lower
 >value of $lower