PATCH: parameter and quoting (was: Re: Completion problems.)

PATCH: parameter and quoting (was: Re: Completion problems.)

[Sven Wischnowsky]

Bart Schaefer wrote:

> On Aug 4, 11:37am, Sven Wischnowsky wrote:
> } Subject: Re: Completion problems.
> }
> } Tanaka Akira wrote:
> } 
> } > In following examples, unquoted forms are not known until runtime.
> } > 
> } > % if some-complex-command; then var=xxx; else var=yyy; fi; tst $var/<TAB>
> } > % tst $(some-complex-command)/<TAB>
> } 
> } I see two ways to go: 1) completely change the completion code to
> } report strings in unquoted form or 2) add a parameter expansion
> } modifier which does something like the opposite of `:q'.
> 
> I think (2) would be quite useful in other contexts anyway.  Perhaps (Q)?
> (And we could add (q) which means the same as :q, just for completeness.)

The patch below tries to do that in the simplest way I could think of.
This is the solution that makes errors be ignored, because I have to
ask first: we could easily add another flag that turns on error-
checking. OR should error-reporting be the normal case and we should
have a flag to turn it off? For modifiers we could have a new modifier 
that can be combined with `Q' a la `g' and `f', e.g. `$a:EQ' or
something like that. And with respect to the flag: this could be made
to work for things like `${...%...}', too. Btw. something like `${foo%(}'
currently gives me a non-zero return status but no error message --
even though there seems to be some extra code for it -- I haven't
investigated any firther yet, but this seems wrong, doesn't it?

Ok. Which way should we go?

Bye
 Sven

diff -u os/subst.c Src/subst.c
--- os/subst.c	Mon Aug  2 11:44:47 1999
+++ Src/subst.c	Thu Aug  5 14:00:43 1999
@@ -721,6 +721,7 @@
     int flnum = 0;
     int sortit = 0, casind = 0;
     int casmod = 0;
+    int quotemod = 0;
     char *sep = NULL, *spsep = NULL;
     char *premul = NULL, *postmul = NULL, *preone = NULL, *postone = NULL;
     char *replstr = NULL;	/* replacement string for /orig/repl */
@@ -822,6 +823,14 @@
 		case 'i':
 		    casind = 1;
 		    break;
+
+		case 'q':
+		    quotemod = 1;
+		    break;
+		case 'Q':
+		    quotemod = -1;
+		    break;
+
 		case 'e':
 		    eval = 1;
 		    break;
@@ -1546,6 +1555,46 @@
 		makecapitals(&val);
 	}
     }
+    if (quotemod) {
+	if (isarr) {
+	    char **ap;
+
+	    if (!copied)
+		aval = arrdup(aval), copied = 1;
+	    ap = aval;
+
+	    if (quotemod > 0)
+		for (; *ap; ap++)
+		    *ap = bslashquote(*ap, NULL, 0);
+	    else {
+		int one = noerrs, oef = errflag;
+
+		noerrs = 1;
+		for (; *ap; ap++) {
+		    parse_subst_string(*ap);
+		    remnulargs(*ap);
+		    untokenize(*ap);
+		}
+		noerrs = one;
+		errflag = oef;
+	    }
+	} else {
+	    if (!copied)
+		val = dupstring(val), copied = 1;
+	    if (quotemod > 0)
+		val = bslashquote(val, NULL, 0);
+	    else {
+		int one = noerrs, oef = errflag;
+
+		noerrs = 1;
+		parse_subst_string(val);
+		noerrs = one;
+		errflag = oef;
+		remnulargs(val);
+		untokenize(val);
+	    }
+	}
+    }
     if (isarr) {
 	char *x;
 	char *y;
@@ -1747,6 +1796,7 @@
 	    case 'l':
 	    case 'u':
 	    case 'q':
+	    case 'Q':
 		c = **ptr;
 		break;
 
@@ -1868,6 +1918,18 @@
 		    case 'q':
 			copy = bslashquote(copy, NULL, 0);
 			break;
+		    case 'Q':
+			{
+			    int one = noerrs, oef = errflag;
+
+			    noerrs = 1;
+			    parse_subst_string(copy);
+			    noerrs = one;
+			    errflag = oef;
+			    remnulargs(copy);
+			    untokenize(copy);
+			}
+			break;
 		    }
 		    tc = *tt;
 		    *tt = '\0';
