zsh-workers
 help / color / mirror / code / Atom feed
* capturing output of !! not working
@ 2015-03-19  1:49 Vin Shelton
  2015-03-19  2:28 ` Bart Schaefer
  2015-03-19 10:57 ` Peter Stephenson
  0 siblings, 2 replies; 33+ messages in thread
From: Vin Shelton @ 2015-03-19  1:49 UTC (permalink / raw)
  To: Zsh Hackers' List

>From latest sources:

$ mkdir foo; cd ./foo
$ touch a b c
$ echo abc >d
$ grep -l abc ?
d
$ ls -l $(!!)
ls -l $()
total 4
-rw-r--r-- 1 acs acs 0 Mar 18 20:23 a
-rw-r--r-- 1 acs acs 0 Mar 18 20:23 b
-rw-r--r-- 1 acs acs 0 Mar 18 20:23 c
-rw-r--r-- 1 acs acs 4 Mar 18 20:23 d

This happens for me even in a "zsh -f".

  - Vin


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: capturing output of !! not working
  2015-03-19  1:49 capturing output of !! not working Vin Shelton
@ 2015-03-19  2:28 ` Bart Schaefer
  2015-03-19 10:57 ` Peter Stephenson
  1 sibling, 0 replies; 33+ messages in thread
From: Bart Schaefer @ 2015-03-19  2:28 UTC (permalink / raw)
  To: Vin Shelton; +Cc: Zsh Hackers' List

On Wed, Mar 18, 2015 at 6:49 PM, Vin Shelton <acs@alumni.princeton.edu> wrote:
> $ ls -l $(!!)
> ls -l $()

If I back all the way off to 870d880befaede1f55f232717a93ed07b9edb4c6,
I get the first inklings of this:

schaefer<502> echo $( !! )                                          5.0.7-dev-0
echo $( echo foo )
zsh: command not found: !!

Note the printed expansion is correct but that's not what gets executed.

It appears it was last working in
93846edb0d5d606e167f929532608eaea273c23f before workers/34160.


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: capturing output of !! not working
  2015-03-19  1:49 capturing output of !! not working Vin Shelton
  2015-03-19  2:28 ` Bart Schaefer
@ 2015-03-19 10:57 ` Peter Stephenson
  2015-03-19 12:27   ` Vin Shelton
  1 sibling, 1 reply; 33+ messages in thread
From: Peter Stephenson @ 2015-03-19 10:57 UTC (permalink / raw)
  To: Zsh Hackers' List

On Wed, 18 Mar 2015 21:49:37 -0400
Vin Shelton <acs@alumni.princeton.edu> wrote:
> $ ls -l $(!!)
> ls -l $()

This is due to special code for handling aliases inside command
substitutions that got applied too widely because the flag that
indicates aliases also indicates history expansion; however, there's a
discriminator in another bit.  I suspect restricting application of
INP_ALIAS is a better fix overall, but it needs a lot more code tracking
down plus safety checks.

Another case where we need a framework for testing interactive shells.

pws

diff --git a/Src/hist.c b/Src/hist.c
index aa07ce8..b7ef522 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -338,7 +338,8 @@ ihwaddc(int c)
 	 * fashion as we never need the expansion in the history
 	 * line, only in the lexer and above.
 	 */
-	!((histactive & HA_INWORD) && (inbufflags & INP_ALIAS))) {
+	!((histactive & HA_INWORD) &&
+	  (inbufflags & (INP_ALIAS|INP_HIST)) == INP_ALIAS)) {
 	/* Quote un-expanded bangs in the history line. */
 	if (c == bangchar && stophist < 2 && qbang)
 	    /* If qbang is not set, we do not escape this bangchar as it's *
@@ -901,7 +902,8 @@ ihungetc(int c)
 	    zlemetall--;
 	    exlast++;
 	}
-	if (!(histactive & HA_INWORD) || !(inbufflags & INP_ALIAS)) {
+	if (!(histactive & HA_INWORD) ||
+	    (inbufflags & (INP_ALIAS|INP_HIST)) != INP_ALIAS) {
 	    DPUTS(hptr <= chline, "BUG: hungetc attempted at buffer start");
 	    hptr--;
 	    DPUTS(*hptr != (char) c, "BUG: wrong character in hungetc() ");


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: capturing output of !! not working
  2015-03-19 10:57 ` Peter Stephenson
@ 2015-03-19 12:27   ` Vin Shelton
  2015-03-19 12:53     ` Peter Stephenson
  0 siblings, 1 reply; 33+ messages in thread
From: Vin Shelton @ 2015-03-19 12:27 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh Hackers' List

But it still doesn't work.  The !! command now appears inside the $(),
but the output is not captured:

$ mkdir foo
$ cd foo
$ touch a
$ echo abc > b
$ grep -l abc ?
b
$ ls -l $(!!)
ls -l $(grep -l abc ?)
total 4
-rw-r--r-- 1 acs acs 0 Mar 19 08:25 a
-rw-r--r-- 1 acs acs 4 Mar 19 08:25 b


  - Vin

