zsh-workers
 help / color / mirror / code / Atom feed
* _expand_alias does not expand aliases that contain an "!"
@ 2014-08-26 16:50 Jonathan H
  2014-08-30 20:32 ` Bart Schaefer
  0 siblings, 1 reply; 14+ messages in thread
From: Jonathan H @ 2014-08-26 16:50 UTC (permalink / raw)
  To: zsh-workers

Hello everyone!

Aliases that contain an exclamation point are not correctly expanded
by _expand_alias. For example:

    [pythonnut ~.z <2>]% zsh -f
    Heptagon% autoload -Uz compinit && compinit
    Heptagon% bindkey "^X^E" _expand_alias
    Heptagon% alias 'gc!'="git commit --amend"
    Heptagon% alias ls="echo"
    Heptagon%

ls will expand on ^X^E to "echo " but gc! won't.

Thanks,
PythonNut


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-08-26 16:50 _expand_alias does not expand aliases that contain an "!" Jonathan H
@ 2014-08-30 20:32 ` Bart Schaefer
  2014-08-31  2:58   ` Jonathan H
  0 siblings, 1 reply; 14+ messages in thread
From: Bart Schaefer @ 2014-08-30 20:32 UTC (permalink / raw)
  To: Jonathan H, zsh-workers

On Aug 26,  9:50am, Jonathan H wrote:
}
} Aliases that contain an exclamation point are not correctly expanded
} by _expand_alias.

You've encountered a fairly generic problem with the completion system,
to wit, special characters like '!' must be quoted in some contexts,
but must not be in other contexts.  You'd find the same issue with an
alias containing an asterisk, question mark, etc.

The following makes this work more of the time in the specific case of
alias expansion, but introduces what might be described as the inverse
problem.  That is, after the patch below,

% gc\!

will expand with _expand_alias, because the backslash is incorrectly
removed prior to looking up the alias.  There's no compstate value or
the like to indicate that the original string on the line contained a
backslash that shouldn't be removed.  (Examining $words[CURRENT] may
be a way around this, I haven't worked that out.)

Incidentally:  Running "compinit" binds ^Xa to _expand_alias for you.


diff --git a/Completion/Base/Completer/_expand_alias b/Completion/Base/Completer/_expand_alias
index 8240e41..9064ce8 100644
--- a/Completion/Base/Completer/_expand_alias
+++ b/Completion/Base/Completer/_expand_alias
@@ -25,6 +25,8 @@ else
   pre=(_main_complete - aliases)
 fi
 
+[[ "$compstate[quoting]" = (single|double) ]] || word="${(Q)word}"
+
 zstyle -s ":completion:${curcontext}:" regular tmp || tmp=yes
 case $tmp in
 always) sel=r;;


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-08-30 20:32 ` Bart Schaefer
@ 2014-08-31  2:58   ` Jonathan H
  2014-08-31 20:50     ` Bart Schaefer
  0 siblings, 1 reply; 14+ messages in thread
From: Jonathan H @ 2014-08-31  2:58 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers

Thank you!

