Re: [RFC] Case-insensitive path completion in _git

[dana <dana@xxxxxxx>]

On 28 Mar 2021, at 03:08, Oliver Kiddle <opk@xxxxxxx> wrote:
> That applies to the whole path including the directory components which
> I don't thing we'd want in this case - but correct me if I'm wrong. ...
> The only reference to the feature I could find was in the top-level
> git.1 man page so maybe the syntax is different.

I found the syntax description under the 'pathspec' section in gitglossary(7).

The documentation doesn't say one way or the other, but this suggests that
icase *doesn't* apply to the whole path:

  % git ls-files -- ":(icase)${(U)PWD}/*Kbuild*"
  fatal: Invalid path '/HOME': No such file or directory

And i found this in git's dir.c, which seems to confirm:

  * Suppose the pathspec is 'foo' and '../bar' running from
  * subdir 'xyz'. The common prefix at #1 will be empty, thanks
  * to "../". We may have xyz/foo _and_ XYZ/foo after #2. The
  * user does not want XYZ/foo, only the "foo" part should be
  * case-insensitive. We need to filter out XYZ/foo here. In
  * other words, we do not trust the caller on comparing the
  * prefix part when :(icase) is involved. We do exact
  * comparison ourselves.

On 28 Mar 2021, at 03:08, Oliver Kiddle <opk@xxxxxxx> wrote:
> That doesn't look too bad at all but I tend not to work in big trees
> like the Linux kernel. :(icase) may be worse for a deeper path where
> it adds extra directory matches. Did you run these benchmarks on a
> case-sensitive or insensitive file-system?

Case-sensitive (ext4). Not sure if it's still like this, but you used to get
unresolvable conflicts if you checked out the Linux source on a
case-insensitive file system...

On 28 Mar 2021, at 03:08, Oliver Kiddle <opk@xxxxxxx> wrote:
> I don't think thats necessary. Committing as per the original patch
> seems good to me.

Thanks.

The only other thing i can think of that's worth mentioning is that the first
ls-files call may now return unusable results that nonetheless bypass the
second call. e.g., if you're using case-sensitive matching, and you try to
complete Foo, but you only find foo, the latter will be used to populate
$files, which will bypass the second ls-files call. I don't know if or when
this would be an issue, but it is different.

If anyone can think of a problem with that, please let me know. Otherwise i'll
commit the following in a few days probably

dana


diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index ced89b501..0267acfa8 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -7155,11 +7155,16 @@ __git_files () {
   local pref=${(Q)${~PREFIX}}
   [[ $pref[1] == '/' ]] || pref=$gittoplevel$gitprefix$pref
 
-  # First allow ls-files to pattern-match in case of remote repository
-  files=(${(0)"$(_call_program files git ls-files -z --exclude-standard ${(q)opts} -- ${(q)${pref:+$pref\*}:-.} 2>/dev/null)"})
+  # First allow ls-files to pattern-match in case of remote repository. Use the
+  # icase pathspec magic word to ensure that we support case-insensitive path
+  # completion for users with the appropriate matcher configuration
+  files=(${(0)"$(_call_program files git ls-files -z --exclude-standard ${(q)opts} -- ${(q)${pref:+:\(icase\)$pref\*}:-.} 2>/dev/null)"})
   __git_command_successful $pipestatus || return
 
-  # If ls-files succeeded but returned nothing, try again with no pattern
+  # If ls-files succeeded but returned nothing, try again with no pattern. Note
+  # that ls-files defaults to the CWD if not given a path, so if the file we
+  # were trying to add is in an *adjacent* directory, this won't return anything
+  # helpful either
   if [[ -z "$files" && -n "$pref" ]]; then
     files=(${(0)"$(_call_program files git ls-files -z --exclude-standard ${(q)opts} -- 2>/dev/null)"})
     __git_command_successful $pipestatus || return