Re: Bug#519535: history expansion: modifier completion missing

Re: Bug#519535: history expansion: modifier completion missing

[Clint Adams]

On Fri, Mar 13, 2009 at 11:55:55AM +0100, martin f krafft wrote:
> I tried to show off zsh to a sceptic today and had to find out the
> hard way that it does not (yet) provide completion for history
> expansion modifiers, e.g.
> 
>   echo !$:<TAB>
> 
> should list
> 
>   h  Remove a trailing pathname component, leaving the head.  This works like ‘dirname’.
>   r  Remove a filename extension of the form ‘.xxx’, leaving the root name.
>   e  Remove all but the extension.
>   t  Remove all leading pathname components, leaving the tail.  This works like  ‘basename’.
>   p  Print the new command but do not execute it.  Only works with history expansion.
>   q  Quote the substituted words, escaping further substitutions.  Works with history expan‐
>     sion and parameter expansion, though for parameters it is only useful if the  resulting
>     text is to be re-evaluated such as by eval.
>   Q  Remove one level of quotes from the substituted words.
>   x  Like q, but break into words at whitespace.  Does not work with parameter expansion.
>   l  Convert the words to all lowercase.
>   u  Convert the words to all uppercase.
> 
> etc.

Anyone want to tackle this?

Re: Bug#519535: history expansion: modifier completion missing

[Peter Stephenson]

On Sun, 15 Mar 2009 06:22:53 +0000
Clint Adams <schizo@debian.org> wrote:
> On Fri, Mar 13, 2009 at 11:55:55AM +0100, martin f krafft wrote:
> > I tried to show off zsh to a sceptic today and had to find out the
> > hard way that it does not (yet) provide completion for history
> > expansion modifiers, e.g.
> > 
> >   echo !$:<TAB>

I thought this was going to be harder than it turned out to be; as you can
see it's really quite simple, particularly since I already wrote modifer
completion and it would appear had the foresight to make it handle the
history case.

It's in _normal which is where we handle normal command arguments.  I think
this is both the first and last place where get to massage command line
arguments generically---we don't just want this in default completion since
history expansion, if active, trumps everything else.

It probably needs to go in _value for assignments; is right at the top the
appropriate place?  Or should it actually go higher, in _complete itself,
outside the "if" that decides whether it's _normal or otherwise?  I don't
think so, because that screws up vared.  Hmmm... I think that's true of
putting it in _value, which _in_vared uses.  Maybe leaving it discretely
alone might work well.

The rum thing is the way $PREFIX acquires quoted !'s.  I thought $PREFIX
was fairly raw, so you could handle special characters?  Obviously I'm
misremembering, because I traced the quoting to some completely general
code for all special characters.  Anyway, that has two effects:  first you
have to check $words, because that does have the raw form, and secondly you
have to update $PREFIX so that you get the raw !'s inserted into the
command line.  (But you don't usually have to do that even if you're
inserting special characters, do you?  You just need to make sure compadd
has the argument -Q, which it does here, except that's not good enough.
Baaaaaaa.)

I think -z $compstate[quote] ensures we're not already doing something
clever inside nested quotes, which would mean the expression couldn't be a
history expansion.  I think.

So it seems to work fine (possibly depending on your options and
completers---there are other things around that will do other things to
!'s), but I'm still a bit confused.

Index: Completion/Base/Core/_normal
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Base/Core/_normal,v
retrieving revision 1.4
diff -u -r1.4 _normal
--- Completion/Base/Core/_normal	13 Mar 2002 09:28:05 -0000	1.4
+++ Completion/Base/Core/_normal	16 Mar 2009 18:05:48 -0000
@@ -9,6 +9,19 @@
   _compskip=''
 fi
 
+# Check for a history reference to complete modifiers.
+# $PREFIX has a quoted form of the !, so we can't test that
+# (it might the start of a real argument), but words has the
+# raw McCoy.
+if [[ -o BANG_HIST && $words[CURRENT] = \!*: && -z $compstate[quote] ]]; then
+  # This looks like a real history expansion; in that case
+  # we'd better put the !'s back the way pfalstad intended.
+  PREFIX=${PREFIX//\\!/!}
+  compset -P '*:'
+  _history_modifiers h
+  return
+fi
+
 # Completing in command position?
 
 if [[ CURRENT -eq 1 ]]; then

Re: Bug#519535: history expansion: modifier completion missing

[Bart Schaefer]

[-bugs.debian.org]

On Mar 16,  6:18pm, Peter Stephenson wrote:
} 
} I thought this was going to be harder than it turned out to be; as you can
} see it's really quite simple, particularly since I already wrote modifer
} completion and it would appear had the foresight to make it handle the
} history case.

Nit-pick:  This doesn't handle the new :a/:A modifiers.

Other nit-pick about :a itself -- get a load of this:

torch% echo foo
torch% !!:a
/usr/local/src/zsh/zsh-build/echo foo
zsh: no such file or directory: /usr/local/src/zsh/zsh-build/echo

That's almost certainly not the intended behavior ... is it?  Replace :a
with :h or :t there and you get "modifier failed" (which seems odd as
well, but ...).

} It's in _normal which is where we handle normal command arguments.
} I think this is both the first and last place where get to massage
} command line arguments generically---we don't just want this in
} default completion since history expansion, if active, trumps
} everything else.

I started out agreeing with this, but now I'm not sure it makes any
difference.  History expansion trumps everything when it works, and
leaves the command line unchanged when it fails.  Notice:

schaefer<511> bindkey | grep expand
"^X*" expand-word
"^XG" list-expand
"^Xa" _expand_alias
"^Xe" _expand_word
"^Xg" list-expand
"^[ " expand-history
"^[!" expand-history
schaefer<512> !!<ESC!>
schaefer<512> bindkey | grep expand<C-u>
schaefer<512> !!:<ESC!>
schaefer<512> !!:

Same happens with <C-x*> or <C-xe> or even <C-xg> and <C-xG>.  If
history can expand, it does, if it can't, it doesn't.  If I try it
with <C-x?> (_complete_debug), I get an expansion with no backtrace
for !!, but a completion with backtrace for !!:.

So I think you can put this pretty much anywhere, as long as it's
somewhere that gets reached before the completion system gives up.
In fact in the unlikely event that one has a file named '!2:u2' one
might even want to be offered both history modifiers and \!2:u2 as
completions after "!2:".  However, this might be complicated by the
quoting business with $PREFIX ...

} It probably needs to go in _value for assignments; is right at the top
} the appropriate place?