Oddly enough, for me _expand_alias works for all characters in
[*#?@:+-^%$.,]. the only character that didn't work and wasn't already
meaningful to the shell was the exclamation point.

I'm on Zsh 5.02, Ubuntu 14.04 x64 and Fedora Rawhide x64. One possible
solution on my end is to define the aliases like this:

Heptagon% alias 'gc!'="git commit --amend"
Heptagon% alias 'gc\!'="git commit --amend"

One for expansion, and one for the shell internally. I'm not sure how
this works on the insides, but it does. Is this unexpected or, worse,
incorrect?

Thanks,
PythonNut

On Sat, Aug 30, 2014 at 1:32 PM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 26,  9:50am, Jonathan H wrote:
> }
> } Aliases that contain an exclamation point are not correctly expanded
> } by _expand_alias.
>
> You've encountered a fairly generic problem with the completion system,
> to wit, special characters like '!' must be quoted in some contexts,
> but must not be in other contexts.  You'd find the same issue with an
> alias containing an asterisk, question mark, etc.
>
> The following makes this work more of the time in the specific case of
> alias expansion, but introduces what might be described as the inverse
> problem.  That is, after the patch below,
>
> % gc\!
>
> will expand with _expand_alias, because the backslash is incorrectly
> removed prior to looking up the alias.  There's no compstate value or
> the like to indicate that the original string on the line contained a
> backslash that shouldn't be removed.  (Examining $words[CURRENT] may
> be a way around this, I haven't worked that out.)
>
> Incidentally:  Running "compinit" binds ^Xa to _expand_alias for you.
>
>
> diff --git a/Completion/Base/Completer/_expand_alias b/Completion/Base/Completer/_expand_alias
> index 8240e41..9064ce8 100644
> --- a/Completion/Base/Completer/_expand_alias
> +++ b/Completion/Base/Completer/_expand_alias
> @@ -25,6 +25,8 @@ else
>    pre=(_main_complete - aliases)
>  fi
>
> +[[ "$compstate[quoting]" = (single|double) ]] || word="${(Q)word}"
> +
>  zstyle -s ":completion:${curcontext}:" regular tmp || tmp=yes
>  case $tmp in
>  always) sel=r;;


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-08-31  2:58   ` Jonathan H
@ 2014-08-31 20:50     ` Bart Schaefer
  2014-09-14 18:30       ` Bart Schaefer
  0 siblings, 1 reply; 14+ messages in thread
From: Bart Schaefer @ 2014-08-31 20:50 UTC (permalink / raw)
  To: Jonathan H; +Cc: zsh-workers

On Aug 30,  7:58pm, Jonathan H wrote:
}
} Oddly enough, for me _expand_alias works for all characters in
} [*#?@:+-^%$.,]. the only character that didn't work and wasn't already
} meaningful to the shell was the exclamation point.

Hmm.  Indeed, something is treating "!" as additionally special.  The
interesting part is that it does not always treat the first character of
the $HISTCHARS variable as special, nor does it treat ! as special when
it is not the first character of $HISTCHARS.

Or (more likely) internally somewhere is explicitly un-quoting file
expansion characters but is not doing so for history characters.  This
could probably be fixed but might take a while to trace down.

} I'm on Zsh 5.02, Ubuntu 14.04 x64 and Fedora Rawhide x64. One possible
} solution on my end is to define the aliases like this:
} 
} Heptagon% alias 'gc!'="git commit --amend"
} Heptagon% alias 'gc\!'="git commit --amend"

Sure, that's fine as long as you never want to be able to type gc\! to
prevent the first alias from expanding.  Or the patch I sent should
apply to _expand_alias in 5.0.2.


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-08-31 20:50     ` Bart Schaefer
@ 2014-09-14 18:30       ` Bart Schaefer
  2014-09-14 20:55         ` Bart Schaefer
  2014-09-23  5:59         ` Jonathan H
  0 siblings, 2 replies; 14+ messages in thread
From: Bart Schaefer @ 2014-09-14 18:30 UTC (permalink / raw)
  To: zsh-workers

On Aug 31,  1:50pm, Bart Schaefer wrote:
} Subject: Re: _expand_alias does not expand aliases that contain an "!"
}
} On Aug 30,  7:58pm, Jonathan H wrote:
} }
} } Oddly enough, for me _expand_alias works for all characters in
} } [*#?@:+-^%$.,]. the only character that didn't work and wasn't already
} } meaningful to the shell was the exclamation point.
} 
} Hmm.  Indeed, something is treating "!" as additionally special.  The
} interesting part is that it does not always treat the first character of
} the $HISTCHARS variable as special, nor does it treat ! as special when
} it is not the first character of $HISTCHARS.
} 
} Or (more likely) internally somewhere is explicitly un-quoting file
} expansion characters but is not doing so for history characters.  This
} could probably be fixed but might take a while to trace down.

What this boils down to is that get_comp_string() calls the lexer to
parse the word to be completed.  Metacharacters like '*' and '?' get
tokenized by the lexer, but the history character does not.

Eventually callcompfunc() invokes multiquote() on the lexed word, which
calls quotestring(), which does not re-quote the tokenized characters
but does quote the history characters.  (compcore.c:706)

callcompfunc() then calls untokenize() and we end up with backslashes
in front of history characters but not in front of pattern characters.

I think that means the fix is as easy as the patch below; this doesn't
seem to break history expansion, and I can't think of any other reason
we'd want history enabled when performing word completion.

The first hunk of the patch backs out the hack in 33069.

There's a remaining, probably autoloading-related, weirdness, which is
that _expand_alias doesn't work the first time you try it unless the
completion system has already been invoked some other way (including
by having tried _expand_alias at least once already).  Tracing with 
_complete_debug shows that the first time through, a backslash is
being inserted before '!' even though NO_banghist is in effect; but
on the second and later attempts, there's no backslash.  Puzzling.


diff --git a/Completion/Base/Completer/_expand_alias b/Completion/Base/Completer/_expand_alias
index 9064ce8..8240e41 100644
--- a/Completion/Base/Completer/_expand_alias
+++ b/Completion/Base/Completer/_expand_alias
@@ -25,8 +25,6 @@ else
   pre=(_main_complete - aliases)
 fi
 
-[[ "$compstate[quoting]" = (single|double) ]] || word="${(Q)word}"
-
 zstyle -s ":completion:${curcontext}:" regular tmp || tmp=yes
 case $tmp in
 always) sel=r;;
diff --git a/Completion/compinit b/Completion/compinit
index f9d2c57..dc84637 100644
--- a/Completion/compinit
+++ b/Completion/compinit
@@ -138,6 +138,7 @@ _comp_options=(
        unset
     NO_allexport
     NO_aliases
+    NO_banghist
     NO_cshnullglob
     NO_cshjunkiequotes
     NO_errexit


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-09-14 18:30       ` Bart Schaefer
@ 2014-09-14 20:55         ` Bart Schaefer
  2014-10-01 14:03           ` Peter Stephenson
  2014-09-23  5:59         ` Jonathan H
  1 sibling, 1 reply; 14+ messages in thread
From: Bart Schaefer @ 2014-09-14 20:55 UTC (permalink / raw)
  To: zsh-workers

Please ignore the patch in 33169, it's insufficient in light of the below.

On Sep 14, 11:30am, Bart Schaefer wrote:
}
} Eventually callcompfunc() invokes multiquote() on the lexed word, which
} calls quotestring(), which does not re-quote the tokenized characters
} but does quote the history characters.  (compcore.c:706)
} 
} I think that means the fix is [adding NO_banghist to _comp_options].
} 
} There's a remaining, probably autoloading-related, weirdness, which is
} that _expand_alias doesn't work the first time you try it unless the
} completion system has already been invoked some other way (including
} by having tried _expand_alias at least once already).  Tracing with 
} _complete_debug shows that the first time through, a backslash is
} being inserted before '!' even though NO_banghist is in effect; but
} on the second and later attempts, there's no backslash.  Puzzling.

This really had me confused.  Adding NO_banghist to _comp_options
*ought* to be "too late" to affect the behavior of callcompfunc(), and
yet somehow it does so -- but only on the second and subsequent
completion attempts.

So I put watchpoints on opts[17] (BANGHIST) and typtab[33] ('!') and
went through the completion twice.

On the first pass:

Hardware watchpoint 1: typtab[33 '!']

Old value = 3072
New value = 0
inittyptab () at ../../zsh-5.0/Src/utils.c:3443
3443        for (t0 = 0; t0 != 256; t0++)
Hardware watchpoint 1: typtab[33 '!']

Old value = 0
New value = 1024
inittyptab () at ../../zsh-5.0/Src/utils.c:3494

It then flips back and forth between 0 and 1024 several times, but never
gets back to the value 3072.  On the second pass it starts out at 1024,
and so quotestring() treats it as ordinary.

What's happening is that the ISPECIAL bit is never restored in the event
that BANGHIST is toggled off and back on again "in the background"; it
is only set if BANGHIST is toggled interactively, e.g., from an init
file or the command line.

Src/utils.c:
   3519     if (isset(BANGHIST) && bangchar && interact && isset(SHINSTDIN))
   3520         typtab[bangchar] |= ISPECIAL;

There's something wrong with that test.  Either we don't need the ISPECIAL
bit on the first character of histchars -- history still seems to work as
long as the BANGHIST option is set, even if '!' is not marked special, but
there may be cases I didn't try -- or we need to fix the conditions in
which ISPECIAL-ness is restored.

And then in turn, I think, have the completion internals disable BANGHIST
during lexing, unless somebody has a better suggestion.


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-09-14 18:30       ` Bart Schaefer
  2014-09-14 20:55         ` Bart Schaefer
@ 2014-09-23  5:59         ` Jonathan H
  1 sibling, 0 replies; 14+ messages in thread
