zsh-workers
 help / color / mirror / Atom feed
* [RFC] Case-insensitive path completion in _git
@ 2020-12-13  4:40 dana
  2020-12-13 13:55 ` m0viefreak
  0 siblings, 1 reply; 3+ messages in thread
From: dana @ 2020-12-13  4:40 UTC (permalink / raw)
  To: Zsh hackers list

I've had this sitting for a while:

_git has issues completing files case-insensitively (if you have matcher-list
'm:{a-zA-Z}={A-Za-z}' or whatever). Looking into it, i found that __git_files
is trying to pass a glob pattern to `git ls-files`, and this fails if there's
not an exact case match, since ls-files is always case-sensitive.

There is a fall-back to `git ls-files` with no path, but this doesn't always
work either, because it defaults to the CWD, and the file you're trying to
complete may not be under the CWD. Even when the fall-back succeeds, it's not
ideal, because it'll pass every single file in the tree to _multi_parts, which
can be slow.

The following hack solves the problem for me, but it might be too silly to
commit. Can anyone think of a more proper fix? If not, would the hack be
viable (probably gated behind a style)?

dana


diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 81a060e4d..7d0201efe 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -7138,6 +7138,7 @@ __git_files_relative () {
 (( $+functions[__git_files] )) ||
 __git_files () {
   local compadd_opts opts tag description gittoplevel gitprefix files expl
+  local MATCH MBEGIN MEND
 
   zparseopts -D -E -a compadd_opts V+: J+: 1 2 o+: n f x+: X+: M+: P: S: r: R: q F:
   zparseopts -D -E -a opts -- -cached -deleted -modified -others -ignored -unmerged -killed x+: --exclude+:
@@ -7152,14 +7153,26 @@ __git_files () {
 
   # TODO: --directory should probably be added to $opts when --others is given.
 
+  # ls-files allows the part of the path under the repository directory to be a
+  # glob pattern. However, these patterns are always case-sensitive (even if the
+  # file system is not). This means that if we have a matcher style set to make
+  # path completion case-insensitive, it won't (necessarily) work, because we
+  # won't get any results back from Git. To work around this, we can transform
+  # the pattern from abc to [Aa][Bb][Cc]. This is very dumb, but seemingly
+  # reliable enough
   local pref=${(Q)${~PREFIX}}
+  pref=${pref//(#m)[[:alpha:]]/\[${(U)MATCH}${(L)MATCH}\]}
   [[ $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)"})
   __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. (If it did, the case-sensitivity issue mentioned above would only
+  # be a minor performance issue rather than a complete failure to return)
   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



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

* Re: [RFC] Case-insensitive path completion in _git
  2020-12-13  4:40 [RFC] Case-insensitive path completion in _git dana
@ 2020-12-13 13:55 ` m0viefreak
  2020-12-14 10:25   ` dana
  0 siblings, 1 reply; 3+ messages in thread
From: m0viefreak @ 2020-12-13 13:55 UTC (permalink / raw)
  To: dana, Zsh hackers list



On 13.12.2020 05:40, dana wrote:
> _git has issues completing files case-insensitively (if you have matcher-list
> 'm:{a-zA-Z}={A-Za-z}' or whatever). Looking into it, i found that __git_files
> is trying to pass a glob pattern to `git ls-files`, and this fails if there's
> not an exact case match, since ls-files is always case-sensitive.

> There is a fall-back to `git ls-files` with no path, but this doesn't always
> work either, because it defaults to the CWD, and the file you're trying to
> complete may not be under the CWD. Even when the fall-back succeeds, it's not
> ideal, because it'll pass every single file in the tree to _multi_parts, which
> can be slow.
> 
> The following hack solves the problem for me, but it might be too silly to
> commit. Can anyone think of a more proper fix? If not, would the hack be
> viable (probably gated behind a style)?
> 
> dana
>
> ...
> +  pref=${pref//(#m)[[:alpha:]]/\[${(U)MATCH}${(L)MATCH}\]}
> ...

This solution only handles simple upper-lower-case conversion matcher-list styles,
but it will not work for more complicated styles like 'r:|[._-]=* r:|=*'.
That's probably fine for most use-cases.

I have been running with the following patch applied locally for quite some time:


diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 81a060e4d..1a6619e31 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -7153,7 +7153,7 @@ __git_files () {
    # TODO: --directory should probably be added to $opts when --others is given.

    local pref=${(Q)${~PREFIX}}
-  [[ $pref[1] == '/' ]] || pref=$gittoplevel$gitprefix$pref
+  [[ $pref[1] == '/' ]] || pref=$gittoplevel$gitprefix

    # 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)"})


It just lets _multi_parts do the matching.
Performance-wise, it's obviously worse than than your proposed solution, but
still better than the fallback, since it still passes $gitprefix to ls-files,
which should also avoid the CWD problem.

The trigger of the fallback is broken anyways, isn't it?

>  # If ls-files succeeded but returned nothing, try again with no pattern
>  if [[ -z "$files" && -n "$pref" ]]; then

It just considers the case where no match was found at all, but it does not
handle the case where *some* matches were found, but not all.



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

* Re: [RFC] Case-insensitive path completion in _git
  2020-12-13 13:55 ` m0viefreak
@ 2020-12-14 10:25   ` dana
  0 siblings, 0 replies; 3+ messages in thread
From: dana @ 2020-12-14 10:25 UTC (permalink / raw)
  To: m0viefreak; +Cc: Zsh hackers list

On 13 Dec 2020, at 07:55, m0viefreak <m0viefreak.cm@googlemail.com> wrote:
> This solution only handles simple upper-lower-case conversion matcher-list styles,
> but it will not work for more complicated styles like 'r:|[._-]=* r:|=*'.
> That's probably fine for most use-cases.

That's true.

On 13 Dec 2020, at 07:55, m0viefreak <m0viefreak.cm@googlemail.com> wrote:
> Performance-wise, it's obviously worse than than your proposed solution, but
> still better than the fallback, since it still passes $gitprefix to ls-files,
> which should also avoid the CWD problem.

Are you sure? It seems to me that $gittoplevel$gitprefix is normally the CWD,
so you'd just be running `git ls-files /path/to/cwd/*`, which will never match
a PREFIX like `../foo`.

I guess we could make it so that it retains any leading segments in PREFIX,
but then you have the same two issues again (those segments have to be matched
case-insensitively, and it doesn't account for more complex matcher-list
styles).

I also just realised that we can't match stuff like `./foo` or `.//foo` or
`../cwd/../`, since `git ls-files` elides unnecessary slashes/segments in its
output.

On 13 Dec 2020, at 07:55, m0viefreak <m0viefreak.cm@googlemail.com> wrote:
> The trigger of the fallback is broken anyways, isn't it? ...
> It just considers the case where no match was found at all, but it does not
> handle the case where *some* matches were found, but not all.

As it is in the repo, yes, actually. If you have like foobar and Foobaz, then
foo<TAB> will return only the former without triggering the fall-back.

dana



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

end of thread, other threads:[~2020-12-14 10:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-13  4:40 [RFC] Case-insensitive path completion in _git dana
2020-12-13 13:55 ` m0viefreak
2020-12-14 10:25   ` dana

zsh-workers

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/zsh-workers

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 zsh-workers zsh-workers/ http://inbox.vuxu.org/zsh-workers \
		zsh-workers@zsh.org
	public-inbox-index zsh-workers

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


code repositories for the project(s) associated with this inbox:

	https://git.vuxu.org/mirror/zsh/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git