On Thu, Mar 19, 2015 at 6:57 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Wed, 18 Mar 2015 21:49:37 -0400
> Vin Shelton <acs@alumni.princeton.edu> wrote:
>> $ ls -l $(!!)
>> ls -l $()
>
> This is due to special code for handling aliases inside command
> substitutions that got applied too widely because the flag that
> indicates aliases also indicates history expansion; however, there's a
> discriminator in another bit.  I suspect restricting application of
> INP_ALIAS is a better fix overall, but it needs a lot more code tracking
> down plus safety checks.
>
> Another case where we need a framework for testing interactive shells.
>
> pws
>
> diff --git a/Src/hist.c b/Src/hist.c
> index aa07ce8..b7ef522 100644
> --- a/Src/hist.c
> +++ b/Src/hist.c
> @@ -338,7 +338,8 @@ ihwaddc(int c)
>          * fashion as we never need the expansion in the history
>          * line, only in the lexer and above.
>          */
> -       !((histactive & HA_INWORD) && (inbufflags & INP_ALIAS))) {
> +       !((histactive & HA_INWORD) &&
> +         (inbufflags & (INP_ALIAS|INP_HIST)) == INP_ALIAS)) {
>         /* Quote un-expanded bangs in the history line. */
>         if (c == bangchar && stophist < 2 && qbang)
>             /* If qbang is not set, we do not escape this bangchar as it's *
> @@ -901,7 +902,8 @@ ihungetc(int c)
>             zlemetall--;
>             exlast++;
>         }
> -       if (!(histactive & HA_INWORD) || !(inbufflags & INP_ALIAS)) {
> +       if (!(histactive & HA_INWORD) ||
> +           (inbufflags & (INP_ALIAS|INP_HIST)) != INP_ALIAS) {
>             DPUTS(hptr <= chline, "BUG: hungetc attempted at buffer start");
>             hptr--;
>             DPUTS(*hptr != (char) c, "BUG: wrong character in hungetc() ");


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: capturing output of !! not working
  2015-03-19 12:27   ` Vin Shelton
@ 2015-03-19 12:53     ` Peter Stephenson
  2015-03-20 10:57       ` Peter Stephenson
  0 siblings, 1 reply; 33+ messages in thread
From: Peter Stephenson @ 2015-03-19 12:53 UTC (permalink / raw)
  To: Zsh Hackers' List

On Thu, 19 Mar 2015 08:27:56 -0400
Vin Shelton <acs@alumni.princeton.edu> wrote:
> But it still doesn't work.  The !! command now appears inside the $(),
> but the output is not captured:
> 
> $ mkdir foo
> $ cd foo
> $ touch a
> $ echo abc > b
> $ grep -l abc ?
> b
> $ ls -l $(!!)
> ls -l $(grep -l abc ?)
> total 4
> -rw-r--r-- 1 acs acs 0 Mar 19 08:25 a
> -rw-r--r-- 1 acs acs 4 Mar 19 08:25 b

You're right --- I was using HIST_VERIFY which works OK now and I didn't
realise there was another problem.

This will be an effect of the history substitution turning up in the
wrong place (hence you're seeing it on stdout which you shouldn't) owing
to the interaction with the nested parsing.  That'll take somewhat
longer to sort out.

pws


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: capturing output of !! not working
  2015-03-19 12:53     ` Peter Stephenson
@ 2015-03-20 10:57       ` Peter Stephenson
  2015-03-20 16:04         ` Bart Schaefer
  0 siblings, 1 reply; 33+ messages in thread
From: Peter Stephenson @ 2015-03-20 10:57 UTC (permalink / raw)
  To: Zsh Hackers' List

On Thu, 19 Mar 2015 12:53:51 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Thu, 19 Mar 2015 08:27:56 -0400
> Vin Shelton <acs@alumni.princeton.edu> wrote:
> > The !! command now appears inside the $(),
> > but the output is not captured:
>
> This will be an effect of the history substitution turning up in the
> wrong place (hence you're seeing it on stdout which you shouldn't) owing
> to the interaction with the nested parsing.

Not quite --- it's another similar problem to the last, that history and
alias expansions are pushed onto the input stack in a similar way but
need different processing.  The new code is actually slightly more
efficient; inungetc() did too much work for the case in question.
The comment I've added is a distinct oversimplification, but I don't
want to write an essay.

I think what I'm seeing here is another symptom of the general
entanglement between input levels (that's been there since the start)
that's also been giving Bart headaches.

You *should* see the expansion on stdout, that's completely standard.
Because I always use HIST_VERIFY I'd forgotten.

It shouldn't be rocket science to simplify the zpty-based stuff to the
point where we have a framework for testing history.  However, the
baseline for what counts as rocket science is a little bit different in
these parts.

pws

diff --git a/Src/input.c b/Src/input.c
index 92b1ad1..30970a0 100644
--- a/Src/input.c
+++ b/Src/input.c
@@ -571,8 +571,20 @@ inpoptop(void)
 {
     if (!lexstop) {
 	inbufflags &= ~INP_ALCONT;
-	while (inbufptr > inbuf)
-	    inungetc(inbufptr[-1]);
+	while (inbufptr > inbuf) {
+	    inbufptr--;
+	    inbufct++;
+	    inbufleft++;
+	    /*
+	     * As elsewhere in input and history mechanisms:
+	     * unwinding aliases and unwinding history have different
+	     * implications as aliases are after the lexer while
+	     * history is before, but they're both pushed onto
+	     * the input stack.
+	     */
+	    if ((inbufflags & (INP_ALIAS|INP_HIST)) == INP_ALIAS)
+		zshlex_raw_back();
+	}
     }
 
     if (inbuf && (inbufflags & INP_FREE))


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: capturing output of !! not working
  2015-03-20 10:57       ` Peter Stephenson
@ 2015-03-20 16:04         ` Bart Schaefer
  2015-03-22 18:35           ` Peter Stephenson
  0 siblings, 1 reply; 33+ messages in thread
From: Bart Schaefer @ 2015-03-20 16:04 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mar 20, 10:57am, Peter Stephenson wrote:
}
} Not quite --- it's another similar problem to the last, that history and
} alias expansions are pushed onto the input stack in a similar way but
} need different processing.  The new code is actually slightly more
} efficient; inungetc() did too much work for the case in question.
} The comment I've added is a distinct oversimplification, but I don't
} want to write an essay.
} 
} I think what I'm seeing here is another symptom of the general
} entanglement between input levels (that's been there since the start)
} that's also been giving Bart headaches.

Your patch does not entirely resolve the problem -- this works:

torch% echo $(!!)
echo $(echo foo bar)
foo bar

But this does not:

torch% echo foo bar
foo bar
torch% echo $( !! )
echo $( echo foo bar )
zsh: command not found: !!echo


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: capturing output of !! not working
  2015-03-20 16:04         ` Bart Schaefer
@ 2015-03-22 18:35           ` Peter Stephenson
  2015-03-22 19:18             ` Peter Stephenson
  2015-03-22 23:22             ` Bart Schaefer
  0 siblings, 2 replies; 33+ messages in thread
From: Peter Stephenson @ 2015-03-22 18:35 UTC (permalink / raw)
  To: Zsh Hackers' List

On Fri, 20 Mar 2015 09:04:20 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Your patch does not entirely resolve the problem -- this works:
> 
> torch% echo $(!!)
> echo $(echo foo bar)
> foo bar
> 
> But this does not:
> 
> torch% echo foo bar
> foo bar
> torch% echo $( !! )
> echo $( echo foo bar )
> zsh: command not found: !!echo

oh for @*!*!'s sake.

As I've noted in the comment, to get this stuff to work cleanly we need
to capture character-by-character input at a level over the history, but
we don't have one and there are probably too many levels anyway.  So
this will have to do --- it's at least fairly obvious what's going on.

pws

diff --git a/Src/hist.c b/Src/hist.c
index b7ef522..f3ab2b5 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -527,10 +527,20 @@ histsubchar(int c)
     static int marg = -1;
     static zlong mev = -1;
     char *buf, *ptr;
-    char *sline;
+    char *sline, *lexraw_mark;
     Histent ehist;
     size_t buflen;
 
+    /*
+     * If accumulating raw input for use in command substitution,
+     * we don't want the history text, so mark it for later removal.
+     * It would be better to do this at a level above the history
+     * and below the lexer --- but there isn't one.
+     *
+     * Include the character we are attempting to substitute.
+     */
+    lexraw_mark = zshlex_raw_mark(-1); 
+
     /* look, no goto's */
     if (isfirstch && c == hatchar) {
 	int gbal = 0;
@@ -864,6 +874,8 @@ histsubchar(int c)
 	}
     }
 
+    zshlex_raw_back_to_mark(lexraw_mark);
+
     /*
      * Push the expanded value onto the input stack,
      * marking this as a history word for purposes of the alias stack.
diff --git a/Src/lex.c b/Src/lex.c
index 1eb0bc7..6b9e942 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -1871,6 +1871,25 @@ zshlex_raw_back(void)
     lexbuf_raw.len--;
 }
 
+/**/
+char *
+zshlex_raw_mark(int offset)
+{
+    if (!lex_add_raw)
+	return NULL;
+    return lexbuf_raw.ptr + offset;
+}
+
+/**/
+void
+zshlex_raw_back_to_mark(char *mark)
+{
+    if (!lex_add_raw)
+	return;
+    lexbuf_raw.len -= lexbuf_raw.ptr - mark;
+    lexbuf_raw.ptr = mark;
+}
+
 /*
  * Skip (...) for command-style substitutions: $(...), <(...), >(...)
  *


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: capturing output of !! not working
  2015-03-22 18:35           ` Peter Stephenson
@ 2015-03-22 19:18             ` Peter Stephenson
  2015-03-22 23:22             ` Bart Schaefer
  1 sibling, 0 replies; 33+ messages in thread
From: Peter Stephenson @ 2015-03-22 19:18 UTC (permalink / raw)
  To: Zsh Hackers' List

On Sun, 22 Mar 2015 18:35:56 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> +/**/
> +char *
> +zshlex_raw_mark(int offset)
> +{
> +    if (!lex_add_raw)
> +	return NULL;
> +    return lexbuf_raw.ptr + offset;
> +}
> +
> +/**/
> +void
> +zshlex_raw_back_to_mark(char *mark)
> +{
> +    if (!lex_add_raw)
> +	return;
> +    lexbuf_raw.len -= lexbuf_raw.ptr - mark;
> +    lexbuf_raw.ptr = mark;
> +}
> +

It may have been reallocated... this is safer.

diff --git a/Src/hist.c b/Src/hist.c
index 70dfac0..b44f4ad 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -527,7 +527,8 @@ histsubchar(int c)
     static int marg = -1;
     static zlong mev = -1;
     char *buf, *ptr;
-    char *sline, *lexraw_mark;
+    char *sline;
+    int lexraw_mark;
     Histent ehist;
     size_t buflen;
 
