Re: broken parsing with $((`:`))

Re: broken parsing with $((`:`))

[Mikael Magnusson]

On Wed, Apr 15, 2015 at 11:03 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Tue, 14 Apr 2015 23:05:31 -0400
> Mike Frysinger <vapier@gentoo.org> wrote:
>> looks like zsh doesn't correctly parse this:
>> $ zsh -c 'echo $((`:`))'
>> zsh:1: bad math expression: illegal character: \M-]
>>
>> looks like it's related to the subshell not outputting anything.
>
> Yes, indeed --- there's a funny internal special case for empty strings
> that I never quite get my head around.
>
> (Please, God, make the problems with command and math substitution
> parsing stop now.)

% $(
cmdsubst> [press ctrl-c here]
zsh: parse error near `$('

I would expect to not get an error message about syntax after pressing
ctrl-c. It doesn't happen with $((, { or any other unbalanced thingers
I tried.

-- 
Mikael Magnusson

PATCH: unwanted error aborting continued command substitution

[Peter Stephenson]

On Fri, 17 Apr 2015 06:39:29 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> % $(
> cmdsubst> [press ctrl-c here]
> zsh: parse error near `$('
> 
> I would expect to not get an error message about syntax after pressing
> ctrl-c.

I think that means we can handle this case as part of a general tidy up
rather than agonising over the complexities of the nested parsing ---
which means we can reap the benefit [sorry, leverage the synergy] of
breaking out the interrupt flag.  The following looks plausible, though
the interactions are complicated.

I believe the fix for only resetting the interrupt flag when resetting
the local keymap if there actually *was* a local keymap has to be
correct, but you never quite know if there are scary special cases that
use things differently...

pws

diff --git a/Src/Zle/zle_keymap.c b/Src/Zle/zle_keymap.c
index cfef882..c6fae25 100644
--- a/Src/Zle/zle_keymap.c
+++ b/Src/Zle/zle_keymap.c
@@ -503,8 +503,9 @@ selectkeymap(char *name, int fb)
 mod_export void
 selectlocalmap(Keymap m)
 {
+    Keymap oldm = localkeymap;
     localkeymap = m;
-    if (!m)
+    if (oldm && !m)
     {
 	/*
 	 * No local keymap; so we are returning to the global map.  If
diff --git a/Src/lex.c b/Src/lex.c
index 184a54b..c929bb9 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -1558,10 +1558,12 @@ parsestr(char **s)
 
     if ((err = parsestrnoerr(s))) {
 	untokenize(*s);
-	if (err > 32 && err < 127)
-	    zerr("parse error near `%c'", err);
-	else
-	    zerr("parse error");
+	if (!(errflag & ERRFLAG_INT)) {
+	    if (err > 32 && err < 127)
+		zerr("parse error near `%c'", err);
+	    else
+		zerr("parse error");
+	}
     }
     return err;
 }
diff --git a/Src/parse.c b/Src/parse.c
index 0b54a90..91a81e1 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -2419,7 +2419,7 @@ yyerror(int noerr)
     for (t0 = 0; t0 != 20; t0++)
 	if (!t || !t[t0] || t[t0] == '\n')
 	    break;
-    if (!(histdone & HISTFLAG_NOEXEC)) {
+    if (!(histdone & HISTFLAG_NOEXEC) && !(errflag & ERRFLAG_INT)) {
 	if (t0 == 20)
 	    zwarn("parse error near `%l...'", t, 20);
 	else if (t0)

Re: PATCH: unwanted error aborting continued command substitution

[Mikael Magnusson]

On Fri, Apr 17, 2015 at 11:16 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Fri, 17 Apr 2015 06:39:29 +0200
> Mikael Magnusson <mikachu@gmail.com> wrote:
>> % $(
>> cmdsubst> [press ctrl-c here]
>> zsh: parse error near `$('
>>
>> I would expect to not get an error message about syntax after pressing
>> ctrl-c.
>
> I think that means we can handle this case as part of a general tidy up
> rather than agonising over the complexities of the nested parsing ---
> which means we can reap the benefit [sorry, leverage the synergy] of
> breaking out the interrupt flag.  The following looks plausible, though
> the interactions are complicated.
>
> I believe the fix for only resetting the interrupt flag when resetting
> the local keymap if there actually *was* a local keymap has to be
> correct, but you never quite know if there are scary special cases that
> use things differently...

I didn't think to try this yesterday, but invoking push-input gives
the same error (sorry).

-- 
Mikael Magnusson

Re: PATCH: unwanted error aborting continued command substitution

[Peter Stephenson]

On Fri, 17 Apr 2015 12:52:56 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> On Fri, Apr 17, 2015 at 11:16 AM, Peter Stephenson
> <p.stephenson@samsung.com> wrote:
> > On Fri, 17 Apr 2015 06:39:29 +0200
> > Mikael Magnusson <mikachu@gmail.com> wrote:
> >> % $(
> >> cmdsubst> [press ctrl-c here]
> >> zsh: parse error near `$('
>
> I didn't think to try this yesterday, but invoking push-input gives
> the same error (sorry).

That's a completely different kettle of fish --- push-input is basically
a hack to try to convince the line editor we've left it so we can start
again.  But we can't start again without leaving the nested parsing of
$(.  I have no idea what to do here except document it as "don't do
that".  Maybe Bart understands the sequence for this better.

pws

Re: PATCH: unwanted error aborting continued command substitution

[Bart Schaefer]

On Apr 17, 12:02pm, Peter Stephenson wrote:
} Subject: Re: PATCH: unwanted error aborting continued command substitution
}
} On Fri, 17 Apr 2015 12:52:56 +0200
} Mikael Magnusson <mikachu@gmail.com> wrote:
} > On Fri, Apr 17, 2015 at 11:16 AM, Peter Stephenson
} > <p.stephenson@samsung.com> wrote:
} > > On Fri, 17 Apr 2015 06:39:29 +0200
} > > Mikael Magnusson <mikachu@gmail.com> wrote:
} > >> % $(
} > >> cmdsubst> [press ctrl-c here]
} > >> zsh: parse error near `$('
} >
} > I didn't think to try this yesterday, but invoking push-input gives
} > the same error (sorry).

So does send-break (using e.g. ESC-x send-break RET):

torch% $(
cmdsubst> 
execute: send-break_
zsh: parse error near `$('
torch% 

} That's a completely different kettle of fish --- push-input is basically
} a hack to try to convince the line editor we've left it so we can start
} again.

Yes it's a hack, but it's the same kettle -- push-input uses send-break,
and send-break is supposed to be a simulated interrupt for cases where
the TTY intr character is not set.

So either send-break should actually set ERRFLAG_INT, or we need a third
error flag for simulated interrupts.

Re: PATCH: unwanted error aborting continued command substitution

[Peter Stephenson]

On Fri, 17 Apr 2015 08:46:45 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> push-input uses send-break,
> and send-break is supposed to be a simulated interrupt for cases where
> the TTY intr character is not set.
> 
> So either send-break should actually set ERRFLAG_INT, or we need a third
> error flag for simulated interrupts.

Actually, it doesn't bother calling send-break, but I've found the point
where it doesn't.

Maybe the following is OK --- in fact, possibly we don't need the
ERRFLAG_ERROR bit set, but I don't think it's doing any harm.

One advantage of using a different bit would be we could abort back to
the top of ZLE rather than out of ZLE and back in again, which would
mean you finally had the ability to embed push-line in other widgets.
But that's for another day.

pws

diff --git a/Src/Zle/zle_hist.c b/Src/Zle/zle_hist.c
index 88623bb..cc66f99 100644
--- a/Src/Zle/zle_hist.c
+++ b/Src/Zle/zle_hist.c
@@ -854,7 +854,7 @@ pushlineoredit(char **args)
     }
     ret = pushline(args);
     if (!isfirstln) {
-	errflag |= ERRFLAG_ERROR;
+	errflag |= ERRFLAG_ERROR|ERRFLAG_INT;
 	done = 1;
     }
     clearlist = 1;
diff --git a/Src/Zle/zle_misc.c b/Src/Zle/zle_misc.c
index 23286fc..4669ef2 100644
--- a/Src/Zle/zle_misc.c
+++ b/Src/Zle/zle_misc.c
@@ -1041,7 +1041,7 @@ copyprevshellword(UNUSED(char **args))
 int
 sendbreak(UNUSED(char **args))
 {
-    errflag |= ERRFLAG_ERROR;
+    errflag |= ERRFLAG_ERROR|ERRFLAG_INT;
     return 1;
 }