From: Jonathan H @ 2014-09-23  5:59 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers

Thank you!

Unfortunately, I'm not much use when it comes to the zsh core. But it
sounds like you've already done some fixes. Thank you for your time.
(If anything else change I'd be eager to know)

~PythonNut

On Sun, Sep 14, 2014 at 11:30 AM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 31,  1:50pm, Bart Schaefer wrote:
> } Subject: Re: _expand_alias does not expand aliases that contain an "!"
> }
> } On Aug 30,  7:58pm, Jonathan H wrote:
> } }
> } } Oddly enough, for me _expand_alias works for all characters in
> } } [*#?@:+-^%$.,]. the only character that didn't work and wasn't already
> } } meaningful to the shell was the exclamation point.
> }
> } Hmm.  Indeed, something is treating "!" as additionally special.  The
> } interesting part is that it does not always treat the first character of
> } the $HISTCHARS variable as special, nor does it treat ! as special when
> } it is not the first character of $HISTCHARS.
> }
> } Or (more likely) internally somewhere is explicitly un-quoting file
> } expansion characters but is not doing so for history characters.  This
> } could probably be fixed but might take a while to trace down.
>
> What this boils down to is that get_comp_string() calls the lexer to
> parse the word to be completed.  Metacharacters like '*' and '?' get
> tokenized by the lexer, but the history character does not.
>
> Eventually callcompfunc() invokes multiquote() on the lexed word, which
> calls quotestring(), which does not re-quote the tokenized characters
> but does quote the history characters.  (compcore.c:706)
>
> callcompfunc() then calls untokenize() and we end up with backslashes
> in front of history characters but not in front of pattern characters.
>
> I think that means the fix is as easy as the patch below; this doesn't
> seem to break history expansion, and I can't think of any other reason
> we'd want history enabled when performing word completion.
>
> The first hunk of the patch backs out the hack in 33069.
>
> There's a remaining, probably autoloading-related, weirdness, which is
> that _expand_alias doesn't work the first time you try it unless the
> completion system has already been invoked some other way (including
> by having tried _expand_alias at least once already).  Tracing with
> _complete_debug shows that the first time through, a backslash is
> being inserted before '!' even though NO_banghist is in effect; but
> on the second and later attempts, there's no backslash.  Puzzling.
>
>
> diff --git a/Completion/Base/Completer/_expand_alias b/Completion/Base/Completer/_expand_alias
> index 9064ce8..8240e41 100644
> --- a/Completion/Base/Completer/_expand_alias
> +++ b/Completion/Base/Completer/_expand_alias
> @@ -25,8 +25,6 @@ else
>    pre=(_main_complete - aliases)
>  fi
>
> -[[ "$compstate[quoting]" = (single|double) ]] || word="${(Q)word}"
> -
>  zstyle -s ":completion:${curcontext}:" regular tmp || tmp=yes
>  case $tmp in
>  always) sel=r;;
> diff --git a/Completion/compinit b/Completion/compinit
> index f9d2c57..dc84637 100644
> --- a/Completion/compinit
> +++ b/Completion/compinit
> @@ -138,6 +138,7 @@ _comp_options=(
>         unset
>      NO_allexport
>      NO_aliases
> +    NO_banghist
>      NO_cshnullglob
>      NO_cshjunkiequotes
>      NO_errexit


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-09-14 20:55         ` Bart Schaefer
@ 2014-10-01 14:03           ` Peter Stephenson
  2014-10-01 14:06             ` Peter Stephenson
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2014-10-01 14:03 UTC (permalink / raw)
  To: zsh-workers

On Sun, 14 Sep 2014 13:55:52 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> What's happening is that the ISPECIAL bit is never restored in the event
> that BANGHIST is toggled off and back on again "in the background"; it
> is only set if BANGHIST is toggled interactively, e.g., from an init
> file or the command line.
> 
> Src/utils.c:
>    3519     if (isset(BANGHIST) && bangchar && interact && isset(SHINSTDIN))
>    3520         typtab[bangchar] |= ISPECIAL;
> 
> There's something wrong with that test.  Either we don't need the ISPECIAL
> bit on the first character of histchars -- history still seems to work as
> long as the BANGHIST option is set, even if '!' is not marked special, but
> there may be cases I didn't try -- or we need to fix the conditions in
> which ISPECIAL-ness is restored.

I haven't thought of a crushingly sane and rational course of action
here [so no change there, then].

I've a vague feeling the specialness is only applicable to tests for
whether the ! is special "in some general sense", e.g. needs quoting
when it gets inserted onto the command line, and that bangchar itself is
always used directly in the history code (which is what your results
suggest).  So if we can do that test other ways we wouldn't need this.
But I'm a bit worried there's some non-completion quoting code that
might need this, e.g. quotestring() with QT_BACKSLASH.  In the case of !
there's perhaps no great problem in quoting it anyway, but it might be
different if the character got changed (another feature I hate).

On possible fix would be to add a separate variable to remember the
original ISPECIAL bit for bangchar (which would be transferable if
bangchar became a different character) and restore it at this point if
this isn't the original setting up of typtab --- which is where the last
two tests really make sense.  So only the first two tests would remain
here in all cases.  That may seem a bit baroque given no other character
needs this, but it makes the code relatively insensitive to context.

pws


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-10-01 14:03           ` Peter Stephenson
@ 2014-10-01 14:06             ` Peter Stephenson
  2014-10-01 14:15               ` Peter Stephenson
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2014-10-01 14:06 UTC (permalink / raw)
  To: zsh-workers

On Wed, 1 Oct 2014 15:03:24 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On possible fix would be to add a separate variable to remember the
> original ISPECIAL bit for bangchar (which would be transferable if
> bangchar became a different character) and restore it at this point if
> this isn't the original setting up of typtab --- which is where the last
> two tests really make sense.  So only the first two tests would remain
> here in all cases.  That may seem a bit baroque given no other character
> needs this, but it makes the code relatively insensitive to context.

Immediately after hitting "Send"(*), it occurred to me this just needs
to be another bit in the typtab element, ISPECIAL_ORIGINALLY, though we
still need to tell the code at this point whether or not this is the
original case (easy enough).

pws

(*) Which, in claws-mail, has an arrow pointing "up".  I'm not sure
that feels intuitively right.  It's not air mail.  You didn't need
to read this bit.


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-10-01 14:06             ` Peter Stephenson
@ 2014-10-01 14:15               ` Peter Stephenson
  2014-10-01 16:29                 ` Bart Schaefer
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2014-10-01 14:15 UTC (permalink / raw)
  Cc: zsh-workers

On Wed, 01 Oct 2014 15:06:41 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Wed, 1 Oct 2014 15:03:24 +0100
> Peter Stephenson <p.stephenson@samsung.com> wrote:
> > On possible fix would be to add a separate variable to remember the
> > original ISPECIAL bit for bangchar (which would be transferable if
> > bangchar became a different character) and restore it at this point if
> > this isn't the original setting up of typtab --- which is where the last
> > two tests really make sense.  So only the first two tests would remain
> > here in all cases.  That may seem a bit baroque given no other character
> > needs this, but it makes the code relatively insensitive to context.
> 
> Immediately after hitting "Send"(*), it occurred to me this just needs
> to be another bit in the typtab element, ISPECIAL_ORIGINALLY, though we
> still need to tell the code at this point whether or not this is the
> original case (easy enough).

Gag.  Sorry, no, that doesn't work if we change bangchar.  We'd have to
check if ISPECIAL_ORIGINALLY was set for the current bangchar before
changing and then pass that information back in, which far outweighs the
convenience.

pws


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-10-01 14:15               ` Peter Stephenson
@ 2014-10-01 16:29                 ` Bart Schaefer
  2014-10-01 16:39                   ` Peter Stephenson
  0 siblings, 1 reply; 14+ messages in thread
From: Bart Schaefer @ 2014-10-01 16:29 UTC (permalink / raw)
  To: zsh-workers

On Oct 1,  3:15pm, Peter Stephenson wrote:
} Subject: Re: _expand_alias does not expand aliases that contain an "!"
}
} On Wed, 01 Oct 2014 15:06:41 +0100
} Peter Stephenson <p.stephenson@samsung.com> wrote:
} > On Wed, 1 Oct 2014 15:03:24 +0100
} > Peter Stephenson <p.stephenson@samsung.com> wrote:
} > > On possible fix would be to add a separate variable to remember the
} > > original ISPECIAL bit for bangchar (which would be transferable if
} > > bangchar became a different character) and restore it at this point
} > 
} > Immediately after hitting "Send"(*), it occurred to me this just needs
} > to be another bit in the typtab element, ISPECIAL_ORIGINALLY
} 
} Gag.  Sorry, no, that doesn't work if we change bangchar.

