file completion(?) erases word typed

file completion(?) erases word typed

[Daniel Shahaf]

I noticed something odd in a completion function that (eventually) calls _path_files:

% git config sendemail.smtpserver <TAB><^C> # autoload
% compdef __git_sendmail_smtpserver_values f 
% f /usr/bin/gtk-update-icon-cache-3.<TAB>
% f <CURSOR>

It erased the word I'd typed. ☹

It works fine when the period hasn't been typed.  I've diffed
_complete_debug traces and saw in $_lastcomp that compstate[unambiguous]
differs between the two cases, but I'm not sure why, since the
completion is unique («/usr/bin/gtk-update-icon-cache-3*» has one
match).

Re: file completion(?) erases word typed

[Bart Schaefer]

On Aug 23, 10:48pm, Daniel Shahaf wrote:
}
} I noticed something odd in a completion function that (eventually) calls
} _path_files:
} 
} % git config sendemail.smtpserver <TAB><^C> # autoload
} % compdef __git_sendmail_smtpserver_values f 
} % f /usr/bin/gtk-update-icon-cache-3.<TAB>
} % f <CURSOR>
} 
} It erased the word I'd typed.

I'm not certain what's going on here either, but loading up a few more
zstyles and using a completion that's not unique might have provided a 
hint:

torch% f zsh-<TAB>
torch% f /usr/local/bin/zsh-5<TAB>
Completing hashed command by absolute path
/usr/local/bin/zsh-5.0.2-dev-0      /usr/local/bin/zsh-5.0.8          
/usr/local/bin/zsh-5.0.3            /usr/local/bin/zsh-5.1            
/usr/local/bin/zsh-5.0.4            /usr/local/bin/zsh-5.1.1          
/usr/local/bin/zsh-5.0.5            /usr/local/bin/zsh-5.2            
/usr/local/bin/zsh-5.0.7                                              
Completing file
zsh-5.0.2-dev-0*  zsh-5.0.5*        zsh-5.1*                          
zsh-5.0.3*        zsh-5.0.7*        zsh-5.1.1*                        
zsh-5.0.4*        zsh-5.0.8*        zsh-5.2*

Note that it lists the individual files as possible completions.  For
one of those to match the command line, the /usr/local/bin/ prefix
would have to be erased.

Repeated whacking of TAB at this point menu-cycles through only the
"hashed command by absolute path" selections, the base file names are
never offered.

If I append the "." and use list-choices (^D) I get the same listing
as above, but as soon as I hit TAB instead, the whole word is erased
like your example.

There are a couple of curious tidbits in the _complete_debug traces.

Here we add the command path but tell completion that the path prefix
should be removed from the resulting command line when completing:

         +_hashed_absolute_command_paths:6> compadd -M 'l:|=/usr/local/bin/' -J -default- -a 'commands[(R)${~i}[^/]#]'

Here we add all the base names but say the path prefix should be pasted
back on -- but (weirdly) that the path without its leading slash should
be an ignored prefix:

          +_path_files:713> compadd -Qf -J -default- -p usr/local/bin/ -s '' -W /usr/local/bin/ -M 'r:|/=* r:|=*' -a tmp1

I have no idea why ignoring the path minus its leading slash would ever
be correct, but in any case this appears to be adding the full path by
two different and contradictory approaches.

Re: file completion(?) erases word typed

[Daniel Shahaf]

Bart Schaefer wrote on Tue, Aug 23, 2016 at 22:52:04 -0700:
> On Aug 23, 10:48pm, Daniel Shahaf wrote:
> }
> } I noticed something odd in a completion function that (eventually) calls
> } _path_files:
> } 
> } % git config sendemail.smtpserver <TAB><^C> # autoload
> } % compdef __git_sendmail_smtpserver_values f 
> } % f /usr/bin/gtk-update-icon-cache-3.<TAB>
> } % f <CURSOR>
> } 
> } It erased the word I'd typed.
> 
> I'm not certain what's going on here either, but loading up a few more
> zstyles and using a completion that's not unique might have provided a 
> hint:
> 
> torch% f zsh-<TAB>
> torch% f /usr/local/bin/zsh-5<TAB>
> Completing hashed command by absolute path
> /usr/local/bin/zsh-5.0.2-dev-0      /usr/local/bin/zsh-5.0.8          
> /usr/local/bin/zsh-5.0.3            /usr/local/bin/zsh-5.1            
> /usr/local/bin/zsh-5.0.4            /usr/local/bin/zsh-5.1.1          
> /usr/local/bin/zsh-5.0.5            /usr/local/bin/zsh-5.2            
> /usr/local/bin/zsh-5.0.7                                              
> Completing file
> zsh-5.0.2-dev-0*  zsh-5.0.5*        zsh-5.1*                          
> zsh-5.0.3*        zsh-5.0.7*        zsh-5.1.1*                        
> zsh-5.0.4*        zsh-5.0.8*        zsh-5.2*
> 
> Note that it lists the individual files as possible completions.  For
> one of those to match the command line, the /usr/local/bin/ prefix
> would have to be erased.
> 
> Repeated whacking of TAB at this point menu-cycles through only the
> "hashed command by absolute path" selections, the base file names are
> never offered.