Lost me here, sorry.

} The rum thing is the way $PREFIX acquires quoted !'s. I thought
} $PREFIX was fairly raw, so you could handle special characters?
} Obviously I'm misremembering, because I traced the quoting to some
} completely general code for all special characters.

_approximate certainly treats $PREFIX as though things like tildes
and glob qualifiers aren't quoted and don't need it, but ...

} Anyway, that has two effects: first you have to check $words, because
} that does have the raw form, and secondly you have to update $PREFIX
} so that you get the raw !'s inserted into the command line. (But
} you don't usually have to do that even if you're inserting special
} characters, do you? You just need to make sure compadd has the
} argument -Q, which it does here, except that's not good enough.
} Baaaaaaa.)

I don't know how the generic quoting code plays in, but I suspect this
is done this way *because* history expansion takes place so early.  If
it looks like a history expansion and it got this far, then it must be
a *failed* history expansion and needs to be quoted if it matches any
other completion -- otherwise the history expansion will fail *again*
after accept-line, and spoil the whole command.
 
} I think -z $compstate[quote] ensures we're not already doing something
} clever inside nested quotes, which would mean the expression couldn't be a
} history expansion.  I think.

History expansion occurs inside double quotes, though, so if the quote
state is double-quote then ... er, um ... perhaps it's necessary to
run compstate -q and examine $compstate[all_quotes] ...

Re: Bug#519535: history expansion: modifier completion missing

[Peter Stephenson]

On Mon, 16 Mar 2009 19:44:34 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> [-bugs.debian.org]
> 
> On Mar 16,  6:18pm, Peter Stephenson wrote:
> } 
> } I thought this was going to be harder than it turned out to be; as you can
> } see it's really quite simple, particularly since I already wrote modifer
> } completion and it would appear had the foresight to make it handle the
> } history case.
> 
> Nit-pick:  This doesn't handle the new :a/:A modifiers.

I've fixed that.

> Other nit-pick about :a itself -- get a load of this:
> 
> torch% echo foo
> torch% !!:a
> /usr/local/src/zsh/zsh-build/echo foo
> zsh: no such file or directory: /usr/local/src/zsh/zsh-build/echo
> 
> That's almost certainly not the intended behavior ... is it?

It depends what you mean by "intended": Michael did say the operation
didn't take account of whether the file existed (but I didn't document
this), and there was never any special behaviour for command words.  You
could make it work like "=", I suppose, but that's an extension.

> Replace :a
> with :h or :t there and you get "modifier failed" (which seems odd as
> well, but ...).

I think it's always done that, although that doesn't mean it's necessarily
useful.  Logically, it seems to me that for :h to fail is probably useful
(there's no directory part, so you won't get it) but for :t to fail is less
so, since you've already got a perfectly good non-directory part (even it
happens to be a directory, this isn't VMS).  So we could probably change
that without anyone complaining.

> I don't know how the generic quoting code plays in, but I suspect this
> is done this way *because* history expansion takes place so early.  If
> it looks like a history expansion and it got this far, then it must be
> a *failed* history expansion and needs to be quoted if it matches any
> other completion -- otherwise the history expansion will fail *again*
> after accept-line, and spoil the whole command.

That's quite possible.  The quote code is hair-raising enough (see utils.c
around line 4504) that it's quite hard to tell.  It's being called from
multiquote() within callcompfunc() in compcore.c, which is called for all
top-level completion widgets.  The commenting in that last function is
quite amusing in an infuriating sort of way: there are three, and all of
them are in blocks marked with "#if 0".

> } I think -z $compstate[quote] ensures we're not already doing something
> } clever inside nested quotes, which would mean the expression couldn't be a
> } history expansion.  I think.
> 
> History expansion occurs inside double quotes, though, so if the quote
> state is double-quote then ... er, um ... perhaps it's necessary to
> run compstate -q and examine $compstate[all_quotes] ...

Hmm... I deliberately didn't make the test too hairy, so it only completes
at the start of a word, but it looks like it's not too hard to fix the
simplest case of double quoting.

Index: Completion/Base/Core/_normal
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Base/Core/_normal,v
retrieving revision 1.5
diff -u -r1.5 _normal
--- Completion/Base/Core/_normal	16 Mar 2009 18:29:39 -0000	1.5
+++ Completion/Base/Core/_normal	17 Mar 2009 09:42:27 -0000
@@ -13,7 +13,9 @@
 # $PREFIX has a quoted form of the !, so we can't test that
 # (it might the start of a real argument), but words has the
 # raw McCoy.