Maybe we just need something along the lines of makecommaspecial() that
can be called from hbegin() to set ISPECIAL on bangchar, and called in
the completion code to temporarily switch it off/on.

Then inittyptab() never has to mess with examining bangchar, and the
new makebangspecial() can be called from histcharsetfn() instead of
doing a full inittyptab().

-- 
Barton E. Schaefer


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-10-01 16:29                 ` Bart Schaefer
@ 2014-10-01 16:39                   ` Peter Stephenson
  2014-10-02  1:11                     ` Bart Schaefer
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2014-10-01 16:39 UTC (permalink / raw)
  To: zsh-workers

On Wed, 01 Oct 2014 09:29:34 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Maybe we just need something along the lines of makecommaspecial() that
> can be called from hbegin() to set ISPECIAL on bangchar, and called in
> the completion code to temporarily switch it off/on.
> 
> Then inittyptab() never has to mess with examining bangchar, and the
> new makebangspecial() can be called from histcharsetfn() instead of
> doing a full inittyptab().

That certainly sounds possible, if you know where to put those calls.

Here's the other proposal, done entirely local to inittyptab(), and
untested.

I'm wondering if there might be other places in the shell that could do
with knowing if the shell started in away that allowed direct user
intervention, whatever the current state may be.

pws

diff --git a/Src/utils.c b/Src/utils.c
index 9109f66..2b57d64 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -3437,11 +3437,17 @@ mod_export short int typtab[256];
 void
 inittyptab(void)
 {
+    static int typtab_flags = 0;
     int t0;
     char *s;
 
-    for (t0 = 0; t0 != 256; t0++)
-	typtab[t0] = 0;
+    if (!(typtab_flags & ZTF_INIT)) {
+	typtab_flags = ZTF_INIT;
+	if (interact && isset(SHINSTDIN))
+	    typtab_flags |= ZTF_INTERACT;
+    }
+
+    memset(typtab, 0, sizeof(typtab));
     for (t0 = 0; t0 != 32; t0++)
 	typtab[t0] = typtab[t0 + 128] = ICNTRL;
     typtab[127] = ICNTRL;
@@ -3516,7 +3522,7 @@ inittyptab(void)
 	typtab[STOUC(*s)] |= ISPECIAL;
     if (specialcomma)
 	typtab[STOUC(',')] |= ISPECIAL;
-    if (isset(BANGHIST) && bangchar && interact && isset(SHINSTDIN))
+    if (isset(BANGHIST) && bangchar && (typtab_flags & ZTF_INTERACT))
 	typtab[bangchar] |= ISPECIAL;
 }
 
diff --git a/Src/ztype.h b/Src/ztype.h
index 14f6610..126aafe 100644
--- a/Src/ztype.h
+++ b/Src/ztype.h
@@ -59,6 +59,13 @@
 #define iwsep(X) zistype(X,IWSEP)
 #define inull(X) zistype(X,INULL)
 
+/*
+ * Bit flags for typtab_flags --- preserved after
+ * shell initialisation.
+ */
+#define ZTF_INIT     (0x0001) /* One-off initialisation done */
+#define ZTF_INTERACT (0x0002) /* Shell interative and reading from stdin */
+
 #ifdef MULTIBYTE_SUPPORT
 #define WC_ZISTYPE(X,Y) wcsitype((X),(Y))
 #define WC_ISPRINT(X)	iswprint(X)


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-10-01 16:39                   ` Peter Stephenson
@ 2014-10-02  1:11                     ` Bart Schaefer
  2014-10-02  5:20                       ` Bart Schaefer
  0 siblings, 1 reply; 14+ messages in thread