With the following style set, basenames are offered:

zstyle ':completion:*' menu 'select=long-list' 'select=3'

> If I append the "." and use list-choices (^D) I get the same listing
> as above, but as soon as I hit TAB instead, the whole word is erased
> like your example.

It seems "." isn't special here at all: in my original reproducer,
«/usr/bin/gtk-update-icon-cache-<TAB>» erases the word too.  (Without the
last hyphen it'd still be ambiguous)

I don't know why list-choices would act differently to expand-or-complete.

> There are a couple of curious tidbits in the _complete_debug traces.
> 
> Here we add the command path but tell completion that the path prefix
> should be removed from the resulting command line when completing:
> 
>          +_hashed_absolute_command_paths:6> compadd -M 'l:|=/usr/local/bin/' -J -default- -a 'commands[(R)${~i}[^/]#]'
> 
> Here we add all the base names but say the path prefix should be pasted
> back on -- but (weirdly) that the path without its leading slash should
> be an ignored prefix:
> 
>           +_path_files:713> compadd -Qf -J -default- -p usr/local/bin/ -s '' -W /usr/local/bin/ -M 'r:|/=* r:|=*' -a tmp1
> 
> I have no idea why ignoring the path minus its leading slash would ever
> be correct, but in any case this appears to be adding the full path by
> two different and contradictory approaches.

What _absolute_command_paths intended to be is:

- You can type a command name and complete it to its absolute path,
  e.g., ls<TAB> → /bin/ls;

- You can type in an absolute path to an executable file, even if that
  file is not in $PATH or in $commands.

So, when you complete something like /usr/local/bin/foo<TAB>
→ presumably /usr/local/bin is in your $PATH — both alternatives add
/usr/local/bin/foo* matches; the first alternative lets you type
'foo<TAB>' to get /usr/local/bin/foo* matches and the second alternative
lets you type '/usr/local/bin/foo<TAB>' to get those matches.  The
first alternative shouldn't be adding any matches in the situation of
the minimal reproducer.

Cheers,

Daniel

Re: file completion(?) erases word typed

[Bart Schaefer]

On Aug 24,  7:13pm, Daniel Shahaf wrote:
}
} >           +_path_files:713> compadd -Qf -J -default- -p usr/local/bin/ -s '' -W /usr/local/bin/ -M 'r:|/=* r:|=*' -a tmp1
} > 
} > I have no idea why ignoring the path minus its leading slash would ever
} > be correct, but in any case this appears to be adding the full path by
} > two different and contradictory approaches.
} 
} What _absolute_command_paths intended to be is:
} 
} - You can type in an absolute path to an executable file, even if that
}   file is not in $PATH or in $commands.

What's the reason for passing -P / to _path_files in _typed-in_* ?

That's what's causing the strange -p usr/local/bin getting passed to
compadd.  Despite what the doc says, _path_files doesn't actually pass
the -P option along to compadd, instead it calls "compset -P" which
makes it an ignored prefix instead.

I'm still not sure whether (or what) this has to do with the word on
the line disappearing.

Re: file completion(?) erases word typed

[Daniel Shahaf]

Bart Schaefer wrote on Wed, Aug 24, 2016 at 17:04:54 -0700:
> On Aug 24,  7:13pm, Daniel Shahaf wrote:
> }
> } >           +_path_files:713> compadd -Qf -J -default- -p usr/local/bin/ -s '' -W /usr/local/bin/ -M 'r:|/=* r:|=*' -a tmp1
> } > 
> } > I have no idea why ignoring the path minus its leading slash would ever
> } > be correct, but in any case this appears to be adding the full path by
> } > two different and contradictory approaches.
> } 
> } What _absolute_command_paths intended to be is:
> } 
> } - You can type in an absolute path to an executable file, even if that
> }   file is not in $PATH or in $commands.
> 
> What's the reason for passing -P / to _path_files in _typed-in_* ?