diff --git a/Src/lex.c b/Src/lex.c
index 6b9e942..6d0079c 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -1872,22 +1872,22 @@ zshlex_raw_back(void)
 }
 
 /**/
-char *
+int
 zshlex_raw_mark(int offset)
 {
     if (!lex_add_raw)
-	return NULL;
-    return lexbuf_raw.ptr + offset;
+	return 0;
+    return lexbuf_raw.len + offset;
 }
 
 /**/
 void
-zshlex_raw_back_to_mark(char *mark)
+zshlex_raw_back_to_mark(int mark)
 {
     if (!lex_add_raw)
 	return;
-    lexbuf_raw.len -= lexbuf_raw.ptr - mark;
-    lexbuf_raw.ptr = mark;
+    lexbuf_raw.ptr = tokstr_raw + mark;
+    lexbuf_raw.len = mark;
 }
 
 /*


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: capturing output of !! not working
  2015-03-22 18:35           ` Peter Stephenson
  2015-03-22 19:18             ` Peter Stephenson
@ 2015-03-22 23:22             ` Bart Schaefer
  2015-03-23 21:34               ` Peter Stephenson
  1 sibling, 1 reply; 33+ messages in thread
From: Bart Schaefer @ 2015-03-22 23:22 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mar 22,  6:35pm, Peter Stephenson wrote:
}
} As I've noted in the comment, to get this stuff to work cleanly we need
} to capture character-by-character input at a level over the history, but
} we don't have one and there are probably too many levels anyway.  So
} this will have to do --- it's at least fairly obvious what's going on.

OK, now we just need to do something similar for this:

torch% alias '{'=echo
torch% {bar
zsh: command not found: echobar
torch% fc -l
    1  alias '{'=echo
    2  {echobar

That must be the same problem as the "!!echo" in my $( !! ) example, no?

Here's a test, which I have dropped in the previously-unused "W" category
for "builtin interactive commands and constructs".  If that doesn't seem
correct, rename to D10history.


diff --git a/Test/W01history.ztst b/Test/W01history.ztst
new file mode 100644
index 0000000..2bdcc32
--- /dev/null
+++ b/Test/W01history.ztst
@@ -0,0 +1,15 @@
+# Tests for BANG_HIST replacements
+
+%test
+
+  $ZTST_testdir/../Src/zsh -fis <<<'
+  echo foo bar
+  echo $(!!) again
+  echo more $( !! )'
+0:Regression test for history references in command substitution
+>foo bar
+>foo bar again
+>more foo bar again
+*?*
+F:Check that a history bug introduced by workers/34160 is working again.
+F:Discarded line of error output consumes prompts printed by "zsh -i".

-- 
Barton E. Schaefer


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: capturing output of !! not working
  2015-03-22 23:22             ` Bart Schaefer
@ 2015-03-23 21:34               ` Peter Stephenson
  2015-03-24  0:54                 ` Testing interactive features (Re: capturing output of !! not working) Bart Schaefer
  2015-03-25 15:48                 ` PATCH: Removing aliases from history, 2015 style (was " Peter Stephenson
  0 siblings, 2 replies; 33+ messages in thread
From: Peter Stephenson @ 2015-03-23 21:34 UTC (permalink / raw)
  To: Zsh Hackers' List

On Sun, 22 Mar 2015 16:22:35 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Mar 22,  6:35pm, Peter Stephenson wrote:
> }
> } As I've noted in the comment, to get this stuff to work cleanly we need
> } to capture character-by-character input at a level over the history, but
> } we don't have one and there are probably too many levels anyway.  So
> } this will have to do --- it's at least fairly obvious what's going on.
> 
> OK, now we just need to do something similar for this:
> 
> torch% alias '{'=echo
> torch% {bar
> zsh: command not found: echobar
> torch% fc -l
>     1  alias '{'=echo
>     2  {echobar

What appears to be happening here is that because there's no end of word
character after the expanded "{" we run straight onto the "bar" and only
call ihwend() via hwend() after that.  As alias expansion is tied to
word-handling, we only then look at the test that resets the position in
the history line, which is too late.

I'm not sure how to fix this.  Ideally, I think we'd handle not adding
the alias text to history the way we've been doing in other cases by
checking INP_* flags, which is much less hit and miss.  That ought not
to be too difficult, but I haven't really got my mind round the
consequences yet.

Everything else I can think of is a bit of a hack (and surely we'd never
etc. etc.)

> Here's a test, which I have dropped in the previously-unused "W" category
> for "builtin interactive commands and constructs".  If that doesn't seem
> correct, rename to D10history.

Looks fine.  I'll try to expand it when it appears.

pws


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Testing interactive features (Re: capturing output of !! not working)
  2015-03-23 21:34               ` Peter Stephenson
@ 2015-03-24  0:54                 ` Bart Schaefer
  2015-03-24  4:12                   ` Bart Schaefer
  2015-03-25 15:48                 ` PATCH: Removing aliases from history, 2015 style (was " Peter Stephenson
  1 sibling, 1 reply; 33+ messages in thread
From: Bart Schaefer @ 2015-03-24  0:54 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mar 23,  9:34pm, Peter Stephenson wrote:
} Subject: Re: capturing output of !! not working
}
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > Here's a test, which I have dropped in the previously-unused "W"
} > category for "builtin interactive commands and constructs". If that
} > doesn't seem correct, rename to D10history.
} 
} Looks fine.  I'll try to expand it when it appears.

Since writing it, I noticed that history aggressively directs output
to /dev/tty.  I haven't come up with any way to suppress that, and if
you run it without a tty the stderr pattern does not match.

However, that's not the only test that has problems if there is no tty:

***************
*** 0 ****
--- 1,3 ----
+ not interactive and can't open terminal
+ not interactive and can't open terminal
+ not interactive and can't open terminal
Test ../../zsh-5.0/Test/X02zlevi.ztst failed: error output differs from
expected as shown above for:
  zpty_run 'bindkey -r "\e~"'
  zletest $'\e' $'~aI\e' $'~o\e' \~
Was testing: swap case on a blank line


I'm surprised that hasn't come up in anyone's automated build system yet.


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Testing interactive features (Re: capturing output of !! not working)
  2015-03-24  0:54                 ` Testing interactive features (Re: capturing output of !! not working) Bart Schaefer
@ 2015-03-24  4:12                   ` Bart Schaefer
  2015-03-24  4:45                     ` Bart Schaefer
  0 siblings, 1 reply; 33+ messages in thread
From: Bart Schaefer @ 2015-03-24  4:12 UTC (permalink / raw)
  To: zsh-workers

On Mar 23,  5:54pm, Bart Schaefer wrote:
}
} Since writing it, I noticed that history aggressively directs output
} to /dev/tty.  I haven't come up with any way to suppress that, and if
} you run it without a tty the stderr pattern does not match.
} 
} However, that's not the only test that has problems if there is no tty:
} 
} + not interactive and can't open terminal
} + not interactive and can't open terminal
} + not interactive and can't open terminal
} Test ../../zsh-5.0/Test/X02zlevi.ztst failed: error output differs from

Here's a revised Test/W01* that works with no TTY and warns about the
output when there is one, plus a patch to comptest for the Test/X01*
failure with no TTY.


diff --git a/Test/W01history.ztst b/Test/W01history.ztst
new file mode 100644
index 0000000..2492c41
--- /dev/null
+++ b/Test/W01history.ztst
@@ -0,0 +1,19 @@
+# Tests for BANG_HIST replacements
+
+%prep
+
+  [[ -t 0 ]] && print -u $ZTST_fd History tests write to /dev/tty
+
+%test
+
+  $ZTST_testdir/../Src/zsh -fis <<<'
+  echo foo bar
+  echo $(!!) again
+  echo more $( !! )' 2>/dev/null
+0:Regression test for history references in command substitution
+>foo bar
+>foo bar again
+>more foo bar again
+*?*
+F:Check that a history bug introduced by workers/34160 is working again.
+F:Discarded line of error output consumes prompts printed by "zsh -i".
diff --git a/Test/comptest b/Test/comptest
index 9c92f96..ef84217 100644
--- a/Test/comptest
+++ b/Test/comptest
@@ -164,7 +164,7 @@ zletest () {
   for input; do
     # zpty_flush Before zletest
     # sleep for $KEYTIMEOUT
-    (( first++ )) && read -t 0.011 -k 1 < /dev/null
+    (( first++ )) && read -t 0.011 -u 0 -k 1 < /dev/null
     zpty -n -w zsh "$input"
   done
   zpty -n -w zsh $'\C-X'


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Testing interactive features (Re: capturing output of !! not working)
  2015-03-24  4:12                   ` Bart Schaefer
@ 2015-03-24  4:45                     ` Bart Schaefer
  2015-03-24 16:09                       ` Bart Schaefer
  0 siblings, 1 reply; 33+ messages in thread
From: Bart Schaefer @ 2015-03-24  4:45 UTC (permalink / raw)
  To: zsh-workers

On Mar 23,  9:12pm, Bart Schaefer wrote:
}
} ... a patch to comptest for the Test/X01* failure with no TTY.
} 
} -    (( first++ )) && read -t 0.011 -k 1 < /dev/null
} +    (( first++ )) && read -t 0.011 -u 0 -k 1 < /dev/null

Well, that shuts up the error message, but it also causes the 0.011
second timeout to be ignored.  The test still succeeds for me, but
should get a more thorough examination.

Maybe we just needed -u 0 *instead* of -t all along.


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Testing interactive features (Re: capturing output of !! not working)
  2015-03-24  4:45                     ` Bart Schaefer
@ 2015-03-24 16:09                       ` Bart Schaefer
  2016-05-23 14:53                         ` Mikael Magnusson
  0 siblings, 1 reply; 33+ messages in thread
From: Bart Schaefer @ 2015-03-24 16:09 UTC (permalink / raw)
  To: zsh-workers

On Mar 23,  9:45pm, Bart Schaefer wrote:
} Subject: Re: Testing interactive features (Re: capturing output of !! not 
}
} } ... a patch to comptest for the Test/X01* failure with no TTY.
} } 
} } -    (( first++ )) && read -t 0.011 -k 1 < /dev/null
} } +    (( first++ )) && read -t 0.011 -u 0 -k 1 < /dev/null
} 
} Well, that shuts up the error message, but it also causes the 0.011
} second timeout to be ignored.  The test still succeeds for me, but
} should get a more thorough examination.

If the real point is to sleep for 0.11 seconds, input needs to be
redirected from somewhere that doesn't instantly return EOF.  That
means tty or an open pipe.  Can anyone think of a less silly-looking
way to do that, than the below?

(The -u 0 -k 1 are probably redundant now.)

First hunk is for a thinko in my %prep.

diff --git a/Test/W01history.ztst b/Test/W01history.ztst
index 2492c41..3a64530 100644
--- a/Test/W01history.ztst
+++ b/Test/W01history.ztst
@@ -2,7 +2,7 @@
 
 %prep
 
-  [[ -t 0 ]] && print -u $ZTST_fd History tests write to /dev/tty
+  if [[ -t 0 ]]; then print -u $ZTST_fd History tests write to /dev/tty; fi
 
 %test
 
diff --git a/Test/comptest b/Test/comptest
index ef84217..20a3a5b 100644
--- a/Test/comptest
+++ b/Test/comptest
@@ -164,7 +164,7 @@ zletest () {
   for input; do
     # zpty_flush Before zletest
     # sleep for $KEYTIMEOUT
-    (( first++ )) && read -t 0.011 -u 0 -k 1 < /dev/null
+    (( first++ )) && { sleep 2 & } | read -t 0.011 -u 0 -k 1
     zpty -n -w zsh "$input"
   done
   zpty -n -w zsh $'\C-X'


^ permalink raw reply	[flat|nested] 33+ messages in thread

* PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working)
  2015-03-23 21:34               ` Peter Stephenson
  2015-03-24  0:54                 ` Testing interactive features (Re: capturing output of !! not working) Bart Schaefer
@ 2015-03-25 15:48                 ` Peter Stephenson
  2015-03-25 17:57                   ` PATCH: Removing aliases from history, 2015 style Peter Stephenson
  2015-03-25 19:40                   ` PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working) Bart Schaefer
  1 sibling, 2 replies; 33+ messages in thread
From: Peter Stephenson @ 2015-03-25 15:48 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mon, 23 Mar 2015 21:34:26 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> Ideally, I think we'd handle not adding
> the alias text to history the way we've been doing in other cases by
> checking INP_* flags, which is much less hit and miss.  That ought not
> to be too difficult, but I haven't really got my mind round the
> consequences yet.

Hmm... I typed this in more or less as below (with one subsequent fix to
testing flags for interaction with !-history) and was expecting it to
fail horribly, and so far it's failed to fail.  Perhaps my expectations
were low simply because it's been the old way since 2 million BC.

I'll try it further.

Obviously this doesn't change the fact that "{bar" turns into "echobar"
for execution after alias expansion (it no longer does so in the history),
since that's up in the lexer.

Some more tidying up may now be possible given that removing aliases
from history is less of a special case.

hwget() had a special hack for aliases, which I don't think should be
needed now, but I'd like to know --- the DEBUG code should tell us if
something nasty is happening.

pws

diff --git a/Src/hist.c b/Src/hist.c
index b44f4ad..990e609 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -245,7 +245,6 @@ hist_context_save(struct hist_stack *hs, int toplevel)
     hs->chwords = chwords;
     hs->chwordlen = chwordlen;
     hs->chwordpos = chwordpos;
-    hs->hwgetword = hwgetword;
     hs->hgetc = hgetc;
     hs->hungetc = hungetc;
     hs->hwaddc = hwaddc;
@@ -289,7 +288,6 @@ hist_context_restore(const struct hist_stack *hs, int toplevel)
     chwords = hs->chwords;
     chwordlen = hs->chwordlen;
     chwordpos = hs->chwordpos;
-    hwgetword = hs->hwgetword;
     hgetc = hs->hgetc;
     hungetc = hs->hungetc;
     hwaddc = hs->hwaddc;
@@ -338,8 +336,7 @@ ihwaddc(int c)
 	 * fashion as we never need the expansion in the history
 	 * line, only in the lexer and above.
 	 */
