* 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: 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: 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
* 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: 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
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).