@@ -1921,6 +1983,18 @@
 		    break;
 		case 'q':
 		    *str = bslashquote(*str, NULL, 0);
+		    break;
+		case 'Q':
+		    {
+			int one = noerrs, oef = errflag;
+
+			noerrs = 1;
+			parse_subst_string(*str);
+			noerrs = one;
+			errflag = oef;
+			remnulargs(*str);
+			untokenize(*str);
+		    }
 		    break;
 		}
 	    }
diff -u od/Zsh/expn.yo Doc/Zsh/expn.yo
--- od/Zsh/expn.yo	Mon Aug  2 11:45:23 1999
+++ Doc/Zsh/expn.yo	Thu Aug  5 14:12:26 1999
@@ -166,6 +166,9 @@
 case it is only useful if the resulting text is to be re-evaluated
 such as by tt(eval).
 )
+item(tt(Q))(
+Remove one level of quotes from the substituted words.
+)
 item(tt(x))(
 Like tt(q), but break into words at each blank.
 )
@@ -577,6 +580,12 @@
 Capitalize the resulting words.  `Words' in this case refers to sequences
 of alphanumeric characters separated by non-alphanumerics, em(not) to words
 that result from field splitting.
+)
+item(tt(q))(
+Quote the resulting words with backslashes.
+)
+item(tt(Q))(
+Remove one level of quotes from the resulting words.
 )
 item(tt(c))(
 With tt(${#)var(name)tt(}), count the total number of characters in an array,

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: parameter and quoting (was: Re: Completion problems.)

[Peter Stephenson]

Sven Wischnowsky wrote:
> This is the solution that makes errors be ignored, because I have to
> ask first: we could easily add another flag that turns on error-
> checking. OR should error-reporting be the normal case and we should
> have a flag to turn it off? For modifiers we could have a new modifier 
> that can be combined with `Q' a la `g' and `f', e.g. `$a:EQ' or
> something like that. And with respect to the flag: this could be made
> to work for things like `${...%...}', too.

Since this is mainly for parameters, it might be better just to alter the
version with flags.  Probably it shouldn't report an error normally.

> Btw. something like `${foo%(}'
> currently gives me a non-zero return status but no error message --
> even though there seems to be some extra code for it -- I haven't
> investigated any firther yet, but this seems wrong, doesn't it?

The behaviour of BAD_PATTERN is a bit inconsistent:  sometimes the option
is respected, sometimes it isn't, sometimes bad patterns aren't reported at
all.  There's probably quite a lot wrong.

-- 
Peter Stephenson <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Dipartimento di Fisica, Via Buonarroti 2, 56127 Pisa, Italy

Re: PATCH: parameter and quoting (was: Re: Completion problems.)

[Bart Schaefer]

On Aug 5,  2:19pm, Sven Wischnowsky wrote:
} Subject: PATCH: parameter and quoting (was: Re: Completion problems.)
}
} 
} Bart Schaefer wrote:
} 
} > On Aug 4, 11:37am, Sven Wischnowsky wrote:
} > 
} > I think (2) would be quite useful in other contexts anyway.  Perhaps (Q)?
} > (And we could add (q) which means the same as :q, just for completeness.)
} 
} The patch below tries to do that in the simplest way I could think of.

Hrm.  One problem which may not even be worth mentioning is that

	${(Qq)param}	acts like ${(q)param} and
	${(qQ)param}	acts like ${(Q)param}