From: Bart Schaefer @ 2014-10-02  1:11 UTC (permalink / raw)
  To: zsh-workers

On Oct 1,  5:39pm, Peter Stephenson wrote:
} Subject: Re: _expand_alias does not expand aliases that contain an "!"
}
} On Wed, 01 Oct 2014 09:29:34 -0700
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > Maybe we just need something along the lines of makecommaspecial() that
} > can be called from hbegin() to set ISPECIAL on bangchar, and called in
} > the completion code to temporarily switch it off/on.
} > 
} > Then inittyptab() never has to mess with examining bangchar, and the
} > new makebangspecial() can be called from histcharsetfn() instead of
} > doing a full inittyptab().
} 
} That certainly sounds possible, if you know where to put those calls.
} 
} Here's the other proposal, done entirely local to inittyptab(), and
} untested.

This looks fine, though it doesn't solve the problem of needing to turn
off special-ness during completion.

Of course there's a conflict:  If completion does not include expansion,
then bangchar should be quoted if it results from completion.  Otherwise
it will already have expanded and therefore appear only where it does
not need quoting (theoretically).  Of course history expansion could
produce a string containing yet another bangchar, which would then be
subject to history expansion AGAIN upon accept-line.  (I suppose the
fact this has never been mentioned shows how rare it is in practice.)