_typed-in_* must complete only absolute paths.  The -P / was an attempt
to disallow non-absolute paths.

Perhaps this would be better?  There may be a way to rewrite it without
-P entirely, but that evades me at the moment.

diff --git a/Completion/Unix/Type/_absolute_command_paths b/Completion/Unix/Type/_absolute_command_paths
index e9ab170..f61f04d 100644
--- a/Completion/Unix/Type/_absolute_command_paths
+++ b/Completion/Unix/Type/_absolute_command_paths
@@ -16,7 +18,13 @@ _hashed_absolute_command_paths() {
 # This function completes absolute pathnames of executables, e.g., /etc/rc.local
 _typed-in_absolute_command_paths() {
   # TODO: the description "full path to an executable" and tag in the caller are ignored by _path_files
-  _path_files -/ -g '*(-*)' -P / -W /
+  if [[ -z $PREFIX ]]; then
+    _path_files -/ -g '*(-*)' -P / -W /
+  elif [[ $PREFIX[1] == / ]]; then
+    _path_files -/ -g '*(-*)' -W /
+  else
+    return 1
+  fi
 }
 
 _absolute_command_paths() {

> That's what's causing the strange -p usr/local/bin getting passed to
> compadd.  Despite what the doc says, _path_files doesn't actually pass
> the -P option along to compadd, instead it calls "compset -P" which
> makes it an ignored prefix instead.
> 
> I'm still not sure whether (or what) this has to do with the word on
> the line disappearing.

*nod*

Re: file completion(?) erases word typed

[Bart Schaefer]

On Aug 24,  7:13pm, Daniel Shahaf wrote:
}
} I don't know why list-choices would act differently to expand-or-complete.

Some additional clues ...

Using my example (because I don't have gtk-* installed):

torch% f /usr/local/bin/zsh-5<TAB>

Here the word on the line is not yet the longest common prefix of any of
the possible matches.  So we enter compresult.c:do_ambiguous() and on
line 786 we delete the entire word on the line, but ainfo->line has a
prefix, which is inserted by cline_str() at line 789.  This happens to
be the same word that's already on the line.  Because a prefix has
been inserted, the set of matching suffixes is listed.

Next with:

torch% f /usr/local/bin/zsh-5.<TAB>

We arrive in do_ambiguous() and hit line 786 as before, but this time
the string that was on the line is already the longest common prefix;
so now ainfo->line does NOT have a prefix with which to replace it.  
In this case cline_str() should re-insert the original word from the
line, which is present in ainfo->line->orig, but for some reason the
value of ainfo->line->olen is zero, so cline_str() ends up doing
nothng and the erased word is never restored.

ainfo->line->olen appears ultimately to come from add_match_part()
called from match_str() at line 739 of compmatch.c; it's merged from
there into ainfo by join_clines() called from add_match_data().  At
this point, though, I get lost trying to keep track of what's going
on with l + loff and llen (which comes from mp->llen) and ll and all
of Sven's other usefully named variables.

======

I *think* what it comes down to is that the matcher-list is causing
the path prefix to be mapped to nothing so as to be able to complete
the basename of the command, but because completion wants to wait for
a second tab before starting the menu on ambiguous results, it ends
up throwing away the data that it would need to do anything useful
when that second tab finally arrives.

This hypothesis is supported by the fact that using zstyle to force
entry into menu-selection results in completions being available.

However, I don't know whether the right thing is to recognized that
the prefix is going to end up empty and jump directly into the menu
without looking for the second tab, or to second-guess the matcher
code and re-insert the original word when that fails to do so; or if
there is instead a fix to compmatch.c needed.  And I don't presently
have any idea how to implement any of those choices.

legend for match_str (was: Re: file completion(?) erases word typed)

[Daniel Shahaf]

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]

Bart Schaefer wrote on Tue, Aug 30, 2016 at 00:03:31 -0700:
> ainfo->line->olen appears ultimately to come from add_match_part()
> called from match_str() at line 739 of compmatch.c; it's merged from
> there into ainfo by join_clines() called from add_match_data().  At
> this point, though, I get lost trying to keep track of what's going
> on with l + loff and llen (which comes from mp->llen) and ll and all
> of Sven's other usefully named variables.

Some variable name documentation attached.

I haven't renamed them for various reasons, e.g., to keep 'blame' usable
and to maintain similarity to other functions which may also use the
same short names.

I'm planning to commit this with distinct X-Seq numbers such as
39309/{0001..0011} — didn't want to send 11 separate emails for such
trivial changes.

Cheers,

Daniel