-if [[ -o BANG_HIST && $words[CURRENT] = \!*: && -z $compstate[quote] ]]; then
+if [[ -o BANG_HIST &&
+     ( ( $words[CURRENT] = \!*: && -z $compstate[quote] ) ||
+       ( $words[CURRENT] = \"\!*: && $compstate[all_quotes] = \" ) ) ]]; then
   # This looks like a real history expansion; in that case
   # we'd better put the !'s back the way pfalstad intended.
   PREFIX=${PREFIX//\\!/!}
Index: Completion/Zsh/Type/_history_modifiers
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Zsh/Type/_history_modifiers,v
retrieving revision 1.2
diff -u -r1.2 _history_modifiers
--- Completion/Zsh/Type/_history_modifiers	23 Feb 2008 18:34:02 -0000	1.2
+++ Completion/Zsh/Type/_history_modifiers	17 Mar 2009 09:42:27 -0000
@@ -64,6 +64,8 @@
       )
     if (( ! global )); then
       list+=(
+	"a:absolute path"
+	"A:absolute path resolving symbolic links"
 	"g:globally apply s or &"
 	"h:head - strip trailing path element"
 	"t:tail - strip directories"



-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: Bug#519535: history expansion: modifier completion missing

[Peter Stephenson]

On Tue, 17 Mar 2009 09:57:17 +0000
Peter Stephenson <pws@csr.com> wrote:
> On Mon, 16 Mar 2009 19:44:34 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > torch% echo foo
> > torch% !!:a
> > /usr/local/src/zsh/zsh-build/echo foo
> > zsh: no such file or directory: /usr/local/src/zsh/zsh-build/echo
> > 
> > That's almost certainly not the intended behavior ... is it?
> 
> It depends what you mean by "intended": Michael did say the operation
> didn't take account of whether the file existed (but I didn't document
> this), and there was never any special behaviour for command words.  You
> could make it work like "=", I suppose, but that's an extension.

I'm not sure if the change below is what you had in mind.  Maybe you were
pointing out that applying :a to the entire expression "echo foo", which is
what happened, didn't make sense, but that's the way modifiers have always
worked:

% /bin/echo hello
% !!:h<TAB>   ->   /bin
% !!:t<TAB>   ->   echo hello

so yes, it would be intended that a and A work the same.

Maybe it's not clear (or is actually confusing) in the change below that
the expansion depends not where the original has come from, but where the
expansion is being used on the command line.

I'm not even sure this change is useful.  Suppose you had:

% script arg

and that didn't work because script was in the current directory rather
than the path, so you ran

% !:0:a !:*

to ensure you ran the script from the current directory.  With the change
below, that wouldn't work any more.

I've a mind to leave a and A as they were, note that they don't care if the
file doesn't exist (which is also entirely consistent with h, t, etc.), and
add c to do the following.  You could then stack :c:A if you really wanted.

Index: Doc/Zsh/expn.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/expn.yo,v
retrieving revision 1.103
diff -u -r1.103 expn.yo
--- Doc/Zsh/expn.yo	15 Mar 2009 01:17:06 -0000	1.103
+++ Doc/Zsh/expn.yo	17 Mar 2009 10:44:41 -0000
@@ -219,6 +219,18 @@
 item(tt(a))(
 Turn a file name into an absolute path:  prepends the current directory,
 if necessary, and resolves any use of `tt(..)' and `tt(.)' in the path.
+If used in history substitution at the start of a command line and the name
+does not contain a tt(/), it is resolved as an external command by
+searching the command path given by the tt(PATH) parameter.  For example,
+assuming the current directory is tt(/home/pwd) and the external command
+tt(echo) exists in tt(/bin), then the following:
+example(echo file
+!:0:a !:1:a
+)
+becomes `tt(/bin/echo /home/pwd/file)', while
+example(./script file
+!:0:a !:1:a)
+becomes `tt(/home/pwd/script /home/pwd/file)'.
 )
 item(tt(A))(
 As `tt(a)', but also resolve use of symbolic links where possible.
Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.90
diff -u -r1.90 hist.c
--- Src/hist.c	15 Mar 2009 01:17:06 -0000	1.90
+++ Src/hist.c	17 Mar 2009 10:44:42 -0000
@@ -624,7 +624,9 @@
 		histdone = HISTFLAG_DONE | HISTFLAG_NOEXEC;
 		break;
 	    case 'a':
-		if (!chabspath(&sline)) {
+		if ((incmdpos && !strchr(sline, '/') &&
+		     !(sline = equalsubstr(sline, 0, 0))) ||
+		    !chabspath(&sline)) {
 		    herrflush();
 		    zerr("modifier failed: a");
 		    return -1;
@@ -632,7 +634,9 @@
 		break;
 
 	    case 'A':
-		if (!chrealpath(&sline)) {
+		if ((incmdpos && !strchr(sline, '/') &&
+		     !(sline = equalsubstr(sline, 0, 0))) ||
+		    !chrealpath(&sline)) {
 		    herrflush();
 		    zerr("modifier failed: A");
 		    return -1;
Index: Src/subst.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/subst.c,v
retrieving revision 1.96
diff -u -r1.96 subst.c
--- Src/subst.c	15 Mar 2009 01:17:06 -0000	1.96
+++ Src/subst.c	17 Mar 2009 10:44:42 -0000
@@ -539,12 +539,43 @@
     }
 }
 
+#define isend(c) ( !(c) || (c)=='/' || (c)==Inpar || (assign && (c)==':') )
+#define isend2(c) ( !(c) || (c)==Inpar || (assign && (c)==':') )
+
+/*
+ * do =foo substitution, or equivalent.
+ * on entry, str should point to the "foo".
+ * if assign, this is in an assignment
+ * if nomatch, report hard error on failure.
+ * if successful, returns the expansion, else NULL.
+ */
+
+/**/
+char *
+equalsubstr(char *str, int assign, int nomatch)
+{
+    char *pp, *cnam, *cmdstr, *ret;
+
+    for (pp = str; !isend2(*pp); pp++)
+	;
+    cmdstr = dupstrpfx(str, pp-str);
+    untokenize(cmdstr);
+    remnulargs(cmdstr);
+    if (!(cnam = findcmd(cmdstr, 1))) {
+	if (nomatch)
+	    zerr("%s not found", cmdstr);
+	return NULL;
+    }
+    ret = dupstring(cnam);
+    if (*pp)
+	ret = dyncat(ret, pp);
+    return ret;
+}
+
 /**/
 mod_export int
 filesubstr(char **namptr, int assign)
 {
-#define isend(c) ( !(c) || (c)=='/' || (c)==Inpar || (assign && (c)==':') )
-#define isend2(c) ( !(c) || (c)==Inpar || (assign && (c)==':') )
     char *str = *namptr;
 
     if (*str == Tilde && str[1] != '=' && str[1] != Equals) {
@@ -606,27 +637,17 @@
 	    return 1;
 	}
     } else if (*str == Equals && isset(EQUALS) && str[1]) {   /* =foo */
-	char *pp, *cnam, *cmdstr, *str1 = str+1;
-
-	for (pp = str1; !isend2(*pp); pp++)
-	    ;
-	cmdstr = dupstrpfx(str1, pp-str1);
-	untokenize(cmdstr);
-	remnulargs(cmdstr);
-	if (!(cnam = findcmd(cmdstr, 1))) {
-	    if (isset(NOMATCH))
-		zerr("%s not found", cmdstr);
-	    return 0;
+	char *expn = equalsubstr(str+1, assign, isset(NOMATCH));
+	if (expn) {
+	    *namptr = expn;
+	    return 1;
 	}
-	*namptr = dupstring(cnam);
-	if (*pp)
-	    *namptr = dyncat(*namptr, pp);
-	return 1;
     }
     return 0;
+}
+
 #undef isend
 #undef isend2
-}
 
 /**/
 static char *


-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: Bug#519535: history expansion: modifier completion missing

[Peter Stephenson]

On Tue, 17 Mar 2009 11:00:03 +0000
Peter Stephenson <pws@csr.com> wrote:
> I've a mind to leave a and A as they were, note that they don't care if the
> file doesn't exist (which is also entirely consistent with h, t, etc.), and
> add c to do the following.  You could then stack :c:A if you really wanted.

I certainly like this better.  You can now do stuff like

% print ${${:-cat}:c:h}
/bin

although since

% print ${${:-=cat}:h}
/bin

also works it's possibly not much of a gain; however, here you don't need to
force an additional level of expansion on parameters; however however,

% foo=cat
% print ${${:-=$foo}:h}
/bin

also works, too.  One other quirk that makes it less useful (which
surprised me enough before I realised it was indeed the way globbing has
always worked that I've mentioned it specially) is that

% print cat(:c)

doesn't work, because the glob qualifiers turn on globbing, which finds
that the file "cat" doesn't exist and rejects it at that point.  Maybe we
need an alternative syntax for this case.

Index: Doc/Zsh/expn.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/expn.yo,v
retrieving revision 1.103
diff -u -r1.103 expn.yo
--- Doc/Zsh/expn.yo	15 Mar 2009 01:17:06 -0000	1.103
+++ Doc/Zsh/expn.yo	17 Mar 2009 12:05:05 -0000
@@ -219,12 +219,21 @@
 item(tt(a))(
 Turn a file name into an absolute path:  prepends the current directory,
 if necessary, and resolves any use of `tt(..)' and `tt(.)' in the path.
+Note that the transformation takes place even if the file or any
+intervening directories do not exist.
 )
 item(tt(A))(
 As `tt(a)', but also resolve use of symbolic links where possible.
 Note that resolution of `tt(..)' occurs em(before) resolution of symbolic
 links.
 )
+item(tt(c))(
+Resolve a command name into an absolute path by searching the command
+path given by the tt(PATH) variable.  This does not work for commands
+containing directory parts.  Note also that this does not usually work as
+a glob qualifier unless a file of the same name is found in the
+current directory.
+)
 item(tt(e))(
 Remove all but the extension.
 )
Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.90
diff -u -r1.90 hist.c
--- Src/hist.c	15 Mar 2009 01:17:06 -0000	1.90
+++ Src/hist.c	17 Mar 2009 12:05:05 -0000
@@ -638,6 +638,13 @@
 		    return -1;
 		}
 		break;
+	    case 'c':
+		if (!(sline = equalsubstr(sline, 0, 0))) {
+		    herrflush();
+		    zerr("modifier failed: c");
+		    return -1;
+		}
+		break;
 	    case 'h':
 		if (!remtpath(&sline)) {
 		    herrflush();
Index: Src/subst.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/subst.c,v
retrieving revision 1.96
diff -u -r1.96 subst.c
--- Src/subst.c	15 Mar 2009 01:17:06 -0000	1.96
+++ Src/subst.c	17 Mar 2009 12:05:05 -0000
@@ -539,12 +539,43 @@
     }
 }
 
+#define isend(c) ( !(c) || (c)=='/' || (c)==Inpar || (assign && (c)==':') )
+#define isend2(c) ( !(c) || (c)==Inpar || (assign && (c)==':') )
+
+/*
+ * do =foo substitution, or equivalent.
+ * on entry, str should point to the "foo".
+ * if assign, this is in an assignment
+ * if nomatch, report hard error on failure.
+ * if successful, returns the expansion, else NULL.
+ */
+
+/**/
+char *
+equalsubstr(char *str, int assign, int nomatch)
+{
+    char *pp, *cnam, *cmdstr, *ret;
+
+    for (pp = str; !isend2(*pp); pp++)
+	;
+    cmdstr = dupstrpfx(str, pp-str);
+    untokenize(cmdstr);
+    remnulargs(cmdstr);
+    if (!(cnam = findcmd(cmdstr, 1))) {
+	if (nomatch)
+	    zerr("%s not found", cmdstr);
+	return NULL;
+    }
+    ret = dupstring(cnam);
+    if (*pp)
+	ret = dyncat(ret, pp);
+    return ret;
+}
+
 /**/
 mod_export int
 filesubstr(char **namptr, int assign)
 {
-#define isend(c) ( !(c) || (c)=='/' || (c)==Inpar || (assign && (c)==':') )
-#define isend2(c) ( !(c) || (c)==Inpar || (assign && (c)==':') )
     char *str = *namptr;
 
     if (*str == Tilde && str[1] != '=' && str[1] != Equals) {
@@ -606,27 +637,17 @@
 	    return 1;
 	}
     } else if (*str == Equals && isset(EQUALS) && str[1]) {   /* =foo */
-	char *pp, *cnam, *cmdstr, *str1 = str+1;
-
-	for (pp = str1; !isend2(*pp); pp++)
-	    ;
-	cmdstr = dupstrpfx(str1, pp-str1);
-	untokenize(cmdstr);
-	remnulargs(cmdstr);
-	if (!(cnam = findcmd(cmdstr, 1))) {
-	    if (isset(NOMATCH))
-		zerr("%s not found", cmdstr);
-	    return 0;
+	char *expn = equalsubstr(str+1, assign, isset(NOMATCH));
+	if (expn) {
+	    *namptr = expn;
+	    return 1;
 	}
-	*namptr = dupstring(cnam);
-	if (*pp)
-	    *namptr = dyncat(*namptr, pp);
-	return 1;
     }
     return 0;
+}
+
 #undef isend
 #undef isend2
-}
 
 /**/
 static char *
@@ -3201,6 +3222,7 @@
 	    switch (**ptr) {
             case 'a':
             case 'A':
+	    case 'c':
 	    case 'h':
 	    case 'r':
 	    case 'e':
@@ -3345,6 +3367,13 @@
 		    case 'A':
 			chrealpath(&copy);
 			break;
+		    case 'c':
+		    {
+			char *copy2 = equalsubstr(copy, 0, 0);
+			if (copy2)
+			    copy = copy2;
+			break;
+		    }
 		    case 'h':
 			remtpath(&copy);
 			break;
@@ -3410,6 +3439,13 @@
 		case 'A':
 		    chrealpath(str);
 		    break;
+		case 'c':
+		{
+		    char *copy2 = equalsubstr(*str, 0, 0);
+		    if (copy2)
+			*str = copy2;
+		    break;
+		}
 		case 'h':
 		    remtpath(str);
 		    break;

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: Bug#519535: history expansion: modifier completion missing

[Bart Schaefer]

On Mar 17,  9:57am, Peter Stephenson wrote:
} Subject: Re: Bug#519535: history expansion: modifier completion missing
}
} On Mon, 16 Mar 2009 19:44:34 -0700
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > I don't know how the generic quoting code plays in, but I suspect this
} > is done this way *because* history expansion takes place so early.  If
} > it looks like a history expansion and it got this far, then it must be
} > a *failed* history expansion and needs to be quoted if it matches any
} > other completion -- otherwise the history expansion will fail *again*
} > after accept-line, and spoil the whole command.
} 
} That's quite possible. The quote code is hair-raising enough (see
} utils.c around line 4504) that it's quite hard to tell.