-	!((histactive & HA_INWORD) &&
-	  (inbufflags & (INP_ALIAS|INP_HIST)) == INP_ALIAS)) {
+	(inbufflags & (INP_ALIAS|INP_HIST)) != INP_ALIAS) {
 	/* Quote un-expanded bangs in the history line. */
 	if (c == bangchar && stophist < 2 && qbang)
 	    /* If qbang is not set, we do not escape this bangchar as it's *
@@ -915,8 +912,7 @@ ihungetc(int c)
 	    zlemetall--;
 	    exlast++;
 	}
-	if (!(histactive & HA_INWORD) ||
-	    (inbufflags & (INP_ALIAS|INP_HIST)) != INP_ALIAS) {
+	if ((inbufflags & (INP_ALIAS|INP_HIST)) != INP_ALIAS) {
 	    DPUTS(hptr <= chline, "BUG: hungetc attempted at buffer start");
 	    hptr--;
 	    DPUTS(*hptr != (char) c, "BUG: wrong character in hungetc() ");
@@ -1536,28 +1532,17 @@ hend(Eprog prog)
     return !(flag & HISTFLAG_NOEXEC || errflag);
 }
 
-/* Gives current expansion word if not last word before chwordpos. */
-
-/**/
-int hwgetword = -1;
-
 /* begin a word */
 
 /**/
 void
 ihwbegin(int offset)
 {
-    if (stophist == 2 || (histactive & HA_INWORD))
+    if (stophist == 2 || (histactive & HA_INWORD) ||
+	(inbufflags & (INP_ALIAS|INP_HIST)) == INP_ALIAS)
 	return;
     if (chwordpos%2)
 	chwordpos--;	/* make sure we're on a word start, not end */
-    /* If we're expanding an alias, we should overwrite the expansion
-     * in the history.
-     */
-    if ((inbufflags & INP_ALIAS) && !(inbufflags & INP_HIST))
-	hwgetword = chwordpos;
-    else
-	hwgetword = -1;
     chwords[chwordpos++] = hptr - chline + offset;
 }
 
@@ -1567,7 +1552,8 @@ ihwbegin(int offset)
 void
 ihwend(void)
 {
-    if (stophist == 2 || (histactive & HA_INWORD))
+    if (stophist == 2 || (histactive & HA_INWORD) ||
+	(inbufflags & (INP_ALIAS|INP_HIST)) == INP_ALIAS)
 	return;
     if (chwordpos%2 && chline) {
 	/* end of word reached and we've already begun a word */
@@ -1578,13 +1564,6 @@ ihwend(void)
 					    (chwordlen += 32) * 
 					    sizeof(short));
 	    }
-	    if (hwgetword > -1 &&
-		(inbufflags & INP_ALIAS) && !(inbufflags & INP_HIST)) {
-		/* We want to reuse the current word position */
-		chwordpos = hwgetword;
-		/* Start from where previous word ended, if possible */
-		hptr = chline + chwords[chwordpos ? chwordpos - 1 : 0];
-	    }
 	} else {
 	    /* scrub that last word, it doesn't exist */
 	    chwordpos--;
@@ -1608,17 +1587,17 @@ histbackword(void)
 static void
 hwget(char **startptr)
 {
-    int pos = hwgetword > -1 ? hwgetword : chwordpos - 2;
+    int pos = chwordpos - 2;
 
 #ifdef DEBUG
     /* debugging only */
-    if (hwgetword == -1 && !chwordpos) {
+    if (!chwordpos) {
 	/* no words available */
 	DPUTS(1, "BUG: hwget() called with no words");
 	*startptr = "";
 	return;
-    } 
-    else if (hwgetword == -1 && chwordpos%2) {
+    }
+    else if (chwordpos%2) {
 	DPUTS(1, "BUG: hwget() called in middle of word");
 	*startptr = "";
 	return;
@@ -1640,9 +1619,9 @@ hwrep(char *rep)
 
     if (!strcmp(rep, start))
 	return;
-    
+
     hptr = start;
-    chwordpos = (hwgetword > -1) ? hwgetword : chwordpos - 2;
+    chwordpos = chwordpos - 2;
     hwbegin(0);
     qbang = 1;
     while (*rep)
@@ -1670,7 +1649,6 @@ hgetline(void)
     /* reset line */
     hptr = chline;
     chwordpos = 0;
-    hwgetword = -1;
 
     return ret;
 }
diff --git a/Src/zsh.h b/Src/zsh.h
index 9a97263..403e8d3 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2709,7 +2709,6 @@ struct hist_stack {
     short *chwords;
     int chwordlen;
     int chwordpos;
-    int hwgetword;
     int (*hgetc) _((void));
     void (*hungetc) _((int));
     void (*hwaddc) _((int));


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style
  2015-03-25 15:48                 ` PATCH: Removing aliases from history, 2015 style (was " Peter Stephenson
@ 2015-03-25 17:57                   ` Peter Stephenson
  2015-03-25 18:42                     ` Mikael Magnusson
  2015-03-25 18:58                     ` Bart Schaefer
  2015-03-25 19:40                   ` PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working) Bart Schaefer
  1 sibling, 2 replies; 33+ messages in thread
From: Peter Stephenson @ 2015-03-25 17:57 UTC (permalink / raw)
  To: Zsh Hackers' List

On Wed, 25 Mar 2015 15:48:53 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Mon, 23 Mar 2015 21:34:26 +0000
> Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > Ideally, I think we'd handle not adding
> > the alias text to history the way we've been doing in other cases by
> > checking INP_* flags, which is much less hit and miss.  That ought not
> > to be too difficult, but I haven't really got my mind round the
> > consequences yet.
> 
> Hmm... I typed this in more or less as below (with one subsequent fix to
> testing flags for interaction with !-history) and was expecting it to
> fail horribly, and so far it's failed to fail.  Perhaps my expectations
> were low simply because it's been the old way since 2 million BC.
> 
> I'll try it further.

Well, it's obviously not *that* broken.

I'll push it so that Mikael can fall over the 1 in 1,000,000 corner case
again.

pws


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style
  2015-03-25 17:57                   ` PATCH: Removing aliases from history, 2015 style Peter Stephenson
@ 2015-03-25 18:42                     ` Mikael Magnusson
  2015-03-27 10:06                       ` Peter Stephenson
  2015-03-25 18:58                     ` Bart Schaefer
  1 sibling, 1 reply; 33+ messages in thread
From: Mikael Magnusson @ 2015-03-25 18:42 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh Hackers' List

On Wed, Mar 25, 2015 at 6:57 PM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Wed, 25 Mar 2015 15:48:53 +0000
> Peter Stephenson <p.stephenson@samsung.com> wrote:
>> On Mon, 23 Mar 2015 21:34:26 +0000
>> Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
>> > Ideally, I think we'd handle not adding
>> > the alias text to history the way we've been doing in other cases by
>> > checking INP_* flags, which is much less hit and miss.  That ought not
>> > to be too difficult, but I haven't really got my mind round the
>> > consequences yet.
>>
>> Hmm... I typed this in more or less as below (with one subsequent fix to
>> testing flags for interaction with !-history) and was expecting it to
>> fail horribly, and so far it's failed to fail.  Perhaps my expectations
>> were low simply because it's been the old way since 2 million BC.
>>
>> I'll try it further.
>
> Well, it's obviously not *that* broken.
>
> I'll push it so that Mikael can fall over the 1 in 1,000,000 corner case
> again.

FWIW, I don't use !-expansion at all, but who knows, maybe that'll
make it more likely I find the corner case.

-- 
Mikael Magnusson


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style
  2015-03-25 17:57                   ` PATCH: Removing aliases from history, 2015 style Peter Stephenson
  2015-03-25 18:42                     ` Mikael Magnusson
@ 2015-03-25 18:58                     ` Bart Schaefer
  1 sibling, 0 replies; 33+ messages in thread
From: Bart Schaefer @ 2015-03-25 18:58 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mar 25,  5:57pm, Peter Stephenson wrote:
} Subject: Re: PATCH: Removing aliases from history, 2015 style
}
} Well, it's obviously not *that* broken.
} 
} I'll push it so that Mikael can fall over the 1 in 1,000,000 corner case
} again.

