* parsing empty alternatives: case foo|) :;;
@ 2017-08-07 13:55 ` Daniel Shahaf
2017-08-07 14:26 ` Peter Stephenson
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Shahaf @ 2017-08-07 13:55 UTC (permalink / raw)
To: zsh-workers
% case '' in (foo|) echo yes;; esac
yes
% case '' in foo|) echo yes;; esac
zsh: parse error near `)'
Why does the second case fail to parse? We (IRC) don't see any ambiguity
in it.
(not a release blocker)
Cheers,
Daniel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: parsing empty alternatives: case foo|) :;;
2017-08-07 13:55 ` parsing empty alternatives: case foo|) :;; Daniel Shahaf
@ 2017-08-07 14:26 ` Peter Stephenson
2017-08-07 14:44 ` Bart Schaefer
0 siblings, 1 reply; 11+ messages in thread
From: Peter Stephenson @ 2017-08-07 14:26 UTC (permalink / raw)
To: Daniel Shahaf; +Cc: zsh-workers
On Mon, 7 Aug 2017 13:55:59 +0000
Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> % case '' in (foo|) echo yes;; esac
> yes
> % case '' in foo|) echo yes;; esac
> zsh: parse error near `)'
>
> Why does the second case fail to parse? We (IRC) don't see any ambiguity
> in it.
I haven't checked, but I think it looks like a pipe --- at that point we
haven't gone further down the case-specific code so that it parses the
line specially.
pws
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: parsing empty alternatives: case foo|) :;;
2017-08-07 14:26 ` Peter Stephenson
@ 2017-08-07 14:44 ` Bart Schaefer
2017-08-07 14:58 ` Peter Stephenson
0 siblings, 1 reply; 11+ messages in thread
From: Bart Schaefer @ 2017-08-07 14:44 UTC (permalink / raw)
To: Peter Stephenson; +Cc: Daniel Shahaf, zsh-workers
On Mon, Aug 7, 2017 at 7:26 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Mon, 7 Aug 2017 13:55:59 +0000
> Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>> % case '' in (foo|) echo yes;; esac
>> yes
>> % case '' in foo|) echo yes;; esac
>> zsh: parse error near `)'
>>
>> Why does the second case fail to parse? We (IRC) don't see any ambiguity
>> in it.
>
> I haven't checked, but I think it looks like a pipe --- at that point we
> haven't gone further down the case-specific code so that it parses the
> line specially.
Several things are going on here.
(1) The "case" syntax with no open paren and only a close paren is
from older Bourne shell
(2) The use of "|" to separate alternatives in a pattern is
zsh-specific globbing syntax, which is only active when parsing
patterns that contain a parenthesized grouping
(3) Because of both of the foregoing, when parsing the pattern in the
older syntax, recognition of "|' as a separator is not enabled -- it's
read as a simple pattern with no grouping
(4) When the newer "case" syntax with a fully parenthesized pattern
was added, the presence of the open paren gave us the parsing hook
needed to activate the grouping syntax
To put it another way, "in foo|)" is rejected for the same reason that
"in |foo)" is.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: parsing empty alternatives: case foo|) :;;
2017-08-07 14:44 ` Bart Schaefer
@ 2017-08-07 14:58 ` Peter Stephenson
2017-08-07 18:03 ` 5.4 almost released Peter Stephenson
2017-08-07 18:21 ` parsing empty alternatives: case foo|) :;; Peter Stephenson
0 siblings, 2 replies; 11+ messages in thread
From: Peter Stephenson @ 2017-08-07 14:58 UTC (permalink / raw)
To: zsh-workers
A bit more experimentation reveals it's just the empty string that's
confusing it. If you put something between the | and the ) it works.
case foo in
foo|'') print Yes
;;
esac
works, also replacing '' by $empty works, but removing it completely doesn't.
I think this might be some fix-up code that got added for the old case
(nothing to do with pipes in that case).
Yes
pws
^ permalink raw reply [flat|nested] 11+ messages in thread
* 5.4 almost released
2017-08-07 14:58 ` Peter Stephenson
@ 2017-08-07 18:03 ` Peter Stephenson
2017-08-07 18:25 ` Eric Cook
2017-08-07 18:21 ` parsing empty alternatives: case foo|) :;; Peter Stephenson
1 sibling, 1 reply; 11+ messages in thread
From: Peter Stephenson @ 2017-08-07 18:03 UTC (permalink / raw)
To: zsh-workers
THe files for version 5.4 are now in www.zsh.org/dev. If a few people
get a chance to check this is OK I'll announce it and tidy up.
Thanks
pws
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: parsing empty alternatives: case foo|) :;;
2017-08-07 14:58 ` Peter Stephenson
2017-08-07 18:03 ` 5.4 almost released Peter Stephenson
@ 2017-08-07 18:21 ` Peter Stephenson
2017-08-07 18:56 ` Peter Stephenson
1 sibling, 1 reply; 11+ messages in thread
From: Peter Stephenson @ 2017-08-07 18:21 UTC (permalink / raw)
To: zsh-workers
On Mon, 07 Aug 2017 15:58:56 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> I think this might be some fix-up code that got added for the old case
> (nothing to do with pipes in that case).
It's complicated because of the two cases, but I'm not sure this ever
worked.
It's straightforward to fix, but note I can't fix "foo||bar)" without
more work because || is a different token. I think we can probably
avoid parsing it that way here if we want to go down that route.
I deliberately haven't tried to squeeze this into 5.4.
pws
diff --git a/Src/parse.c b/Src/parse.c
index ba9cd61..d99826d 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -1194,6 +1194,7 @@ par_case(int *cmplx)
for (;;) {
char *str;
+ int skip_zshlex;
while (tok == SEPER)
zshlex();
@@ -1201,11 +1202,17 @@ par_case(int *cmplx)
break;
if (tok == INPAR)
zshlex();
- if (tok != STRING)
- YYERRORV(oecused);
- if (!strcmp(tokstr, "esac"))
- break;
- str = dupstring(tokstr);
+ if (tok == BAR) {
+ str = dupstring("");
+ skip_zshlex = 1;
+ } else {
+ if (tok != STRING)
+ YYERRORV(oecused);
+ if (!strcmp(tokstr, "esac"))
+ break;
+ str = dupstring(tokstr);
+ skip_zshlex = 0;
+ }
type = WC_CASE_OR;
pp = ecadd(0);
palts = ecadd(0);
@@ -1245,8 +1252,9 @@ par_case(int *cmplx)
*/
incasepat = 0;
incmdpos = 1;
- for (;;) {
+ if (!skip_zshlex)
zshlex();
+ for (;;) {
if (tok == OUTPAR) {
ecstr(str);
ecadd(ecnpats++);
@@ -1302,9 +1310,24 @@ par_case(int *cmplx)
}
zshlex();
- if (tok != STRING)
+ switch (tok) {
+ case STRING:
+ /* Normal case */
+ str = dupstring(tokstr);
+ zshlex();
+ break;
+
+ case OUTPAR:
+ case BAR:
+ /* Empty string */
+ str = dupstring("");
+ break;
+
+ default:
+ /* Oops. */
YYERRORV(oecused);
- str = dupstring(tokstr);
+ break;
+ }
}
par_save_list(cmplx);
if (tok == SEMIAMP)
diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
index 9625a15..db62875 100644
--- a/Test/A01grammar.ztst
+++ b/Test/A01grammar.ztst
@@ -820,6 +820,26 @@
0:case keeps exit status of last command executed in compound-list
>37
+ case '' in
+ burble) print No.
+ ;;
+ spurble|) print Yes!
+ ;;
+ |burble) print Not quite.
+ ;;
+ esac
+ case '' in
+ burble) print No.
+ ;;
+ |burble) print Wow!
+ ;;
+ spurble|) print Sorry.
+ ;;
+ esac
+0: case with no opening parentheses and empty string
+>Yes!
+>Wow!
+
x=1
x=2 | echo $x
echo $x
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 5.4 almost released
2017-08-07 18:03 ` 5.4 almost released Peter Stephenson
@ 2017-08-07 18:25 ` Eric Cook
2017-08-07 18:34 ` Peter Stephenson
0 siblings, 1 reply; 11+ messages in thread
From: Eric Cook @ 2017-08-07 18:25 UTC (permalink / raw)
To: zsh-workers
On 08/07/2017 02:03 PM, Peter Stephenson wrote:
> THe files for version 5.4 are now in www.zsh.org/dev. If a few people
> get a chance to check this is OK I'll announce it and tidy up.
>
> Thanks
> pws
>
pardon the typo, 5.4 is on http://www.zsh.org/pub
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 5.4 almost released
2017-08-07 18:25 ` Eric Cook
@ 2017-08-07 18:34 ` Peter Stephenson
2017-08-08 10:26 ` Martijn Dekker
0 siblings, 1 reply; 11+ messages in thread
From: Peter Stephenson @ 2017-08-07 18:34 UTC (permalink / raw)
To: zsh-workers
On Mon, 7 Aug 2017 14:25:35 -0400
Eric Cook <llua@gmx.com> wrote:
> On 08/07/2017 02:03 PM, Peter Stephenson wrote:
> > THe files for version 5.4 are now in www.zsh.org/dev. If a few people
> > get a chance to check this is OK I'll announce it and tidy up.
> >
> > Thanks
> > pws
> >
>
> pardon the typo, 5.4 is on http://www.zsh.org/pub
Er, yes, thank you.
pws
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: parsing empty alternatives: case foo|) :;;
2017-08-07 18:21 ` parsing empty alternatives: case foo|) :;; Peter Stephenson
@ 2017-08-07 18:56 ` Peter Stephenson
0 siblings, 0 replies; 11+ messages in thread
From: Peter Stephenson @ 2017-08-07 18:56 UTC (permalink / raw)
To: zsh-workers
On Mon, 7 Aug 2017 19:21:03 +0100
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> It's straightforward to fix, but note I can't fix "foo||bar)" without
> more work because || is a different token. I think we can probably
> avoid parsing it that way here if we want to go down that route.
Looks like this isn't too difficult, either.
pws
diff --git a/Src/lex.c b/Src/lex.c
index b2d9b3f..8493d47 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -760,7 +760,7 @@ gettok(void)
return AMPER;
case LX1_BAR:
d = hgetc();
- if (d == '|')
+ if (d == '|' && !incasepat)
return DBAR;
else if (d == '&')
return BARAMP;
@@ -1058,7 +1058,7 @@ gettokstr(int c, int sub)
if (isset(SHGLOB)) {
if (sub || in_brace_param)
break;
- if (incasepat && !lexbuf.len)
+ if (incasepat > 0 && !lexbuf.len)
return INPAR;
if (!isset(KSHGLOB) && lexbuf.len)
goto brk;
@@ -1859,7 +1859,7 @@ exalias(void)
Reswd rw;
hwend();
- if (interact && isset(SHINSTDIN) && !strin && !incasepat &&
+ if (interact && isset(SHINSTDIN) && !strin && incasepat <= 0 &&
tok == STRING && !nocorrect && !(inbufflags & INP_ALIAS) &&
(isset(CORRECTALL) || (isset(CORRECT) && incmdpos)))
spckword(&tokstr, 1, incmdpos, 1);
diff --git a/Src/parse.c b/Src/parse.c
index ba9cd61..2705252 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -48,7 +48,11 @@ mod_export int incond;
/**/
mod_export int inredir;
-/* != 0 if we are about to read a case pattern */
+/*
+ * 1 if we are about to read a case pattern
+ * -1 if we are not quite sure
+ * 0 otherwise
+ */
/**/
int incasepat;
@@ -1194,6 +1198,7 @@ par_case(int *cmplx)
for (;;) {
char *str;
+ int skip_zshlex;
while (tok == SEPER)
zshlex();
@@ -1201,11 +1206,17 @@ par_case(int *cmplx)
break;
if (tok == INPAR)
zshlex();
- if (tok != STRING)
- YYERRORV(oecused);
- if (!strcmp(tokstr, "esac"))
- break;
- str = dupstring(tokstr);
+ if (tok == BAR) {
+ str = dupstring("");
+ skip_zshlex = 1;
+ } else {
+ if (tok != STRING)
+ YYERRORV(oecused);
+ if (!strcmp(tokstr, "esac"))
+ break;
+ str = dupstring(tokstr);
+ skip_zshlex = 0;
+ }
type = WC_CASE_OR;
pp = ecadd(0);
palts = ecadd(0);
@@ -1243,10 +1254,11 @@ par_case(int *cmplx)
* this doesn't affect our ability to match a | or ) as
* these are valid on command lines.
*/
- incasepat = 0;
+ incasepat = -1;
incmdpos = 1;
- for (;;) {
+ if (!skip_zshlex)
zshlex();
+ for (;;) {
if (tok == OUTPAR) {
ecstr(str);
ecadd(ecnpats++);
@@ -1302,10 +1314,26 @@ par_case(int *cmplx)
}
zshlex();
- if (tok != STRING)
+ switch (tok) {
+ case STRING:
+ /* Normal case */
+ str = dupstring(tokstr);
+ zshlex();
+ break;
+
+ case OUTPAR:
+ case BAR:
+ /* Empty string */
+ str = dupstring("");
+ break;
+
+ default:
+ /* Oops. */
YYERRORV(oecused);
- str = dupstring(tokstr);
+ break;
+ }
}
+ incasepat = 0;
par_save_list(cmplx);
if (tok == SEMIAMP)
type = WC_CASE_AND;
diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
index 9625a15..0302c96 100644
--- a/Test/A01grammar.ztst
+++ b/Test/A01grammar.ztst
@@ -820,6 +820,43 @@
0:case keeps exit status of last command executed in compound-list
>37
+ case '' in
+ burble) print No.
+ ;;
+ spurble|) print Yes!
+ ;;
+ |burble) print Not quite.
+ ;;
+ esac
+ case '' in
+ burble) print No.
+ ;;
+ |burble) print Wow!
+ ;;
+ spurble|) print Sorry.
+ ;;
+ esac
+ case '' in
+ gurgle) print No.
+ ;;
+ wurgle||jurgle) print Yikes!
+ ;;
+ durgle|) print Hmm.
+ ;;
+ |zurgle) print Hah.
+ ;;
+ esac
+ case '' in
+ # Useless doubled empty string to check special case.
+ ||jurgle) print Ok.
+ ;;
+ esac
+0: case with no opening parentheses and empty string
+>Yes!
+>Wow!
+>Yikes!
+>Ok.
+
x=1
x=2 | echo $x
echo $x
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 5.4 almost released
2017-08-07 18:34 ` Peter Stephenson
@ 2017-08-08 10:26 ` Martijn Dekker
2017-08-08 11:00 ` Peter Stephenson
0 siblings, 1 reply; 11+ messages in thread
From: Martijn Dekker @ 2017-08-08 10:26 UTC (permalink / raw)
To: Peter Stephenson, Zsh hackers list
Op 07-08-17 om 19:34 schreef Peter Stephenson:
> On Mon, 7 Aug 2017 14:25:35 -0400
> Eric Cook <llua@gmx.com> wrote:
>> On 08/07/2017 02:03 PM, Peter Stephenson wrote:
>>> THe files for version 5.4 are now in www.zsh.org/dev. If a few people
>>> get a chance to check this is OK I'll announce it and tidy up.
>>>
>>> Thanks
>>> pws
>>>
>>
>> pardon the typo, 5.4 is on http://www.zsh.org/pub
>
> Er, yes, thank you.
The recent POSIX_STRINGS-related change (41499, 2eacbef) introduced at
least one serious bug. I think you should probably revert that commit
before the 5.4 release so it can have more testing before 5.4.1 or
whatever the release after this one is.
$* concatenated, IFS is space:
emulate sh
set " abc " " def ghi " " jkl "
IFS=' '
set xx$*yy
echo "$#,$1|$2|$3|$4|$5|$6"
Actual output: 5,xx|abc|def|ghi|jklyy|
Expected output: 6,xx|abc|def|ghi|jkl|yy
The 'yy' is joined to the final 'jkl' instead of becoming a separate
argument.
The same bug happens with $@ (which, when unquoted, acts identically to $*).
Thanks,
- M.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 5.4 almost released
2017-08-08 10:26 ` Martijn Dekker
@ 2017-08-08 11:00 ` Peter Stephenson
0 siblings, 0 replies; 11+ messages in thread
From: Peter Stephenson @ 2017-08-08 11:00 UTC (permalink / raw)
To: Martijn Dekker, Zsh hackers list
On Tue, 8 Aug 2017 11:26:47 +0100
Martijn Dekker <martijn@inlv.org> wrote:
> The recent POSIX_STRINGS-related change (41499, 2eacbef) introduced at
> least one serious bug. I think you should probably revert that commit
> before the 5.4 release so it can have more testing before 5.4.1 or
> whatever the release after this one is.
I'll probably back it off and make a 5.4.1 this evening as 5.4 is
already tagged. Any fix will therefore be delayed until at least the
next full release (presumably 5.5).
> $* concatenated, IFS is space:
>
> emulate sh
> set " abc " " def ghi " " jkl "
> IFS=' '
> set xx$*yy
> echo "$#,$1|$2|$3|$4|$5|$6"
>
> Actual output: 5,xx|abc|def|ghi|jklyy|
> Expected output: 6,xx|abc|def|ghi|jkl|yy
>
> The 'yy' is joined to the final 'jkl' instead of becoming a separate
> argument.
In other words, we are splitting on $* because that's the parameter we
are substituting, and performing any further processing on the result of
that, whereas in effect the rules want us to think of xx$1 ... ${N}yy as
arguments to be split (and the yy might itself be a further substitution).
I'm not sure we have a suitable level of expansion at which to fix this
up neatly, and cases like $*$x where $x is empty but we haven't expanded
it yet when we encounter $* are likely to be horrible. I will therefore
probably leave the fix to someone who has more interest in POSIX tweaks.
pws
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-08-08 11:01 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20170807135641epcas3p4bc0a64f832ae7fcd76051ac198722045@epcas3p4.samsung.com>
2017-08-07 13:55 ` parsing empty alternatives: case foo|) :;; Daniel Shahaf
2017-08-07 14:26 ` Peter Stephenson
2017-08-07 14:44 ` Bart Schaefer
2017-08-07 14:58 ` Peter Stephenson
2017-08-07 18:03 ` 5.4 almost released Peter Stephenson
2017-08-07 18:25 ` Eric Cook
2017-08-07 18:34 ` Peter Stephenson
2017-08-08 10:26 ` Martijn Dekker
2017-08-08 11:00 ` Peter Stephenson
2017-08-07 18:21 ` parsing empty alternatives: case foo|) :;; Peter Stephenson
2017-08-07 18:56 ` Peter Stephenson
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).