Yes, I can see where it might be hard to follow exactly how we come
to be passing through there.  And it certainly looks as if at least
tildes as well as histchars should be getting quoted.  Are the other
backslashes perhaps being added here but removed elsewhere?

} It's being called from multiquote() within callcompfunc() in
} compcore.c, which is called for all top-level completion widgets. The
} commenting in that last function is quite amusing in an infuriating
} sort of way: there are three, and all of them are in blocks marked
} with "#if 0".

Apparently Sven only felt it necessary to remind himself why he did NOT
do things.  There must be some discussion of group numbers lurking in
the list archives.

Modifiers, command position, and so forth (Re: Bug#519535: history expansion: modifier completion missing)

[Bart Schaefer]

On Mar 17,  9:57am, Peter Stephenson wrote:
}
} > torch% echo foo
} > torch% !!:a
} > /usr/local/src/zsh/zsh-build/echo foo
} > zsh: no such file or directory: /usr/local/src/zsh/zsh-build/echo
} > 
} > That's almost certainly not the intended behavior ... is it?
} 
} It depends what you mean by "intended": Michael did say the operation
} didn't take account of whether the file existed (but I didn't document
} this), and there was never any special behaviour for command words.

I guess I'm a little skeptical of the utility of this new option in
general.  All the other modifiers do something "inside" the history
string, which would otherwise be difficult to do -- change case,
truncate, substitute, etc.  This one is just prepending something in
a way that can already easily be done with $PWD/!$ (for example).