Here are some tests:


diff --git a/Test/A02alias.ztst b/Test/A02alias.ztst
index 314ec03..fe7155e 100644
--- a/Test/A02alias.ztst
+++ b/Test/A02alias.ztst
@@ -57,3 +57,27 @@
 >And so is this
 >And this too
 >And aliases are expanded
+
+  $ZTST_testdir/../Src/zsh -fis <<<'
+  PROMPT=""
+  exec 2>&1
+  alias \{=echo
+  { begin
+  {end
+  fc -l -2' 2>/dev/null
+0:Aliasing reserved tokens
+>begin
+>zsh: command not found: echoend
+*>*4*{ begin
+*>*5*{end
+
+  $ZTST_testdir/../Src/zsh -fis <<<'
+  PROMPT=""
+  exec 2>&1
+  alias -g S=\"
+  echo S a string S "
+  fc -l -1' 2>/dev/null
+0:Global aliasing quotes
+> a string S 
+*>*4*echo S a string S "
+# Note there is a trailing space on the "> a string S " line


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working)
  2015-03-25 15:48                 ` PATCH: Removing aliases from history, 2015 style (was " Peter Stephenson
  2015-03-25 17:57                   ` PATCH: Removing aliases from history, 2015 style Peter Stephenson
@ 2015-03-25 19:40                   ` Bart Schaefer
  2015-03-26  3:43                     ` Word breaks around aliased tokens (was Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working)) Bart Schaefer
  2015-03-26  9:41                     ` PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working) Peter Stephenson
  1 sibling, 2 replies; 33+ messages in thread
From: Bart Schaefer @ 2015-03-25 19:40 UTC (permalink / raw)
  To: Zsh Hackers' List

It may be appropriate to point out this tidbit:

torch% alias -g '&&'=foo
torch% echo foo&&bar
foo foobar
torch% fc -l -1     
    2  echo foo&&bar

The point being that a space is introduced before the && expansion because
&& is parsed as a separate word, but no space is added after it.

If this were going to change, I would recommend using something akin to
my patch from workers/34738 to add the second space, rather than attempt
to remove the first one.

-- 
Barton E. Schaefer


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Word breaks around aliased tokens (was Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working))
  2015-03-25 19:40                   ` PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working) Bart Schaefer
@ 2015-03-26  3:43                     ` Bart Schaefer
  2015-03-30 18:04                       ` Daniel Shahaf
  2015-03-30 18:08                       ` Daniel Shahaf
  2015-03-26  9:41                     ` PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working) Peter Stephenson
  1 sibling, 2 replies; 33+ messages in thread
From: Bart Schaefer @ 2015-03-26  3:43 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mar 25, 12:40pm, Bart Schaefer wrote:
} 
} torch% echo foo&&bar
} foo foobar
} 
} If this were going to change, I would recommend using something akin to
} my patch from workers/34738 to add the second space, rather than attempt
} to remove the first one.