[-- Attachment #2: 0001-internals-match_str-Document-some-local-variables.-S.patch --]
[-- Type: text/x-diff, Size: 2325 bytes --]

>From f2fd645003518e60a8045b3c9c42dcf4488932ae Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 14 Sep 2016 03:38:34 +0000
Subject: [PATCH 01/10] internals: match_str: Document some local variables. 
 See 39123.

---
 Src/Zle/compmatch.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
index 0e41ac3..a920c8a 100644
--- a/Src/Zle/compmatch.c
+++ b/Src/Zle/compmatch.c
@@ -593,9 +593,59 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 		    continue;
 
 		if (mp->wlen < 0) {
-		    int both, loff, aoff, llen, alen, zoff, moff, ct, ict, aol;
+		    /* 
+		     * 1 iff the anchor and the word are on the same side of
+		     * the line pattern; that is: if either
+		     * - the anchor is on the left and we are matching a prefix; or
+		     * - the anchor is on the right and we are matching a suffix.
+		     */
+		    int both;
+		    /*
+		     * Offset from the line pattern pointer ('l') to the start of the
+		     * line pattern.
+		     */
+		    int loff;
+		    /*
+		     * Offset from the line pattern pointer ('l') to the start of the
+		     * anchor.
+		     */
+		    int aoff;
+		    /*
+		     * The length of the line pattern.
+		     */
+		    int llen;
+		    /*
+		     * The length of the anchor.
+		     *
+		     * SEE: ap; aol, aop
+		     */
+		    int alen;
+		    /*
+		     * ### These two are related: they're set symmetrically.
+		     */
+		    int zoff, moff;
+		    /*
+		     * ### These two are related.
+		     */
+		    int ct, ict;
+		    /*
+		     * The length of the OTHER anchor: the left anchor when we're anchored
+		     * on the right, and of the right anchor when we're anchored on the left.
+		     */
+		    int aol;
+		    /*
+		     * LOST: Documentation comment.  Last seen 10 years ago in the temporal lobe.
+		     * Reward promised for its safe return.  Contact zsh-workers@zsh.org.
+		     */
 		    char *tp, savl = '\0', savw;
-		    Cpattern ap, aop;
+		    /*
+		     * The anchor on this end.
+		     */
+		    Cpattern ap;
+		    /*
+		     * The anchor on the other end.
+		     */
+		    Cpattern aop;
 
 		    /* This is for `*' patterns, first initialise some
 		     * local variables. */

[-- Attachment #3: 0002-internals-match_str-Simplify-by-removing-zoff.patch --]
[-- Type: text/x-diff, Size: 2828 bytes --]

>From 15b342777a9fe5b8d20c2f5119c0d704091d9900 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 14 Sep 2016 03:38:34 +0000
Subject: [PATCH 02/10] internals: match_str: Simplify by removing 'zoff'.

'zoff' was only used within 'if (sfx)' blocks, in which case it was initialized
to 'alen', so simply s/zoff/alen/g.  'alen' is not const but it first changes
on line 794, after the last use of 'zoff'.
---
 Src/Zle/compmatch.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
index a920c8a..1e5f551 100644
--- a/Src/Zle/compmatch.c
+++ b/Src/Zle/compmatch.c
@@ -621,9 +621,9 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 		     */
 		    int alen;
 		    /*
-		     * ### These two are related: they're set symmetrically.
+		     * ### Related to 'zoff', which was removed in 2016.
 		     */
-		    int zoff, moff;
+		    int moff;
 		    /*
 		     * ### These two are related.
 		     */
@@ -661,14 +661,14 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 			continue;
 
 		    if (mp->flags & CMF_LEFT) {
-			ap = mp->left; zoff = 0; moff = alen; aop = mp->right;
+			ap = mp->left; moff = alen; aop = mp->right;
 			if (sfx) {
 			    both = 0; loff = -llen; aoff = -(llen + alen);
 			} else {
 			    both = 1; loff = alen; aoff = 0;
 			}
 		    } else {
-			ap = mp->right; zoff = alen; moff = 0; aop = mp->left;
+			ap = mp->right; moff = 0; aop = mp->left;
 			if (sfx) {
 			    both = 1; loff = -(llen + alen); aoff = -alen;
 			} else {
@@ -694,8 +694,8 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 
 		    /* Fine, now we call ourselves recursively to find the
 		     * string matched by the `*'. */
-		    if (sfx && (savl = l[-(llen + zoff)]))
-			l[-(llen + zoff)] = '\0';
+		    if (sfx && (savl = l[-(llen + alen)]))
+			l[-(llen + alen)] = '\0';
 		    for (t = 0, tp = w, ct = 0, ict = lw - alen + 1;
 			 ict;
 			 tp += add, ct++, ict--) {
@@ -717,12 +717,12 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 				!match_parts(l + aoff , tp - moff, alen, part))
 				break;
 			    if (sfx) {
-				if ((savw = tp[-zoff]))
-				    tp[-zoff] = '\0';
+				if ((savw = tp[-alen]))
+				    tp[-alen] = '\0';
 				t = match_str(l - ll, w - lw,
 					      NULL, 0, NULL, 1, 2, part);
 				if (savw)
-				    tp[-zoff] = savw;
+				    tp[-alen] = savw;
 			    } else
 				t = match_str(l + llen + moff, tp + moff,
 					      NULL, 0, NULL, 0, 1, part);
@@ -732,7 +732,7 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 		    }
 		    ict = ct;
 		    if (sfx && savl)
-			l[-(llen + zoff)] = savl;
+			l[-(llen + alen)] = savl;
 
 		    /* Have we found a position in w where the rest of l
 		     * matches? */

[-- Attachment #4: 0003-internals-match_str-Document-savw-.-Avoid-magic-numb.patch --]
[-- Type: text/x-diff, Size: 1597 bytes --]

>From 6333669d7dc9525eb8103d993d6bf90c002c97d0 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 14 Sep 2016 03:38:34 +0000
Subject: [PATCH 03/10] internals: match_str: Document 'savw'.  Avoid magic
 number.

All callees checked to ensure that they only check that parameter for nonzeroness.
---
 Src/Zle/compmatch.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
index 1e5f551..654eec4 100644
--- a/Src/Zle/compmatch.c
+++ b/Src/Zle/compmatch.c
@@ -637,7 +637,7 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 		     * LOST: Documentation comment.  Last seen 10 years ago in the temporal lobe.
 		     * Reward promised for its safe return.  Contact zsh-workers@zsh.org.
 		     */
-		    char *tp, savl = '\0', savw;
+		    char *tp, savl = '\0';
 		    /*
 		     * The anchor on this end.
 		     */
@@ -717,15 +717,18 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 				!match_parts(l + aoff , tp - moff, alen, part))
 				break;
 			    if (sfx) {
+				/* Call ourselves recursively with the
+				 * anchor removed. */
+				char savw;
 				if ((savw = tp[-alen]))
 				    tp[-alen] = '\0';
 				t = match_str(l - ll, w - lw,
-					      NULL, 0, NULL, 1, 2, part);
+					      NULL, 0, NULL, sfx, 2, part);
 				if (savw)
 				    tp[-alen] = savw;
 			    } else
 				t = match_str(l + llen + moff, tp + moff,
-					      NULL, 0, NULL, 0, 1, part);
+					      NULL, 0, NULL, sfx, 1, part);
 			    if (t || (mp->wlen == -1 && !both))
 				break;
 			}

[-- Attachment #5: 0004-internals-match_str-Document-savl.patch --]
[-- Type: text/x-diff, Size: 1181 bytes --]

>From 8f7e6f89ba493f47939adc093b1708597fba9705 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 14 Sep 2016 03:38:34 +0000
Subject: [PATCH 04/10] internals: match_str: Document 'savl'.

---
 Src/Zle/compmatch.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
index 654eec4..001a166 100644
--- a/Src/Zle/compmatch.c
+++ b/Src/Zle/compmatch.c
@@ -637,7 +637,22 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 		     * LOST: Documentation comment.  Last seen 10 years ago in the temporal lobe.
 		     * Reward promised for its safe return.  Contact zsh-workers@zsh.org.
 		     */
-		    char *tp, savl = '\0';
+		    char *tp;
+		    /* 
+		     * Temporary variable.  Used as temporary storage for a
+		     *
+		     *     {
+		     *         () {
+		     *           local foo="$foo"
+		     *           foo[1]=bar
+		     *           ...
+		     *         }
+		     *         (use original $foo here)
+		     *     }
+		     *
+		     * operation.  Similar to savw.
+		     */
+		    char savl;
 		    /*
 		     * The anchor on this end.
 		     */

[-- Attachment #6: 0005-internals-match_str-Constify-some-local-variables.patch --]
[-- Type: text/x-diff, Size: 1463 bytes --]

>From 4fb33a729171beba2309cae3cf3ec5866950babe Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 14 Sep 2016 03:38:34 +0000
Subject: [PATCH 05/10] internals: match_str: Constify some local variables.

---
 Src/Zle/compmatch.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
index 001a166..c51d849 100644
--- a/Src/Zle/compmatch.c
+++ b/Src/Zle/compmatch.c
@@ -498,14 +498,17 @@ add_match_sub(Cmatcher m, char *l, int ll, char *w, int wl)
 /**/
 int
 match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
-	  int sfx, int test, int part)
+	  const int sfx, int test, int part)
 {
     int ll = strlen(l), lw = strlen(w), oll = ll, olw = lw, exact = 0, wexact = 0;
-    int il = 0, iw = 0, t, ind, add, he = 0, bpc, obc = bc, bslash;
+    int il = 0, iw = 0, t, he = 0, bpc, bslash;
     char *ow;
-    Cmlist ms;
+    Cmlist ms; /* loop variable */
     Cmatcher mp, lm = NULL;
     Brinfo bp = NULL;
+    const int obc = bc;
+    const int ind = (sfx ? -1 : 0);
+    const int add = (sfx ? -1 : 1);
 
     if (!test) {
 	start_match();
@@ -516,9 +519,6 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 
     if (sfx) {
 	l += ll; w += lw;
-	ind = -1; add = -1;
-    } else {
-	ind = 0; add = 1;
     }
     /* ow will always point to the beginning (or end) of that sub-string
      * in w that wasn't put in the match-variables yet. */

[-- Attachment #7: 0006-internals-match_str-Downscope-local-variable-bpc.patch --]
[-- Type: text/x-diff, Size: 1614 bytes --]

>From 9151d81b576f0b7d560f828b1a28f02326ccec51 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 14 Sep 2016 03:38:34 +0000
Subject: [PATCH 06/10] internals: match_str: Downscope local variable 'bpc'.

---
 Src/Zle/compmatch.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
index c51d849..f436a49 100644
--- a/Src/Zle/compmatch.c
+++ b/Src/Zle/compmatch.c
@@ -501,7 +501,7 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 	  const int sfx, int test, int part)
 {
     int ll = strlen(l), lw = strlen(w), oll = ll, olw = lw, exact = 0, wexact = 0;
-    int il = 0, iw = 0, t, he = 0, bpc, bslash;
+    int il = 0, iw = 0, t, he = 0, bslash;
     char *ow;
     Cmlist ms; /* loop variable */
     Cmatcher mp, lm = NULL;
@@ -820,12 +820,14 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 		    bc += llen;
 		    exact = 0;
 
-		    if (!test)
+		    if (!test) {
+			int bpc;
 			while (bp &&
 			       bc >= (bpc = (useqbr ? bp->qpos : bp->pos))) {
 			    bp->curpos = matchbufadded + bpc - bc + obc;
 			    bp = bp->next;
 			}
+		    }
 		    ow = w;
 
 		    if (!llen && !alen) {
@@ -943,12 +945,14 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 		    bc += mp->llen;
 		    exact = 0;
 
-		    if (!test)
+		    if (!test) {
+			int bpc;
 			while (bp &&
 			       bc >= (bpc = (useqbr ? bp->qpos : bp->pos))) {
 			    bp->curpos = matchbufadded + bpc - bc + obc;
 			    bp = bp->next;
 			}
+		    }
 		    ow = w;
 		    lm = NULL;
 		    he = 0;

[-- Attachment #8: 0007-internals-match_str-Rename-and-constify-local-variab.patch --]
[-- Type: text/x-diff, Size: 1513 bytes --]

>From 7db7a9a55f3897e34300ef7462c45fda4a7fbc91 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 14 Sep 2016 03:38:35 +0000
Subject: [PATCH 07/10] internals: match_str: Rename and constify local
 variables 'oll', 'olw'.

---
 Src/Zle/compmatch.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
index f436a49..673347e 100644
--- a/Src/Zle/compmatch.c
+++ b/Src/Zle/compmatch.c
@@ -500,7 +500,7 @@ int
 match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 	  const int sfx, int test, int part)
 {
-    int ll = strlen(l), lw = strlen(w), oll = ll, olw = lw, exact = 0, wexact = 0;
+    int ll = strlen(l), lw = strlen(w), exact = 0, wexact = 0;
     int il = 0, iw = 0, t, he = 0, bslash;
     char *ow;
     Cmlist ms; /* loop variable */
@@ -509,6 +509,7 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
     const int obc = bc;
     const int ind = (sfx ? -1 : 0);
     const int add = (sfx ? -1 : 1);
+    const int original_ll = ll, original_lw = lw;
 
     if (!test) {
 	start_match();
@@ -585,7 +586,7 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 	    for (mp = ms->matcher; mp; mp = mp->next) {
 		t = 1;
 		if ((lm && lm == mp) ||
-		    ((oll == ll || olw == lw) &&
+		    ((original_ll == ll || original_lw == lw) &&
 		     (test == 1 || (test && !mp->left && !mp->right)) &&
 		     mp->wlen < 0))
 		    /* If we were called recursively, don't use `*' patterns

[-- Attachment #9: 0008-internals-match_str-Document-several-local-variables.patch --]
[-- Type: text/x-diff, Size: 1849 bytes --]

>From 62442f34017813d9241a2808f4ddf167e12fb68e Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 14 Sep 2016 03:38:35 +0000
Subject: [PATCH 08/10] internals: match_str: Document several local variables.

---
 Src/Zle/compmatch.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
index 673347e..16705f6 100644
--- a/Src/Zle/compmatch.c
+++ b/Src/Zle/compmatch.c
@@ -500,8 +500,15 @@ int
 match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 	  const int sfx, int test, int part)
 {
-    int ll = strlen(l), lw = strlen(w), exact = 0, wexact = 0;
-    int il = 0, iw = 0, t, he = 0, bslash;
+    /* How many characters from the line string and from the word string are yet to be matched. */
+    int ll = strlen(l), lw = strlen(w);
+    /* Number of characters from the line string and word string matched. */
+    int il = 0, iw = 0;
+    /* How many characters were matched exactly in the line and in the word. */
+    int exact = 0, wexact = 0;
+    int he = 0;
+    int bslash;
+    int t;
     char *ow;
     Cmlist ms; /* loop variable */
     Cmatcher mp, lm = NULL;
@@ -511,6 +518,8 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
     const int add = (sfx ? -1 : 1);
     const int original_ll = ll, original_lw = lw;
 
+    /* INVARIANT: il+ll == original_ll; iw+lw == original_lw */
+
     if (!test) {
 	start_match();
 	bp = *bpp;
@@ -627,6 +636,10 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 		    int moff;
 		    /*
 		     * ### These two are related.
+		     *
+		     * ### They may have a relation similar to that of lw/iw (q.v.), 
+		     * ### at least during the 'for' loop.  They may be overloaded/repurposed
+		     * ### after it.
 		     */
 		    int ct, ict;
 		    /*

[-- Attachment #10: 0009-internals-match_str-Downscope-local-variable-t.patch --]
[-- Type: text/x-diff, Size: 1976 bytes --]

>From 1304e6473db8d1fdb6d7a8ada741a61a38984b88 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 14 Sep 2016 03:38:35 +0000
Subject: [PATCH 09/10] internals: match_str: Downscope local variable 't'.

Remove needless initialization (it is written to again before it is ever read).

Note there was another 't' variable at the end of the function that shadowed
the int 't'.
---
 Src/Zle/compmatch.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
index 16705f6..1d4bef8 100644
--- a/Src/Zle/compmatch.c
+++ b/Src/Zle/compmatch.c
@@ -508,7 +508,6 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
     int exact = 0, wexact = 0;
     int he = 0;
     int bslash;
-    int t;
     char *ow;
     Cmlist ms; /* loop variable */
     Cmatcher mp, lm = NULL;
@@ -593,7 +592,6 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 	/* First try the matchers. Err... see above. */
 	for (mp = NULL, ms = mstack; !mp && ms; ms = ms->next) {
 	    for (mp = ms->matcher; mp; mp = mp->next) {
-		t = 1;
 		if ((lm && lm == mp) ||
 		    ((original_ll == ll || original_lw == lw) &&
 		     (test == 1 || (test && !mp->left && !mp->right)) &&
@@ -603,6 +601,11 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 		    continue;
 
 		if (mp->wlen < 0) {
+		    /* `*'-pattern. */
+		    /*
+		     * Similar to the identically-named variable in the 'else' block.
+		     */
+		    int t;
 		    /* 
 		     * 1 iff the anchor and the word are on the same side of
 		     * the line pattern; that is: if either
@@ -856,6 +859,10 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 		    break;
 		} else if (ll >= mp->llen && lw >= mp->wlen) {
 		    /* Non-`*'-pattern. */
+		    /*
+		     * Similar to the identically-named variable in the 'if' block.
+		     */
+		    int t = 1;
 		    char *tl, *tw;
 		    int tll, tlw, til, tiw;
 

[-- Attachment #11: 0010-internals-match_str-Simplify-expression.patch --]
[-- Type: text/x-diff, Size: 1812 bytes --]

>From d5b919941b4d20641cafd9ff7d329a80d144126c Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 14 Sep 2016 03:38:35 +0000
Subject: [PATCH 10/10] internals: match_str: Simplify expression.

In the first hunk we actually know that ind==0 since sfx==0, but keep it identical
to the last hunk.

Also add a comment (unrelated).
---
 Src/Zle/compmatch.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
index 1d4bef8..193b105 100644
--- a/Src/Zle/compmatch.c
+++ b/Src/Zle/compmatch.c
@@ -568,8 +568,7 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 	bslash = 0;
 	if (!sfx && lw && (!part || test) &&
 	    (l[ind] == w[ind] ||
-	     (bslash = (lw > 1 && w[ind] == '\\' &&
-			(ind ? (w[0] == l[0]) : (w[1] == l[0])))))) {
+	     (bslash = (lw > 1 && w[ind] == '\\' && w[ind+1] == l[0])))) {
 	    /* No matcher could be used, but the strings have the same
 	     * character here, skip over it. */
 	    l += add; w += (bslash ? (add + add) : add);
@@ -849,8 +848,10 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 
 		    if (!llen && !alen) {
 			lm = mp;
-			if (he)
+			if (he) {
+			    /* Signal the outer for loop to continue. */
 			    mp = NULL;
+			}
 			else
 			    he = 1;
 		    } else {
@@ -989,8 +990,7 @@ match_str(char *l, char *w, Brinfo *bpp, int bc, int *rwlp,
 	bslash = 0;
 	if ((!test || sfx) && lw &&
 	    (l[ind] == w[ind] ||
-	     (bslash = (lw > 1 && w[ind] == '\\' &&
-			(ind ? (w[0] == l[0]) : (w[1] == l[0])))))) {
+	     (bslash = (lw > 1 && w[ind] == '\\' && w[ind+1] == l[0])))) {
 	    /* No matcher could be used, but the strings have the same
 	     * character here, skip over it. */
 	    l += add; w += (bslash ? (add + add ) : add);

Re: legend for match_str (was: Re: file completion(?) erases word typed)

[Bart Schaefer]

On Sep 14,  3:49am, Daniel Shahaf wrote:
}
} I'm planning to commit this with distinct X-Seq numbers such as
} 39309/{0001..0011} - didn't want to send 11 separate emails for such
} trivial changes.

Request before you commit this:  Line-wrap the comments before column 80.

Re: file completion(?) erases word typed

[Bart Schaefer]

I think the comment included in the diff is fairly explanatory.  This
change means menu completion sometimes starts earlier than expected
(i.e., on the first or second tab instead of on the second or third)
but that seems preferable to throwing away the set of matches and
starting over at what is probably the wrong place.

It's likely that there are still situations -- probably including
_multi_parts with separators other than "/" -- where this heuristic
fails, but this should catch a lot of common cases (I hope).

If I'm wrong about any of the assertions in that comment, please
point out how the tests could be tweaked.


diff --git a/Completion/Base/Core/_main_complete b/Completion/Base/Core/_main_complete
index c292ce7..01e6eaa 100644
--- a/Completion/Base/Core/_main_complete
+++ b/Completion/Base/Core/_main_complete
@@ -284,6 +284,29 @@ if [[ $compstate[old_list] = keep || nm -gt 1 ]]; then
     fi
   fi
 
+  if [[ "$compstate[insert]" = *unambiguous &&
+	"$compstate[old_list]" != keep &&
+	"$compstate[nmatches]" -gt 1 &&
+	( "$compstate[insert_positions]" != *:* ||
+	  "$compstate[unambiguous_positions]" == 0* ) &&
+	( "$compstate[unambiguous]" == /## ||
+	  "$compstate[unambiguous_cursor]" -lt ${#PREFIX} ) ]]; then
+    #
+    # We have multiple matches but no useful common prefix to show.
+    # This would result in either erasing an existing prefix on the
+    # line, or adding a prefix that breaks subsequent completions.
+    # Instead, forcibly enter menu completion to cycle the matches.
+    #
+    # The above test is heuristic; what we really need to know
+    # is whether compadd -U or -M is going to cause the current
+    # string on the line to be shortened or removed, but that
+    # information isn't present in the user-visible state.  The
+    # test for unambiguous == /## ideally would apply only for
+    # file completion, but there's no way to detect that either.
+    #
+    compstate[insert]=menu
+  fi
+
   if [[ "$compstate[insert]" = *menu* ]]; then
     [[ "$MENUSELECT" = 00 ]] && MENUSELECT=0
     if [[ -n "$_menu_style[(r)no-select*]" ]]; then