So if it's not going to be intelligent about what it does, what's the
point?  On the other hand I agree with your conclusion later in this
thread that having :a behave like `whence` depending on whether it
is used on the command word, is also confusing.  Consequently I can't
decide what intelligence is appropriate.  It'd be a lot more useful
if :w worked on history; !*:w:a to make all the arguments into paths
would be handy.

It'd be REALLY handy if :a figured out what the current directory was
at the time the historical command was originally executed and pasted
THAT directory onto the front of the command word, rather than always
using the value of $PWD.  Sort of an uber-$OLDPWD for all history.

Here's another case where some intelligence could be applied:

schaefer<549> cd Src/Zle
schaefer<550> echo ../b*
../builtin.c
schaefer<551> echo !$:a
echo /usr/local/src/zsh/zsh-4.0/Src//b*
/usr/local/src/zsh/zsh-4.0/Src//builtin.c

Double slash?

Incidentally, it's not just (:c) that doesn't work if there's no file
matching the name.  Any other modifier-as-qualifier will fail, too.
This apparently leads to yet other oddities for "command position".

schaefer<503> /bin/echo xxx
xxx
schaefer<504> /bin/echo(:h) xx
zsh: permission denied: /bin
schaefer<505> /bin/echo(:t) xx
xx
schaefer<506> /bin/echo(:a) xx
xx

All as expected.  But now it gets weird:

schaefer<508> echo(:t) xx
zsh: command not found: xx
schaefer<509> echo echo(:t) xx
zsh: no match
schaefer<510> echo(:a) xx 
zsh: command not found: xx

The command disappears entirely when the qualifier fails.  This turns
out to be some strange side-effect of cshnullglob:

schaefer<514> unsetopt cshnullglob 
schaefer<515> echo(:a) xx         
zsh: no matches found: echo(:a)

(Hrm, different error message, too.)

-- 

Re: Modifiers, command position, and so forth (Re: Bug#519535: history expansion: modifier completion missing)

[Peter Stephenson]

On Tue, 17 Mar 2009 11:46:05 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> I guess I'm a little skeptical of the utility of this new option in
> general.  All the other modifiers do something "inside" the history
> string, which would otherwise be difficult to do -- change case,
> truncate, substitute, etc.  This one is just prepending something in
> a way that can already easily be done with $PWD/!$ (for example).

If anything, the boot's on the other foot.  Most of the other modifiers
do simple string handling: find a fixed character, strip something off
in front or behind.  This one rationalises internal components and
possibly resolves symbolic links.

> Here's another case where some intelligence could be applied:
> 
> schaefer<549> cd Src/Zle
> schaefer<550> echo ../b*
> ../builtin.c
> schaefer<551> echo !$:a
> echo /usr/local/src/zsh/zsh-4.0/Src//b*
> /usr/local/src/zsh/zsh-4.0/Src//builtin.c
> 
> Double slash?

That's a bug.

The Meta handling didn't seem to be consistent, either.  I presume the
functions should be both taking and returning metafied strings.

I see it uses realpath() for normalising symbolic links.  I'm not sure
how standard that is (or rather used to be), and the Linux manual warns
about possible size problems.  We should probably use an improved
version of zgetdir(), which could also benefit from having PATH_MAX
removed.  However, zgetdir() is completely undocumented, so that's for
later.

Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.90
diff -u -r1.90 hist.c
--- Src/hist.c	15 Mar 2009 01:17:06 -0000	1.90
+++ Src/hist.c	18 Mar 2009 19:34:27 -0000
@@ -1507,7 +1507,7 @@
 	return 1;
 
     if (**junkptr != '/') {
-	*junkptr = zhtricat(zgetcwd(), "/", *junkptr);
+	*junkptr = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP), "/", *junkptr);
     }
 
     current = *junkptr;
@@ -1556,6 +1556,8 @@
 		    if (dest[-1] != '/')
 			dest--;
 		    current += 2;
+		    if (*current == '/')
+			current++;
 		} else if (dest == *junkptr + 1) {
 		    /* This might break with Cygwin's leading double slashes? */
 		    current += 2;
@@ -1567,7 +1569,7 @@
 	} else {
 	    while (*current != '/' && *current != '\0')
 		if ((*dest++ = *current++) == Meta)
-		    dest[-1] = *current++ ^ 32;
+		    *dest++ = *current++;
 	}
     }
     return 1;
@@ -1593,6 +1595,8 @@
     if (**junkptr != '/')
 	return 0;
 
+    unmetafy(*junkptr, NULL);
+
     lastpos = strend(*junkptr);
     nonreal = lastpos + 1;
 
@@ -1618,7 +1622,7 @@
 	str++;
     }
 