That would be as below, which is almost identical to 34738 (the salient
difference being use of INP_ALIAS instead of INP_CONT).

I think this makes sense[*]:

torch% alias -g '&&'=AND 
torch% echo foo&&bar    
foo AND bar

(the pre-patch output is "foo ANDbar") but I will wait for feedback before
pushing.

[*] To the extent I think aliasing these tokens makes sense at all.


diff --git a/Src/lex.c b/Src/lex.c
index 6d0079c..4c1555e 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -1747,6 +1747,16 @@ checkalias(void)
 	if (an && !an->inuse &&
 	    ((an->node.flags & ALIAS_GLOBAL) ||
 	     (incmdpos && tok == STRING) || inalmore)) {
+	    if (!lexstop) {
+		/*
+		 * Tokens that don't require a space after, get one,
+		 * because they are treated as if preceded by one.
+		 */
+		int c = hgetc();
+		hungetc(c);
+		if (!iblank(c))
+		    inpush(" ", INP_ALIAS, 0);
+	    }
 	    inpush(an->text, INP_ALIAS, an);
 	    if (an->text[0] == ' ' && !(an->node.flags & ALIAS_GLOBAL))
 		aliasspaceflag = 1;
diff --git a/Test/A02alias.ztst b/Test/A02alias.ztst
index fe7155e..08163eb 100644
--- a/Test/A02alias.ztst
+++ b/Test/A02alias.ztst
@@ -67,7 +67,7 @@
   fc -l -2' 2>/dev/null
 0:Aliasing reserved tokens
 >begin
->zsh: command not found: echoend
+>end
 *>*4*{ begin
 *>*5*{end
 


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working)
  2015-03-25 19:40                   ` PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working) Bart Schaefer
  2015-03-26  3:43                     ` Word breaks around aliased tokens (was Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working)) Bart Schaefer
@ 2015-03-26  9:41                     ` Peter Stephenson
  2015-03-26 15:22                       ` Bart Schaefer
  1 sibling, 1 reply; 33+ messages in thread
From: Peter Stephenson @ 2015-03-26  9:41 UTC (permalink / raw)
  To: Zsh Hackers' List

On Wed, 25 Mar 2015 12:40:32 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> It may be appropriate to point out this tidbit:
> 
> torch% alias -g '&&'=foo
> torch% echo foo&&bar
> foo foobar

It's somewhat belatedly (sorry) occurred to me that the requirement to
use "-g" with this makes it unlikely there's any use case at all for
aliasing "&&".  The original one was to be able to do

  && stuff

by aliasing (not to "foo", obviously) at the start of an expression, so
it had its normal effect at the end.  You can't do that any more.

pws


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working)
  2015-03-26  9:41                     ` PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working) Peter Stephenson
@ 2015-03-26 15:22                       ` Bart Schaefer
  0 siblings, 0 replies; 33+ messages in thread
From: Bart Schaefer @ 2015-03-26 15:22 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mar 26,  9:41am, Peter Stephenson wrote:
} Subject: Re: PATCH: Removing aliases from history, 2015 style (was capturi
}
} > It may be appropriate to point out this tidbit:
} > 
} > torch% alias -g '&&'=foo
} > torch% echo foo&&bar
} > foo foobar
} 
} The original one was to be able to do
} 
}   && stuff
} 
} by aliasing (not to "foo", obviously) at the start of an expression, so
} it had its normal effect at the end.  You can't do that any more.

Sure you can.  Just not exactly the way the original example was written.

torch% alias -g '&&'='; (( $? == 0 )) && '
torch% && print this is OK
this is OK
torch% && print and so is this && print also this
and so is this
also this
torch% 


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style
  2015-03-25 18:42                     ` Mikael Magnusson
@ 2015-03-27 10:06                       ` Peter Stephenson
  2015-03-27 15:25                         ` Bart Schaefer
  2015-03-29 18:38                         ` Peter Stephenson
  0 siblings, 2 replies; 33+ messages in thread
From: Peter Stephenson @ 2015-03-27 10:06 UTC (permalink / raw)
  To: Zsh Hackers' List

On Wed, 25 Mar 2015 19:42:22 +0100
Mikael Magnusson <mikachu@gmail.com> wrote:
> FWIW, I don't use !-expansion at all, but who knows, maybe that'll
> make it more likely I find the corner case.

You do use HISTLEXWORDS, however, so you might notice the follwoing:
it looks like I've managed to make that work less well when
interrupted.  If I ^C during the history read I get


^CWarning: backing up wrong character.
Warning: backing up wrong character.
Warning: backing up wrong character.
Warning: backing up wrong character.
Warning: backing up wrong character.
Warning: backing up wrong character.
 hist.c:3524: bad wordsplit reading history: ((print foo); print bar)
at: ((print foo); print bar)
word: print foo
 hist.c:3524: bad wordsplit reading history: (( ( print foo); print bar)
at: (( ( print foo); print bar)
word:  ( print foo); print bar
 hist.c:3524: bad wordsplit reading history: ((print foo); print bar\n)
at: ((print foo); print bar\n)
word: print foo


Some of that's obviously specific to what I've got in my history.  It's
definitely tied to HISTLEXWORDS being set; I can't provoke it otherwise.

I've rather lost the will to live with all the history, input and
lexical changes but it probably wants looking at.

pws


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style
  2015-03-27 10:06                       ` Peter Stephenson
@ 2015-03-27 15:25                         ` Bart Schaefer
  2015-03-27 15:41                           ` Peter Stephenson
  2015-03-29 18:38                         ` Peter Stephenson
  1 sibling, 1 reply; 33+ messages in thread
From: Bart Schaefer @ 2015-03-27 15:25 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mar 27, 10:06am, Peter Stephenson wrote:
} Subject: Re: PATCH: Removing aliases from history, 2015 style
}
} it looks like I've managed to make that work less well when
} interrupted.  If I ^C during the history read I get
} 
} ^CWarning: backing up wrong character.

That's probably an unintended side-effect of 34543.  Try this:

diff --git a/Src/lex.c b/Src/lex.c
index 6d0079c..4d8355b 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -513,7 +513,7 @@ cmd_or_math(int cs_type)
     /* else unsuccessful: unget the whole thing */
     hungetc(c);
     lexstop = 0;