whereas other "inversions" like using (j///s///) always both happen and
always happen in the same order regardless of the order in which the flags
appear.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: PATCH: parameter and quoting (was: Re: Completion problems.)

[Sven Wischnowsky]

Peter Stephenson wrote:

> Sven Wischnowsky wrote:
> > Btw. something like `${foo%(}'
> > currently gives me a non-zero return status but no error message --
> > even though there seems to be some extra code for it -- I haven't
> > investigated any firther yet, but this seems wrong, doesn't it?
> 
> The behaviour of BAD_PATTERN is a bit inconsistent:  sometimes the option
> is respected, sometimes it isn't, sometimes bad patterns aren't reported at
> all.  There's probably quite a lot wrong.

Ah, right, hadn't thought about patterns (ahem)... "${a%'}" reports
the error.

Hm. Should the proposed new flag apply to those, too, and change the
current behaviour to not report the error or should the current
behaviour of (Q) be changed?
Personally I prefer the first one, but that would require an
incompatible change. (And making the flag turn off errors for
${...%...} and friends and turn it on for (Q) is almost certainly the
wrong way).

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: parameter and quoting (was: Re: Completion problems.)

[Peter Stephenson]

Sven Wischnowsky wrote:
> Ah, right, hadn't thought about patterns (ahem)... "${a%'}" reports
> the error.
> 
> Hm. Should the proposed new flag apply to those, too, and change the
> current behaviour to not report the error or should the current
> behaviour of (Q) be changed?

The first possibility sounds sensible to me, too, since ksh allows
you to do

  a="'"
  echo "${a%'}"

although bash complains as well.  It's probably a grey area, since the
single UNIX specification says

   The double-quoting of patterns is different depending on where the
   double-quotes are placed:
   ${x#*}
          The asterisk is a pattern character.
   ${x#"*"}
          The literal asterisk is quoted and not special.

(in the first case they presumably mean the whole expression is quoted), so
properly placed quotes are supposed to be active and this seems to work
OK, i.e. ${a%"'"}.

-- 
Peter Stephenson <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Dipartimento di Fisica, Via Buonarroti 2, 56127 Pisa, Italy

RE: PATCH: parameter and quoting (was: Re: Completion problems.)

[Andrej Borsenkow]

>
> The first possibility sounds sensible to me, too, since ksh allows
> you to do
>
>   a="'"
>   echo "${a%'}"
>
> although bash complains as well.  It's probably a grey area, since the
> single UNIX specification says
>

Unfortunately, Single UNIX requires, that quotes be matched. This is from the
description of double-quotes:

==
Within the string of characters from an enclosed ${ to the matching "}", an even
number of unescaped double-quotes or single-quotes, if any, must occur.
==

It means, that preceding example MUST be

echo "${a%\'}"

/andrej

Re: PATCH: parameter and quoting (was: Re: Completion problems.)

[Sven Wischnowsky]

Peter Stephenson wrote:

> Sven Wischnowsky wrote:
> > Ah, right, hadn't thought about patterns (ahem)... "${a%'}" reports
> > the error.
> > 
> > Hm. Should the proposed new flag apply to those, too, and change the
> > current behaviour to not report the error or should the current
> > behaviour of (Q) be changed?
> 
> The first possibility sounds sensible to me, too, since ksh allows
> you to do

Fine. This adds the (X) flag to turn on error-reporting. Both `e' and
`E' were already taken, I could have used `w' or `W' for `warning' or 
something like that, but that isn't really correct. Other free
characters are: DGHJKTVWYZabcdghmnuwxyz -- any suggestions, or is `X'
acceptable?

This also makes `:Q' work for history expansions. I always forget that 
history doesn't use modify().


Then Andrej Borsenkow wrote:

> Unfortunately, Single UNIX requires, that quotes be matched. This is from the
> description of double-quotes:
> 
> ==
> Within the string of characters from an enclosed ${ to the matching "}", an even
> number of unescaped double-quotes or single-quotes, if any, must occur.
> ==
> 
> It means, that preceding example MUST be
> 
> echo "${a%\'}"

Damn, I just had the patch... but even if we decide to follow the
standard and not ksh here, it will be easier with this patch.

Bye
 Sven

diff -u os/hist.c Src/hist.c
--- os/hist.c	Thu Aug  5 14:19:19 1999
+++ Src/hist.c	Thu Aug  5 16:16:43 1999
@@ -577,6 +577,18 @@
 	    case 'q':
 		quote(&sline);
 		break;
+	    case 'Q':
+		{
+		    int one = noerrs, oef = errflag;
+
+		    noerrs = 1;
+		    parse_subst_string(sline);
+		    noerrs = one;
+		    errflag = oef;
+		    remnulargs(sline);
+		    untokenize(sline);
+		}
+		break;
 	    case 'x':
 		quotebreak(&sline);
 		break;
diff -u os/subst.c Src/subst.c
--- os/subst.c	Thu Aug  5 14:19:21 1999
+++ Src/subst.c	Thu Aug  5 16:10:54 1999
@@ -721,7 +721,7 @@
     int flnum = 0;
     int sortit = 0, casind = 0;
     int casmod = 0;
-    int quotemod = 0;
+    int quotemod = 0, quoteerr = 0;
     char *sep = NULL, *spsep = NULL;
     char *premul = NULL, *postmul = NULL, *preone = NULL, *postone = NULL;
     char *replstr = NULL;	/* replacement string for /orig/repl */
@@ -830,6 +830,9 @@
 		case 'Q':
 		    quotemod = -1;
 		    break;
+		case 'X':
+		    quoteerr = 1;
+		    break;
 
 		case 'e':
 		    eval = 1;
@@ -1388,12 +1391,23 @@
 	case '#':
 	case Pound:
 	case '/':
-	    if (qt)
-		if (parse_subst_string(s)) {
+	    if (qt) {
+		int one = noerrs, oef = errflag, haserr;
+
+		if (!quoteerr)
+		    noerrs = 1;
+		haserr = parse_subst_string(s);
+		noerrs = one;
+		if (!quoteerr) {
+		    errflag = oef;
+		    if (haserr)
+			tokenize(s);
+		} else if (haserr || errflag) {
 		    zerr("parse error in ${...%c...} substitution",
 			 NULL, s[-1]);
 		    return NULL;
 		}
+	    }
 	    {
 		char t = s[-1];
 
@@ -1567,16 +1581,22 @@
 		for (; *ap; ap++)
 		    *ap = bslashquote(*ap, NULL, 0);
 	    else {
-		int one = noerrs, oef = errflag;
+		int one = noerrs, oef = errflag, haserr = 0;
 
-		noerrs = 1;
+		if (!quoteerr)
+		    noerrs = 1;
 		for (; *ap; ap++) {
-		    parse_subst_string(*ap);
+		    haserr |= parse_subst_string(*ap);
 		    remnulargs(*ap);
 		    untokenize(*ap);
 		}
 		noerrs = one;
-		errflag = oef;
+		if (!quoteerr)
+		    errflag = oef;
+		else if (haserr || errflag) {
+		    zerr("parse error in parameter value", NULL, 0);
+		    return NULL;
+		}
 	    }
 	} else {
 	    if (!copied)
@@ -1584,12 +1604,18 @@
 	    if (quotemod > 0)
 		val = bslashquote(val, NULL, 0);
 	    else {
-		int one = noerrs, oef = errflag;
+		int one = noerrs, oef = errflag, haserr;
 
-		noerrs = 1;
-		parse_subst_string(val);
+		if (!quoteerr)
+		    noerrs = 1;
+		haserr = parse_subst_string(val);
 		noerrs = one;
-		errflag = oef;
+		if (!quoteerr)
+		    errflag = oef;
+		else if (haserr || errflag) {
+		    zerr("parse error in parameter value", NULL, 0);
+		    return NULL;
+		}
 		remnulargs(val);
 		untokenize(val);
 	    }
diff -u od/Zsh/expn.yo Doc/Zsh/expn.yo
--- od/Zsh/expn.yo	Thu Aug  5 14:19:13 1999
+++ Doc/Zsh/expn.yo	Thu Aug  5 16:17:48 1999
@@ -587,6 +587,11 @@
 item(tt(Q))(
 Remove one level of quotes from the resulting words.
 )
+item(tt(X))(
+With this flag parsing errors occuring with the tt(Q) flag or the
+pattern matching forms such as `tt(${)var(name)tt(#)var(pattern)tt(})' 
+are reported. Without the flag they are silently ignored.
+)
 item(tt(c))(
 With tt(${#)var(name)tt(}), count the total number of characters in an array,
 as if the elements were concatenated with spaces between them.

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

RE: PATCH: parameter and quoting (was: Re: Completion problems.)

[Andrej Borsenkow]

> Then Andrej Borsenkow wrote:
>
> > Unfortunately, Single UNIX requires, that quotes be matched. This
> is from the
> > description of double-quotes:
> >
> > ==
> > Within the string of characters from an enclosed ${ to the matching
> "}", an even
> > number of unescaped double-quotes or single-quotes, if any, must occur.
> > ==
> >
> > It means, that preceding example MUST be
> >
> > echo "${a%\'}"
>
> Damn, I just had the patch... but even if we decide to follow the
> standard and not ksh here, it will be easier with this patch.
>

Ehem ... reading it a bit more closely, it is still ambiguous. Single UNIX
explicitly diallows "${foo%'}" - but it does not tell us, how to actually escape
single quote in this case. Another line in the same paragraph says:

==
The backslash retains its special meaning as an escape character (see Escape
Character (Backslash) ) only when followed by one of the characters:
$   `   "   \   <newline>
==

OTOH it contains valid example:

unset bar
foo="${bar-\}}"

This implies, that in "${...}" backslash quotes } and (probably) ' ... horrors.

Anybody has better description?

/andrej

Re: PATCH: parameter and quoting (was: Re: Completion problems.)

[Bart Schaefer]

On Aug 5,  4:26pm, Sven Wischnowsky wrote:
} Subject: Re: PATCH: parameter and quoting (was: Re: Completion problems.)
}
} Fine. This adds the (X) flag to turn on error-reporting. Both `e' and
} `E' were already taken, I could have used `w' or `W' for `warning' or 
} something like that, but that isn't really correct. Other free
} characters are: DGHJKTVWYZabcdghmnuwxyz -- any suggestions, or is `X'
} acceptable?

I think X is OK.  I keep hoping for csh's :x, though ...

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: PATCH: parameter and quoting (was: Re: Completion problems.)

[Sven Wischnowsky]

Bart Schaefer wrote:

> Hrm.  One problem which may not even be worth mentioning is that
> 
> 	${(Qq)param}	acts like ${(q)param} and
> 	${(qQ)param}	acts like ${(Q)param}
> 
> whereas other "inversions" like using (j///s///) always both happen and
> always happen in the same order regardless of the order in which the flags
> appear.

This makes the result look as if both flags were used, `q' first,
which means that `${(qQ)foo}' is the same as ${foo}.

Bye
 Sven

--- os/subst.c	Thu Aug  5 16:27:16 1999
+++ Src/subst.c	Fri Aug  6 09:11:21 1999
@@ -825,10 +825,10 @@
 		    break;
 
 		case 'q':
-		    quotemod = 1;
+		    quotemod++;
 		    break;
 		case 'Q':
-		    quotemod = -1;
+		    quotemod--;
 		    break;
 		case 'X':
 		    quoteerr = 1;

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de