Re: Inconsistent history expansion of characters adjacent to histchar

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Mon, Oct 7, 2013 at 8:55 AM, Hauke Petersen <hkptrsn@xxxxxxxxx> wrote:

> It does not immediately make sense to me why a single `!' would be
> interpreted as a double `!!' before a `;' and tried, unsuccessfully,
> to find an explanation.

Bluntly, it's because that history documentation is lying to you. The
correct bit is the first sentence here:

>     Event Designators
>     !   Start a history expansion, except when followed by a blank, new-line,
>     `=' or `('.  If followed immediately by a word designator, this forms a
>     history reference with no event designator.

Semicolon is not one of the things that prevents a "!" from starting
a history expansion, so everything from there forward behaves like a
history reference with no event.

This bug appears to have been around for longer than we've been keeping
source control copies of the shell. For some reason ';' is called out
in histsubchar() along with things that begin (or end) event or word
designators. The same is true of any kind of quotes, e.g.:

    print !`echo foo`
    print !!`echo foo`

are equivalent; "!}" also has the same problem though it usually causes
a parse error.

I think the intent is that those characters only end a history reference
after at least one other character [not in that set] has been read, but
that's not the way the loop is written, and it doesn't account for other
characters that can't be word designators, as seen here:

> there is no previous command starting with ';'.  Like otherwise
> similar `&':
>
>     % print !& print foo
>     zsh: event not found: &

Same happens with "!|" and probably a few other obscure variations.

In short, the history parser has always been very ad-hoc and deviates
from its own spec in several ways.

Here's a patch that fixes the unintended expansion, but it doesn't fix
the inconsistent behavior with !& !| et al.  There should probably be
more method to this madness instead of multiple series of individual
character comparisons.

The (c == '"') condition is probably unnecessary because '!"' has been
handled much earlier, but the loop above this also tests all three
quotes, so ...  

diff --git a/Src/hist.c b/Src/hist.c
index d1af30a..bd650e8 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -521,6 +521,12 @@ histsubchar(int c)
 		}
 		c = ingetc();
 	    }
+	    if (ptr == buf &&
+		(c == '}' ||  c == ';' || c == '\'' || c == '"' || c == '`')) {
+	      /* Neither event nor word designator, no expansion */
+	      safeinungetc(c);
+	      return bangchar;
+	    }
 	    *ptr = 0;
 	    if (!*buf) {
 		if (c != '%') {