-    while (lexbuf.len > oldlen && !errflag) {
+    while (lexbuf.len > oldlen && !(errflag & ERRFLAG_ERROR)) {
 	lexbuf.len--;
 	hungetc(itok(*--lexbuf.ptr) ?
 		ztokens[*lexbuf.ptr - Pound] : *lexbuf.ptr);


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style
  2015-03-27 15:25                         ` Bart Schaefer
@ 2015-03-27 15:41                           ` Peter Stephenson
  2015-03-27 17:17                             ` Bart Schaefer
  0 siblings, 1 reply; 33+ messages in thread
From: Peter Stephenson @ 2015-03-27 15:41 UTC (permalink / raw)
  To: Zsh Hackers' List

On Fri, 27 Mar 2015 08:25:15 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Mar 27, 10:06am, Peter Stephenson wrote:
> } Subject: Re: PATCH: Removing aliases from history, 2015 style
> }
> } it looks like I've managed to make that work less well when
> } interrupted.  If I ^C during the history read I get
> } 
> } ^CWarning: backing up wrong character.
> 
> That's probably an unintended side-effect of 34543.  Try this:
> 
> diff --git a/Src/lex.c b/Src/lex.c
> index 6d0079c..4d8355b 100644
> --- a/Src/lex.c
> +++ b/Src/lex.c
> @@ -513,7 +513,7 @@ cmd_or_math(int cs_type)
>      /* else unsuccessful: unget the whole thing */
>      hungetc(c);
>      lexstop = 0;
> -    while (lexbuf.len > oldlen && !errflag) {
> +    while (lexbuf.len > oldlen && !(errflag & ERRFLAG_ERROR)) {
>  	lexbuf.len--;
>  	hungetc(itok(*--lexbuf.ptr) ?
>  		ztokens[*lexbuf.ptr - Pound] : *lexbuf.ptr);

That doesn't seem to make a major difference --- I can't rule out the
possibility it perturbs the error messages a bit but they look similar
to the naked eye.

pws


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style
  2015-03-27 15:41                           ` Peter Stephenson
@ 2015-03-27 17:17                             ` Bart Schaefer
  2015-03-27 17:47                               ` Peter Stephenson
  0 siblings, 1 reply; 33+ messages in thread
From: Bart Schaefer @ 2015-03-27 17:17 UTC (permalink / raw)
  To: Zsh Hackers' List

On Mar 27,  3:41pm, Peter Stephenson wrote:
} Subject: Re: PATCH: Removing aliases from history, 2015 style
}
} On Fri, 27 Mar 2015 08:25:15 -0700
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > -    while (lexbuf.len > oldlen && !errflag) {
} > +    while (lexbuf.len > oldlen && !(errflag & ERRFLAG_ERROR)) {
} 
} That doesn't seem to make a major difference --- I can't rule out the
} possibility it perturbs the error messages a bit but they look similar
} to the naked eye.

Seems like a reasonable thing to commit anyway?


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style
  2015-03-27 17:17                             ` Bart Schaefer
@ 2015-03-27 17:47                               ` Peter Stephenson
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Stephenson @ 2015-03-27 17:47 UTC (permalink / raw)
  To: Zsh Hackers' List

On Fri, 27 Mar 2015 10:17:01 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Mar 27,  3:41pm, Peter Stephenson wrote:
> } Subject: Re: PATCH: Removing aliases from history, 2015 style
> }
> } On Fri, 27 Mar 2015 08:25:15 -0700
> } Bart Schaefer <schaefer@brasslantern.com> wrote:
> } > -    while (lexbuf.len > oldlen && !errflag) {
> } > +    while (lexbuf.len > oldlen && !(errflag & ERRFLAG_ERROR)) {
> } 
> } That doesn't seem to make a major difference --- I can't rule out the
> } possibility it perturbs the error messages a bit but they look similar
> } to the naked eye.
> 
> Seems like a reasonable thing to commit anyway?

Yes, it looks sensible --- it's basically like an "unwind protect" for
something we need to do on an interrupt.

There may be others like this lurking.

pws


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PATCH: Removing aliases from history, 2015 style
  2015-03-27 10:06                       ` Peter Stephenson
  2015-03-27 15:25                         ` Bart Schaefer
@ 2015-03-29 18:38                         ` Peter Stephenson
  1 sibling, 0 replies; 33+ messages in thread
From: Peter Stephenson @ 2015-03-29 18:38 UTC (permalink / raw)
  To: Zsh Hackers' List

On Fri, 27 Mar 2015 10:06:48 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Wed, 25 Mar 2015 19:42:22 +0100
> Mikael Magnusson <mikachu@gmail.com> wrote:
> > FWIW, I don't use !-expansion at all, but who knows, maybe that'll
> > make it more likely I find the corner case.
> 
> You do use HISTLEXWORDS, however, so you might notice the following:
> it looks like I've managed to make that work less well when
> interrupted.  If I ^C during the history read I get
> 
> 
> ^CWarning: backing up wrong character.
>...
>  hist.c:3524: bad wordsplit reading history: ((print foo); print bar)
>...

Various changes here.  Backing  up the wrong character is because
sometimes we "goto brk" when there's an error; if there is, it looks
benign to return LEXERR at that point.  Note we do this from lots of
places and generally set "peek = LEXERR" but don't worry about fixing up
"c" --- but clearly "c" has become irrelevant.  So I didn't try to muck
around fixing up "c" in the case in question, which I think is a return
from cmd_or_math_sub() where we don't have a useful character.

In history, if there was an error buffering words, don't try to do the
word split.

Those two seem to fix the specific problems above.  In both cases as we
were in an inner loop and depended on the previous processing being
successful it seemed reasonable to be sensitive to both bits of
errflag.

The other two are in the history reading code.  They make it more
sensitive to interruption by stopping loops over words and lines in that
case.  Here it's specific to the interruption rather than any error.

diff --git a/Src/hist.c b/Src/hist.c
index 990e609..185d0a0 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -2604,6 +2604,8 @@ readhistfile(char *fn, int err, int readflags)
 	     */
 	    if (uselex || remeta)
 		freeheap();
+	    if (errflag & ERRFLAG_INT)
+		break;
 	}
 	if (start && readflags & HFILE_USE_OPTIONS) {
 	    zsfree(lasthist.text);
@@ -3331,7 +3333,7 @@ bufferwords(LinkList list, char *buf, int *index, int flags)
 	    got = 1;
 	    cur = num - 1;
 	}
-    } while (tok != ENDINPUT && tok != LEXERR);
+    } while (tok != ENDINPUT && tok != LEXERR && !(errflag & ERRFLAG_INT));
     if (buf && tok == LEXERR && tokstr && *tokstr) {
 	int plen;
 	untokenize((p = dupstring(tokstr)));
@@ -3408,6 +3410,8 @@ histsplitwords(char *lineptr, short **wordsp, int *nwordsp, int *nwordposp,
 
 	wordlist = bufferwords(NULL, lineptr, NULL,
 			       LEXFLAGS_COMMENTS_KEEP);
+	if (errflag)
+	    return;
 	nwords_max = 2 * countlinknodes(wordlist);
 	if (nwords_max > nwords) {
 	    *nwordsp = nwords = nwords_max;
diff --git a/Src/lex.c b/Src/lex.c
index 5fed2be..184a54b 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -1345,6 +1345,8 @@ gettokstr(int c, int sub)
 	    break;
     }
   brk:
+    if (errflag)
+	return LEXERR;
     hungetc(c);
     if (unmatched)
 	zerr("unmatched %c", unmatched);


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Word breaks around aliased tokens (was Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working))
  2015-03-26  3:43                     ` Word breaks around aliased tokens (was Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working)) Bart Schaefer