Let me see if I can produce a patch for this (and back out the changes
to _expand_alias) before you do 5.0.7.


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

* Re: _expand_alias does not expand aliases that contain an "!"
  2014-10-02  1:11                     ` Bart Schaefer
@ 2014-10-02  5:20                       ` Bart Schaefer
  0 siblings, 0 replies; 14+ messages in thread
From: Bart Schaefer @ 2014-10-02  5:20 UTC (permalink / raw)
  To: zsh-workers

On Oct 1,  6:11pm, Bart Schaefer wrote:
} Subject: Re: _expand_alias does not expand aliases that contain an "!"
}
} On Oct 1,  5:39pm, Peter Stephenson wrote:
} } Subject: Re: _expand_alias does not expand aliases that contain an "!"
} }
} } > new makebangspecial() can be called from histcharsetfn() instead of
} } > doing a full inittyptab().
} } 
} } That certainly sounds possible, if you know where to put those calls.
} } 
} } Here's the other proposal, done entirely local to inittyptab(), and
} } untested.
} 
} This looks fine, though it doesn't solve the problem of needing to turn
} off special-ness during completion.
} 
} Let me see if I can produce a patch for this (and back out the changes
} to _expand_alias) before you do 5.0.7.

OK, here's that patch.  This starts with PWS's patch from 33311, merges
the semantics of specialcomma into typtab_flags (which sadly becomes
global to replace specialcomma) and adds makebangspecial(yesno), which
is then used in the smallest sensible scope in compcall().