-    *junkptr = bicat(real, nonreal);
+    *junkptr = metafy(bicat(real, nonreal), -1, META_HEAPDUP);
 
     return 1;
 }


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: Modifiers, command position, and so forth (Re: Bug#519535: history expansion: modifier completion missing)

[Bart Schaefer]

On Wed, Mar 18, 2009 at 12:36 PM, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
> On Tue, 17 Mar 2009 11:46:05 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
>> I guess I'm a little skeptical of the utility of this new option in
>> general.  All the other modifiers do something "inside" the history
>> string, which would otherwise be difficult to do -- change case,
>> truncate, substitute, etc.  This one is just prepending something in
>> a way that can already easily be done with $PWD/!$ (for example).
>
> If anything, the boot's on the other foot.  Most of the other modifiers
> do simple string handling: find a fixed character, strip something off
> in front or behind.  This one rationalises internal components and
> possibly resolves symbolic links.

The question isn't how much work it does, the question is how much
work it saves the user.

Consider that, in order to perform the operation "find a fixed
characeter, strip something off in front or behind" the user would
have to either:
-- retrieve the correct word from the history
-- store it in a variable
-- retype the command with a parameter substitution reference

Or, using the line editor, some variation of:
-- find, copy, and paste the word from the history
-- move to the character within the word
-- delete the appropriate substrings

The above as contrasted with typing !!:4:t:r (or whatever).

Compare this to to !!:4:a vs. $PWD/!!:4:t and you see what I'm getting
at.  !!:4:A is a little different but it's still only $(pwd
-P)/!!:4:t.  Yes, there's always MT1WTD any of the other history
operations as well (basename, dirname, sed, etc.), it's all a question
of degree of difficulty.

Throw in that I'd expect to encounter build problems with realpath()
somewhere, and I end up wondering whether the benefit is worth the
cost.  That's all I'm saying.

Re: Modifiers, command position, and so forth (Re: Bug#519535: history expansion: modifier completion missing)

[Peter Stephenson]

On Wed, 18 Mar 2009 19:36:17 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> I see it uses realpath() for normalising symbolic links.  I'm not sure
> how standard that is (or rather used to be), and the Linux manual warns
> about possible size problems.  We should probably use an improved
> version of zgetdir(), which could also benefit from having PATH_MAX
> removed.  However, zgetdir() is completely undocumented, so that's for
> later.

zgetdir() appears to be quite hairy enough already.  (I wonder if we still
need those holdintr()s---wouldn't queue_interrupts() be more appropriate?)
I've added some comments, fixed a misplaced #ifdef, then ignored it.

This tests for realpath() and the GNU extension that allows the function to
malloc() the path.  I've taken the easy way out and used the alternative
interface to the latter, but if there are other systems that allocate the
path when NULL is passed as the second argument to realpath() but don't
have canonicalize_file_name() it would probably be worth doing a configure
test for working realpath(path, NULL) instead.

There was also another stray variable declaration in the middle of the
function which an older compiler wouldn't like.

Index: configure.ac
===================================================================
RCS file: /cvsroot/zsh/zsh/configure.ac,v
retrieving revision 1.121
diff -u -r1.121 configure.ac
--- configure.ac	16 Mar 2009 05:20:36 -0000	1.121
+++ configure.ac	19 Mar 2009 14:27:47 -0000
@@ -1157,7 +1157,8 @@
 	       grantpt unlockpt ptsname \
 	       htons ntohs \
 	       regcomp regexec regerror regfree \
-	       gdbm_open getxattr)
+	       gdbm_open getxattr \
+	       realpath canonicalize_file_name)
 AC_FUNC_STRCOLL
 
 if test x$enable_cap = xyes; then
Index: Doc/Zsh/expn.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/expn.yo,v
retrieving revision 1.103
diff -u -r1.103 expn.yo
--- Doc/Zsh/expn.yo	15 Mar 2009 01:17:06 -0000	1.103
+++ Doc/Zsh/expn.yo	19 Mar 2009 14:27:47 -0000
@@ -223,7 +223,8 @@
 item(tt(A))(
 As `tt(a)', but also resolve use of symbolic links where possible.
 Note that resolution of `tt(..)' occurs em(before) resolution of symbolic
-links.
+links.  This call is equivalent to tt(a) unless your system has the
+tt(realpath) system call (modern systems do).
 )
 item(tt(e))(
 Remove all but the extension.
Index: Src/compat.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/compat.c,v
retrieving revision 1.17
diff -u -r1.17 compat.c
--- Src/compat.c	22 Apr 2008 15:08:12 -0000	1.17
+++ Src/compat.c	19 Mar 2009 14:27:48 -0000
@@ -227,6 +227,26 @@
 }
 #endif
 
+/*
+ * Rationalise the current directory, returning the string.
+ *
+ * If "d" is not NULL, it is used to store information about the
+ * directory.  The returned name is also present in d->dirname and is in
+ * permanently allocated memory.  The handling of this case depends on
+ * whether the fchdir() system call is available; if it is, it is assumed
+ * the caller is able to restore the current directory.  On successfully
+ * identifying the directory the function returns immediately rather
+ * than ascending the hierarchy.
+ *
+ * If "d" is NULL, no assumption about the caller's behaviour is
+ * made.  The returned string is in heap memory.  This case is
+ * always handled by changing directory up the hierarchy.
+ *
+ * On Cygwin or other systems where USE_GETCWD is defined (at the
+ * time of writing only QNX), we skip all the above and use the
+ * getcwd() system call.
+ */
+
 /**/
 mod_export char *
 zgetdir(struct dirsav *d)
@@ -257,25 +277,30 @@
 	return buf;
     }
 
+    /* Record the initial inode and device */
     pino = sbuf.st_ino;
     pdev = sbuf.st_dev;
     if (d)
 	d->ino = pino, d->dev = pdev;
+#if !defined(__CYGWIN__) && !defined(USE_GETCWD)
 #ifdef HAVE_FCHDIR
     else
 #endif