@ 2015-03-30 18:04                       ` Daniel Shahaf
  2015-03-30 20:05                         ` Mikael Magnusson
  2015-03-30 18:08                       ` Daniel Shahaf
  1 sibling, 1 reply; 33+ messages in thread
From: Daniel Shahaf @ 2015-03-30 18:04 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh Hackers' List

Bart Schaefer wrote on Wed, Mar 25, 2015 at 20:43:05 -0700:
> On Mar 25, 12:40pm, Bart Schaefer wrote:
> } 
> } torch% echo foo&&bar
> } foo foobar
> } 
> } If this were going to change, I would recommend using something akin to
> } my patch from workers/34738 to add the second space, rather than attempt
> } to remove the first one.
> 
> That would be as below, which is almost identical to 34738 (the salient
> difference being use of INP_ALIAS instead of INP_CONT).
> 
> I think this makes sense[*]:
> 
> torch% alias -g '&&'=AND 
> torch% echo foo&&bar    
> foo AND bar
> 
> (the pre-patch output is "foo ANDbar") but I will wait for feedback before
> pushing.
> 

Agreed: "foo AND bar" makes more sense than either "foo ANDbar" or
"fooAND bar".

I'm not sure whether the following option is defensible:

% echo foo&&bar
fooANDbar
% echo foo && bar
foo AND bar

> [*] To the extent I think aliasing these tokens makes sense at all.

I'm not sure it's a good idea either.  The only use-case I recall seeing
is the one of using '% && baz' interactively, and I don't find it
compelling, since there are numerous other ways to approach that problem
without controversial C code changes.

After all, no other language allows changing the syntax at runtime.¹

Cheers,

Daniel

¹ Well, TeX does, but I'm not aware of anyone using it this way.


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Word breaks around aliased tokens (was Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working))
  2015-03-26  3:43                     ` Word breaks around aliased tokens (was Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working)) Bart Schaefer
  2015-03-30 18:04                       ` Daniel Shahaf
@ 2015-03-30 18:08                       ` Daniel Shahaf
  1 sibling, 0 replies; 33+ messages in thread
From: Daniel Shahaf @ 2015-03-30 18:08 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh Hackers' List

Bart Schaefer wrote on Wed, Mar 25, 2015 at 20:43:05 -0700:
> torch% alias -g '&&'=AND 
> torch% echo foo&&bar    

I suppose there's a project idea here: implement lisp macros for zsh.

That is, have some hook (maybe preexec or precmd?) get as argument
a list of tokens, and have it return an alternative list of tokens to be
executed further.

Then the && idea could be implemented as:

prefoo() {
  if (( $#reply )) && [[ $reply[1] == '&&' ]]; then
    reply=( ';' '()' '{' 'return' '$?' '}' $reply )
  fi
}

Daniel


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Word breaks around aliased tokens (was Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working))
  2015-03-30 18:04                       ` Daniel Shahaf
@ 2015-03-30 20:05                         ` Mikael Magnusson
  0 siblings, 0 replies; 33+ messages in thread
From: Mikael Magnusson @ 2015-03-30 20:05 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh Hackers' List

On Mon, Mar 30, 2015 at 8:04 PM, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> After all, no other language allows changing the syntax at runtime.

http://www.csse.monash.edu.au/~damian/papers/HTML/Perligata.html
(I don't know perl well enough to say if this qualifies as "at
runtime", but it's still funny).

-- 
Mikael Magnusson


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Testing interactive features (Re: capturing output of !! not working)
  2015-03-24 16:09                       ` Bart Schaefer
@ 2016-05-23 14:53                         ` Mikael Magnusson
  0 siblings, 0 replies; 33+ messages in thread
From: Mikael Magnusson @ 2016-05-23 14:53 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh workers

On Tue, Mar 24, 2015 at 5:09 PM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Mar 23,  9:45pm, Bart Schaefer wrote:
> } Subject: Re: Testing interactive features (Re: capturing output of !! not
> }
> } } ... a patch to comptest for the Test/X01* failure with no TTY.
> } }
> } } -    (( first++ )) && read -t 0.011 -k 1 < /dev/null
> } } +    (( first++ )) && read -t 0.011 -u 0 -k 1 < /dev/null
> }
> } Well, that shuts up the error message, but it also causes the 0.011
> } second timeout to be ignored.  The test still succeeds for me, but
> } should get a more thorough examination.
>
> If the real point is to sleep for 0.11 seconds, input needs to be
> redirected from somewhere that doesn't instantly return EOF.  That
> means tty or an open pipe.  Can anyone think of a less silly-looking
> way to do that, than the below?
>
> (The -u 0 -k 1 are probably redundant now.)
>
> First hunk is for a thinko in my %prep.
>
> diff --git a/Test/W01history.ztst b/Test/W01history.ztst
> index 2492c41..3a64530 100644
> --- a/Test/W01history.ztst
> +++ b/Test/W01history.ztst
> @@ -2,7 +2,7 @@
>
>  %prep
>
> -  [[ -t 0 ]] && print -u $ZTST_fd History tests write to /dev/tty
> +  if [[ -t 0 ]]; then print -u $ZTST_fd History tests write to /dev/tty; fi
>
>  %test
>
> diff --git a/Test/comptest b/Test/comptest
> index ef84217..20a3a5b 100644
> --- a/Test/comptest
> +++ b/Test/comptest
> @@ -164,7 +164,7 @@ zletest () {
>    for input; do
>      # zpty_flush Before zletest
>      # sleep for $KEYTIMEOUT
> -    (( first++ )) && read -t 0.011 -u 0 -k 1 < /dev/null
> +    (( first++ )) && { sleep 2 & } | read -t 0.011 -u 0 -k 1
>      zpty -n -w zsh "$input"
>    done
>    zpty -n -w zsh $'\C-X'

This patch causes the test suite to fail on a shell account I have
that has ulimit -l 50 in effect. X02 does about 90 tests, each of them
starts that sleep which stays around forever, and then real tests
can't fork. I don't know if we care about this situation or not, but
just thought I'd let you know. If there's an easy way to kill that
sleep synchronously on the next line, that'd be nice...

I was trying to do just that with something like
    { { (( first++ )) && { sleep 2 & echo $! } | read -t 0.011 -u 0 -k
1 } 9>&1 } | read
    (( REPLY )) && kill $REPLY
but the kill was failing every time, then I noticed this also seems to fix it,
    { (( first++ )) && { sleep 2 & } | read -t 0.011 -u 0 -k 1 } | :
and I am not sure why. It doesn't introduce any delays, so it's not
waiting for the sleep to finish; is it killing them automatically
because it's in a subshell? That's nice if so, but seems unlikely.

-- 
Mikael Magnusson


^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2016-05-23 14:53 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-19  1:49 capturing output of !! not working Vin Shelton
2015-03-19  2:28 ` Bart Schaefer
2015-03-19 10:57 ` Peter Stephenson
2015-03-19 12:27   ` Vin Shelton
2015-03-19 12:53     ` Peter Stephenson
2015-03-20 10:57       ` Peter Stephenson
2015-03-20 16:04         ` Bart Schaefer
2015-03-22 18:35           ` Peter Stephenson
2015-03-22 19:18             ` Peter Stephenson
2015-03-22 23:22             ` Bart Schaefer
2015-03-23 21:34               ` Peter Stephenson
2015-03-24  0:54                 ` Testing interactive features (Re: capturing output of !! not working) Bart Schaefer
2015-03-24  4:12                   ` Bart Schaefer
2015-03-24  4:45                     ` Bart Schaefer
2015-03-24 16:09                       ` Bart Schaefer
2016-05-23 14:53                         ` Mikael Magnusson
2015-03-25 15:48                 ` PATCH: Removing aliases from history, 2015 style (was " Peter Stephenson
2015-03-25 17:57                   ` PATCH: Removing aliases from history, 2015 style Peter Stephenson
2015-03-25 18:42                     ` Mikael Magnusson
2015-03-27 10:06                       ` Peter Stephenson
2015-03-27 15:25                         ` Bart Schaefer
2015-03-27 15:41                           ` Peter Stephenson
2015-03-27 17:17                             ` Bart Schaefer
2015-03-27 17:47                               ` Peter Stephenson
2015-03-29 18:38                         ` Peter Stephenson
2015-03-25 18:58                     ` Bart Schaefer
2015-03-25 19:40                   ` PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working) Bart Schaefer
2015-03-26  3:43                     ` Word breaks around aliased tokens (was Re: PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working)) Bart Schaefer
2015-03-30 18:04                       ` Daniel Shahaf
2015-03-30 20:05                         ` Mikael Magnusson
2015-03-30 18:08                       ` Daniel Shahaf
2015-03-26  9:41                     ` PATCH: Removing aliases from history, 2015 style (was capturing output of !! not working) Peter Stephenson
2015-03-26 15:22                       ` Bart Schaefer

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