There may be other places where makebangspecial() should be called, but
I don't think it's correct to blanket docompletion() with it the way
makecommaspecial() is used.  In fact that makecommaspecial() scope may
be too wide leading to other bugs, but I'm not going to mess with that
at this point.

Also add signal safety for inittyptab(), since we're munging globals.

All tests pass and the 'gc!' alias expansion from the original thread
now works as expected.

diff --git a/Completion/Base/Completer/_expand_alias b/Completion/Base/Completer/_expand_alias
index 9064ce8..8240e41 100644
--- a/Completion/Base/Completer/_expand_alias
+++ b/Completion/Base/Completer/_expand_alias
@@ -25,8 +25,6 @@ else
   pre=(_main_complete - aliases)
 fi
 
-[[ "$compstate[quoting]" = (single|double) ]] || word="${(Q)word}"
-
 zstyle -s ":completion:${curcontext}:" regular tmp || tmp=yes
 case $tmp in
 always) sel=r;;
diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index ac7785a..35d410c 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -702,6 +702,7 @@ callcompfunc(char *s, char *fn)
 	}
 	zsfree(compprefix);
 	zsfree(compsuffix);
+	makebangspecial(0);
 	if (unset(COMPLETEINWORD)) {
 	    tmp = (linwhat == IN_MATH ? dupstring(s) : multiquote(s, 0));
 	    untokenize(tmp);
@@ -722,6 +723,7 @@ callcompfunc(char *s, char *fn)
 	    untokenize(ss);
 	    compsuffix = ztrdup(ss);
 	}
+	makebangspecial(1);
         zsfree(complastprefix);
         zsfree(complastsuffix);
         complastprefix = ztrdup(compprefix);
diff --git a/Src/utils.c b/Src/utils.c
index 9109f66..e6eb8e6 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -3424,12 +3424,12 @@ equalsplit(char *s, char **t)
     return 0;
 }
 
-static int specialcomma;
 
 /* the ztypes table */
 
 /**/
 mod_export short int typtab[256];
+static int typtab_flags = 0;
 
 /* initialize the ztypes table */
 
