Re: Bug: cd auto-completion of .. fails with parentheses in directory name

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Georg Nebehay wrote on Wed, Sep 21, 2016 at 08:12:06 +0200:
> there appears to be a bug in cd autocompletion. Please find the details
> here: https://github.com/robbyrussell/oh-my-zsh/issues/5424

(For future reference, please restate the issue in the email, don't just
link to an external description.  Among other reasons: because github
may shut down some day.)

I can reproduce this with latest master:
.
    % cd $(mktemp -d)
    % mkdir -p 'A(B)/C'
    % cd $_
    % cd ../<TAB>
.
completes nothing.

This is due to the following bit in _path_files:

   463	    elif [[ "$sopt" = *[/f]* ]]; then
   464	      compfiles -p$cfopt tmp1 accex "$skipped" "$_matcher $matcher[2]" "$sdirs" fake "$pats[@]"
   465	    else
   466	      compfiles -p$cfopt tmp1 accex "$skipped" "$_matcher $matcher[2]" '' fake "$pats[@]"
   467	    fi
   468	    tmp1=( $~tmp1 ) 2> /dev/null

At entry to this code, tmp1=( $PWD ).  compfiles changes that to
tmp1=( '/tmp/tmp.elGZzgGNwi/A(B)/*(-/)' ), and line 468 the parentheses
are interpreted as grouping characters.

Now, bin_compfiles() calls cf_pats() calls cfp_test_exact(), which has
a docstring, which says the values originating from $tmp1 are filenames.
Not patterns.  This means it is up to bin_compfiles() or its callees to
quote the literal parentheses in the input.

So perhaps this? —

[[[
diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index 16b681c..27b78cd 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -4830,8 +4830,11 @@ cf_remove_other(char **names, char *pre, int *amb)
  *     3. compfiles -P  parnam1 parnam2 skipped matcher sdirs parnam3 
  *
  *     1. Set parnam1 to an array of patterns....
+ *        ${(P)parnam1} is an in/out parameter.
  *     2. Like #1 but without calling cfp_opt_pats().  (This is only used by _approximate.)
  *     3. Like #1 but varargs is implicitly set to  char *varargs[2] = { "*(-/)", NULL };.
+ *
+ *     parnam2 has to do with the accept-exact style (see cfp_test_exact()).
  */
 
 static int
@@ -4866,7 +4869,7 @@ bin_compfiles(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 		return 0;
 	    }
 	    for (l = newlinklist(); *tmp; tmp++)
-		addlinknode(l, *tmp);
+		addlinknode(l, quotestring(*tmp, QT_BACKSLASH_PATTERN));
 	    set_list_array(args[1], cf_pats((args[0][1] == 'P'), !!args[0][2],
 					    l, getaparam(args[2]), args[3],
 					    args[4], args[5],
]]]

Questions:

1) Is the lifetime correct?  I'm not sure whether elemnts of 'l' should
be malloc()ed or heap allocated.

2) Should quoting be added in bin_compfiles() or at a later point during
cf_pats()?  Although the docstring of cfp_test_exact() says the elements
of 'l' are filenames, they are then passed to ztat() which ignores
backslashes, so it's not clear to me what quoting is expected where.

(I haven't checked whether the rest of cf_pats() expects the elements of
'l' to be quoted or not.)

Thanks for the bug report.

Cheers,

Daniel

> Regards,
> Georg