From: dana <dana@dana.is>
To: Oliver Kiddle <opk@zsh.org>
Cc: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: [RFC] Case-insensitive path completion in _git
Date: Tue, 30 Mar 2021 00:59:06 -0500 [thread overview]
Message-ID: <850A9481-BE4A-4EED-A43A-129A2C3EB1E7@dana.is> (raw)
In-Reply-To: <59566-1616918897.215402@6qHq.M4f6.Ov84>
On 28 Mar 2021, at 03:08, Oliver Kiddle <opk@zsh.org> 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@zsh.org> 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@zsh.org> 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
next prev parent reply other threads:[~2021-03-30 5:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-13 4:40 dana
2020-12-13 13:55 ` m0viefreak
2020-12-14 10:25 ` dana
2021-03-27 21:08 ` Daniel Shahaf
2021-03-28 5:07 ` dana
2021-03-28 8:08 ` Oliver Kiddle
2021-03-30 5:59 ` dana [this message]
2021-04-10 20:43 ` Lawrence Velázquez
2021-04-10 20:56 ` dana
2021-03-27 20:06 ` Lawrence Velázquez
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=850A9481-BE4A-4EED-A43A-129A2C3EB1E7@dana.is \
--to=dana@dana.is \
--cc=opk@zsh.org \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).