-#if !defined(__CYGWIN__) && !defined(USE_GETCWD)
 	holdintr();
 
     for (;;) {
+	/* Examine the parent of the current directory. */
 	if (stat("..", &sbuf) < 0)
 	    break;
 
+	/* Inode and device of curtent directory */
 	ino = pino;
 	dev = pdev;
+	/* Inode and device of current directory's parent */
 	pino = sbuf.st_ino;
 	pdev = sbuf.st_dev;
 
+	/* If they're the same, we've reached the root directory. */
 	if (ino == pino && dev == pdev) {
 	    if (!buf[pos])
 		buf[--pos] = '/';
@@ -291,6 +316,7 @@
 	    return buf + pos;
 	}
 
+	/* Search the parent for the current directory. */
 	if (!(dir = opendir("..")))
 	    break;
 
@@ -303,6 +329,7 @@
 		continue;
 #ifdef HAVE_STRUCT_DIRENT_D_STAT
 	    if(de->d_stat.st_dev == dev && de->d_stat.st_ino == ino) {
+		/* Found the directory we're currently in */
 		strncpy(nbuf + 3, fn, PATH_MAX);
 		break;
 	    }
@@ -311,6 +338,7 @@
 	    if (dev != pdev || (ino_t) de->d_ino == ino)
 # endif /* HAVE_STRUCT_DIRENT_D_INO */
 	    {
+		/* Maybe found directory, need to check device & inode */
 		strncpy(nbuf + 3, fn, PATH_MAX);
 		lstat(nbuf, &sbuf);
 		if (sbuf.st_dev == dev && sbuf.st_ino == ino)
@@ -320,7 +348,7 @@
 	}
 	closedir(dir);
 	if (!de)
-	    break;
+	    break;		/* Not found */
 	len = strlen(nbuf + 2);
 	pos -= len;
 	while (pos <= 1) {
Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.90
diff -u -r1.90 hist.c
--- Src/hist.c	15 Mar 2009 01:17:06 -0000	1.90
+++ Src/hist.c	19 Mar 2009 14:27:48 -0000
@@ -1522,7 +1522,7 @@
 	current += 3;
     }
 #endif
-	
+
     for (;;) {
 	if (*current == '/') {
 #ifdef __CYGWIN__
@@ -1577,7 +1577,14 @@
 int
 chrealpath(char **junkptr)
 {
+    char *str;
+#ifdef HAVE_CANONICALIZE_FILE_NAME
+    char *lastpos, *nonreal, *real;
+#else
+# ifdef HAVE_REAL_PATH
     char *lastpos, *nonreal, real[PATH_MAX];
+# endif
+#endif
 
     if (!**junkptr)
 	return 1;
@@ -1586,6 +1593,9 @@
     if (!chabspath(junkptr))
 	return 0;
 
+#if !defined(HAVE_REALPATH) && !defined(HAVE_CANONICALIZE_FILE_NAME)
+    return 1;
+#else
     /*
      * Notice that this means you cannot pass relative paths into this
      * function!
@@ -1596,7 +1606,19 @@
     lastpos = strend(*junkptr);
     nonreal = lastpos + 1;
 
-    while (!realpath(*junkptr, real)) {
+    while (!
+#ifdef HAVE_CANONICALIZE_FILE_NAME
+	   /*
+	    * This is a GNU extension to realpath(); it's the
+	    * same as calling realpath() with a NULL second argument
+	    * which uses malloc() to get memory.  The alternative
+	    * interface is easier to test for, however.
+	    */
+	   (real = canonicalize_file_name(*junkptr))
+#else
+	   realpath(*junkptr, real)
+#endif
+	) {
 	if (errno == EINVAL || errno == ELOOP ||
 	    errno == ENAMETOOLONG || errno == ENOMEM)
 	    return 0;
@@ -1611,7 +1633,7 @@
 	*nonreal = '\0';
     }
 
-    char *str = nonreal;
+    str = nonreal;
     while (str <= lastpos) {
 	if (*str == '\0')
 	    *str = '/';
@@ -1619,6 +1641,10 @@
     }
 
     *junkptr = bicat(real, nonreal);
+#ifdef HAVE_CANONICALIZE_FILE_NAME
+    free(real);
+#endif
+#endif
 
     return 1;
 }


-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: Bug#519535: history expansion: modifier completion missing

[Mikael Magnusson]

2009/3/16 Peter Stephenson <pws@csr.com>:
> On Sun, 15 Mar 2009 06:22:53 +0000
> Clint Adams <schizo@debian.org> wrote:
>> On Fri, Mar 13, 2009 at 11:55:55AM +0100, martin f krafft wrote:
>> > I tried to show off zsh to a sceptic today and had to find out the
>> > hard way that it does not (yet) provide completion for history
>> > expansion modifiers, e.g.
>> >
>> >   echo !$:<TAB>
>
> I thought this was going to be harder than it turned out to be; as you can
> see it's really quite simple, particularly since I already wrote modifer
> completion and it would appear had the foresight to make it handle the
> history case.
>
> It's in _normal which is where we handle normal command arguments.  I think
> this is both the first and last place where get to massage command line
> arguments generically---we don't just want this in default completion since
> history expansion, if active, trumps everything else.
[...]
> So it seems to work fine (possibly depending on your options and
> completers---there are other things around that will do other things to
> !'s), but I'm still a bit confused.

Is it supposed to work here? $PWD:<tab> (it doesn't for me). It does
complete if you write $PWD(:<tab>, but also in ${PWD(:<tab>, but
accepting one of the latter produces a syntax error:
% echo ${PWD(:A)}
zsh: bad substitution
(regardless of which modifier you use)

Also, i get this:
$PWD(:s-<tab>
_history_modifiers:34: bad math expression: operand expected at `^-'
_history_modifiers:34: bad math expression: operand expected at `^-'
_history_modifiers:34: bad math expression: operand expected at `^-'
_history_modifiers:34: bad math expression: operand expected at `^-'
_history_modifiers:34: bad math expression: operand expected at `^-'
_history_modifiers:34: bad math expression: operand expected at `^-'
(same with / or other separator (but it says `^/' then, of course))

As an aside, after i write $PWD(:<tab> to get the s, how do i "accept"
the completion to make tab complete the - instead of cycling to the
next completer? The only way i found is typing something and deleting
it... When completing directories i usually just type a /.

-- 
Mikael Magnusson

Re: Bug#519535: history expansion: modifier completion missing

[Peter Stephenson]

Mikael Magnusson wrote:
> Is it supposed to work here? $PWD:<tab> (it doesn't for me).

No, wihin parameters the completion system doesn't handle any case
except parameter names and values properly.  It doesn't know there's
anything special to do there.  Parsing it would be horrible.  (It will,
of course, expand a complete expression as an entirely separate matter.)

> It does
> complete if you write $PWD(:<tab>, but also in ${PWD(:<tab>, but
> accepting one of the latter produces a syntax error:
> % echo ${PWD(:A)}
> zsh: bad substitution
> (regardless of which modifier you use)

That's because it is a syntax error.  Same answer: the completion system
doesn't know that's special, handling all these special cases is
horrific.  It ought to be possible for someone with that sort of time on
their hands to extend check_param() in compcore.c to add new parameter
contexts, but it seems a funny thing to do with your life.  (Actually,
in the case of brace parameters that ought to be easy, it already knows
you're in a brace parameter and it already knows the brace isn't
complete, so it could do the same as it does for completing the
parameter but set a different context to show it's in the trailing
matter.  But that still leaves the non-brace case.)

(Of course, the right way to do this is to have the shell parsed by
examining a grammar which is also used for generating the cases for
completing shell syntax.  Our infinite team of monkeys should have this
done by the next ice age.)

> Also, i get this:
> $PWD(:s-<tab>
> _history_modifiers:34: bad math expression: operand expected at `^-'
> _history_modifiers:34: bad math expression: operand expected at `^-'
> _history_modifiers:34: bad math expression: operand expected at `^-'
> _history_modifiers:34: bad math expression: operand expected at `^-'
> _history_modifiers:34: bad math expression: operand expected at `^-'
> _history_modifiers:34: bad math expression: operand expected at `^-'
> (same with / or other separator (but it says `^/' then, of course))

I obviously didn't try this bit.  It's supposed to give you hints.  Note
this doesn't currently work for !-history itself---out of fear, I used a
very simple test in _normal that only works with a trailing ":".

> As an aside, after i write $PWD(:<tab> to get the s, how do i "accept"
> the completion to make tab complete the - instead of cycling to the
> next completer? The only way i found is typing something and deleting
> it... When completing directories i usually just type a /.

Just type the string that's got to come next; it always does, or the s
is useless: there's no point typing anything you need to delete, just
use what you don't need to delete.  Actually, why don't you just type
"s-"?  The "completion" is really only there as a mnemonic of what can
go at that point, it doesn't save you any typing even in the optimal
case.

Index: Completion/Zsh/Type/_history_modifiers
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Zsh/Type/_history_modifiers,v
retrieving revision 1.3
diff -u -r1.3 _history_modifiers
--- Completion/Zsh/Type/_history_modifiers	17 Mar 2009 10:03:24 -0000	1.3
+++ Completion/Zsh/Type/_history_modifiers	19 Mar 2009 15:54:28 -0000
@@ -31,11 +31,11 @@
       fi
       delim=$PREFIX[1]
       compset -p 1
-      if ! compset "[^$delim]#$delim[^$delim]#$delim"; then
-	if compset "[^$delim]#$delim"; then
-	  _message original string
+      if ! compset -P "[^${delim}]#${delim}[^${delim}]#${delim}"; then
+	if compset -P "[^${delim}]#${delim}"; then
+	  _message "replacement string"
 	else
-	  _message replacement string
+	  _message "original string"
 	fi
 	return
       fi


-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: Bug#519535: history expansion: modifier completion missing

[Mikael Magnusson]

2009/3/19 Peter Stephenson <pws@csr.com>:
> Mikael Magnusson wrote:
>> Is it supposed to work here? $PWD:<tab> (it doesn't for me).
>
> No, wihin parameters the completion system doesn't handle any case
> except parameter names and values properly.  It doesn't know there's
> anything special to do there.  Parsing it would be horrible.  (It will,
> of course, expand a complete expression as an entirely separate matter.)
>
>> It does
>> complete if you write $PWD(:<tab>, but also in ${PWD(:<tab>, but
>> accepting one of the latter produces a syntax error:
>> % echo ${PWD(:A)}
>> zsh: bad substitution
>> (regardless of which modifier you use)
>
> That's because it is a syntax error.  Same answer: the completion system
> doesn't know that's special, handling all these special cases is
> horrific.  It ought to be possible for someone with that sort of time on
> their hands to extend check_param() in compcore.c to add new parameter
> contexts, but it seems a funny thing to do with your life.  (Actually,
> in the case of brace parameters that ought to be easy, it already knows
> you're in a brace parameter and it already knows the brace isn't
> complete, so it could do the same as it does for completing the
> parameter but set a different context to show it's in the trailing
> matter.  But that still leaves the non-brace case.)
>
> (Of course, the right way to do this is to have the shell parsed by
> examining a grammar which is also used for generating the cases for
> completing shell syntax.  Our infinite team of monkeys should have this
> done by the next ice age.)

Heh, i think i'll just go with typing the extra ( when i need the hints then.

>> Also, i get this:
>> $PWD(:s-<tab>
>> _history_modifiers:34: bad math expression: operand expected at `^-'
>> _history_modifiers:34: bad math expression: operand expected at `^-'
>> _history_modifiers:34: bad math expression: operand expected at `^-'
>> _history_modifiers:34: bad math expression: operand expected at `^-'
>> _history_modifiers:34: bad math expression: operand expected at `^-'
>> _history_modifiers:34: bad math expression: operand expected at `^-'
>> (same with / or other separator (but it says `^/' then, of course))
>
> I obviously didn't try this bit.  It's supposed to give you hints.  Note
> this doesn't currently work for !-history itself---out of fear, I used a
> very simple test in _normal that only works with a trailing ":".
>
>> As an aside, after i write $PWD(:<tab> to get the s, how do i "accept"
>> the completion to make tab complete the - instead of cycling to the
>> next completer? The only way i found is typing something and deleting
>> it... When completing directories i usually just type a /.
>
> Just type the string that's got to come next; it always does, or the s
> is useless: there's no point typing anything you need to delete, just
> use what you don't need to delete.  Actually, why don't you just type
> "s-"?  The "completion" is really only there as a mnemonic of what can
> go at that point, it doesn't save you any typing even in the optimal
> case.

Well, i sometimes am in the same situation with other completions where the
next string is something you would want to complete. I can't come up with any
examples at the moment though... I agree it is a bit silly in this specific
case.

-- 
Mikael Magnusson

Re: Bug#519535: history expansion: modifier completion missing

[Bart Schaefer]

On Mar 19,  4:28pm, Mikael Magnusson wrote:
}
} Is it supposed to work here? $PWD:<tab> (it doesn't for me).

I don't think so, but that's probably just an omission.

} It does complete if you write $PWD(:<tab>

That's completing a glob qualifier, not a parameter modifier.  So in
pratice it would first expand $PWD and then attempt to glob it, which
works for $PWD but not for parameter values in general.

} but also in ${PWD(:<tab>

That should probably be considered a bug; it's still completing glob
qualifiers, but ${PWD isn't a valid glob pattern.

} but
} accepting one of the latter produces a syntax error:
} % echo ${PWD(:A)}
} zsh: bad substitution

Not surprising, as it is not valid sytax.

} Also, i get this:
} $PWD(:s-<tab>
} _history_modifiers:34: bad math expression: operand expected at `^-'

Hrm.  $delim[...] is being interpreted as an array reference when it
should be "$delim" followed by a character class pattern.


Index: Completion/Zsh/Type/_history_modifiers
===================================================================
diff -c -r1.1 _history_modifiers
--- _history_modifiers	13 Mar 2008 15:46:07 -0000	1.1
+++ _history_modifiers	19 Mar 2009 16:15:13 -0000
@@ -31,11 +31,11 @@
       fi
       delim=$PREFIX[1]
       compset -p 1
-      if ! compset "[^$delim]#$delim[^$delim]#$delim"; then
-	if compset "[^$delim]#$delim"; then
-	  _message original string
+      if ! compset -P "[^$delim]#$delim""[^$delim]#$delim"; then
+	if compset -P "[^$delim]#$delim"; then
+	  _message "replacement string"
 	else
-	  _message replacement string
+	  _message "original string"
 	fi
 	return
       fi