@@ -3440,8 +3440,15 @@ inittyptab(void)
     int t0;
     char *s;
 
-    for (t0 = 0; t0 != 256; t0++)
-	typtab[t0] = 0;
+    if (!(typtab_flags & ZTF_INIT)) {
+	typtab_flags = ZTF_INIT;
+	if (interact && isset(SHINSTDIN))
+	    typtab_flags |= ZTF_INTERACT;
+    }
+
+    queue_signals();
+
+    memset(typtab, 0, sizeof(typtab));
     for (t0 = 0; t0 != 32; t0++)
 	typtab[t0] = typtab[t0 + 128] = ICNTRL;
     typtab[127] = ICNTRL;
@@ -3514,20 +3521,43 @@ inittyptab(void)
 #endif
     for (s = SPECCHARS; *s; s++)
 	typtab[STOUC(*s)] |= ISPECIAL;
-    if (specialcomma)
+    if (typtab_flags & ZTF_SP_COMMA)
 	typtab[STOUC(',')] |= ISPECIAL;
-    if (isset(BANGHIST) && bangchar && interact && isset(SHINSTDIN))
+    if (isset(BANGHIST) && bangchar && (typtab_flags & ZTF_INTERACT)) {
+	typtab_flags |= ZTF_BANGCHAR;
 	typtab[bangchar] |= ISPECIAL;
+    } else
+	typtab_flags &= ~ZTF_BANGCHAR;
+
+    unqueue_signals();
 }
 
 /**/
 mod_export void
 makecommaspecial(int yesno)
 {
-    if ((specialcomma = yesno) != 0)
+    if (yesno != 0) {
+	typtab_flags |= ZTF_SP_COMMA;
 	typtab[STOUC(',')] |= ISPECIAL;
-    else
+    } else {
+	typtab_flags &= ~ZTF_SP_COMMA;
 	typtab[STOUC(',')] &= ~ISPECIAL;
+    }
+}
+
+/**/
+mod_export void
+makebangspecial(int yesno)
+{
+    /* Name and call signature for congruence with makecommaspecial(),
+     * but in this case when yesno is nonzero we defer to the state
+     * saved by inittyptab().
+     */ 
+    if (yesno == 0) {
+	typtab[bangchar] &= ~ISPECIAL;
+    } else if (typtab_flags & ZTF_BANGCHAR) {
+	typtab[bangchar] |= ISPECIAL;
+    }
 }
 
 
diff --git a/Src/ztype.h b/Src/ztype.h
index 14f6610..eef0f23 100644
--- a/Src/ztype.h
+++ b/Src/ztype.h
@@ -59,6 +59,15 @@
 #define iwsep(X) zistype(X,IWSEP)
 #define inull(X) zistype(X,INULL)
 
+/*
+ * Bit flags for typtab_flags --- preserved after
+ * shell initialisation.
+ */
+#define ZTF_INIT     (0x0001) /* One-off initialisation done */
+#define ZTF_INTERACT (0x0002) /* Shell interative and reading from stdin */
+#define ZTF_SP_COMMA (0x0004) /* Treat comma as a special characters */
+#define ZTF_BANGCHAR (0x0008) /* Treat bangchar as a special character */
+
 #ifdef MULTIBYTE_SUPPORT
 #define WC_ZISTYPE(X,Y) wcsitype((X),(Y))
 #define WC_ISPRINT(X)	iswprint(X)

-- 
Barton E. Schaefer


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

end of thread, other threads:[~2014-10-02  5:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-26 16:50 _expand_alias does not expand aliases that contain an "!" Jonathan H
2014-08-30 20:32 ` Bart Schaefer
2014-08-31  2:58   ` Jonathan H
2014-08-31 20:50     ` Bart Schaefer
2014-09-14 18:30       ` Bart Schaefer
2014-09-14 20:55         ` Bart Schaefer
2014-10-01 14:03           ` Peter Stephenson
2014-10-01 14:06             ` Peter Stephenson
2014-10-01 14:15               ` Peter Stephenson
2014-10-01 16:29                 ` Bart Schaefer
2014-10-01 16:39                   ` Peter Stephenson
2014-10-02  1:11                     ` Bart Schaefer
2014-10-02  5:20                       ` Bart Schaefer
2014-09-23  5:59         ` Jonathan H

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