Zsh Mailing List Archive
Messages sorted by:
Reverse Date,
Date,
Thread,
Author
__git_ignore_line positional argument (un)escaping (was: Re: _git-reset doesn't complete newly staged files)
I wrote this before I saw Mikael's post, but I'm sending it anyway,
since it may still be relevant. [Search for "bQ" for the
non-_git-specific discussion.]
Jun T. wrote on Thu, Mar 10, 2016 at 21:03:59 +0900:
> On 2016/03/09, at 22:24, Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> > I think you're looking for «git diff-index --cached -z --name-only
> > $treeish».
>
> What I was not sure was whether to offer conflicting files or not.
> 'git reset conflicting_file' leaves the working tree unmodified
> (so '<<<<<<< HEAD' etc. remain there) while the index becomes clean
> (conflict fixed) and you can commit it. I thought this would not
> be useful.
>
> But it is not an error anyway, and maybe there are someone who
> really want to do that (if they are going to run 'git reset' while
> there is a conflict, they may well know what they are doing).
>
Agreed: it wouldn't be a typical workflow, but it's admitted by the
tool.
I lean on the side of just offering all valid completions, including
conflicted files. That:
- Keeps the completion interface simpler, with fewer exceptions for
users to have to know about;
- Leaves issueing warnings about inadvisable usages to git(1) itself,
which is better placed than compsys to decide when such warnings are
warranted.
> > I took the liberty of changing the tag and description you used, since
> > "staged files" isn't exactly right for this syntax: files that are in
> > $treeish but not staged may also be completed.
>
> I thought those files are 'staged for delete', but your more
> descriptive tag/description would be better.
>
Okay, then I'll keep them when I commit that patch.
> > Incidentally, it would be nice to have a docstring for __git_ignore_line:
>
> Yes, and
>
Did you intend to type more words in that sentence?
> > Why does __git_ignore_line escape some characters with backslashes
> > before adding them to $ignored?
>
> I'm rather confused. Isn't it better to UN-quote them?
>
> % touch 'a b.txt'
> % git add <TAB>
> this completes a\ b.txt, and
> % git add a\ b.txt <TAB>
> this still completes a\ b.txt.
>
> Using
> ignored+=(${(Q)line[1,CURRENT-1]})
> ignored+=(${(Q)line[CURRENT+1,-1]})
> fixes this.
>
> Moreover,
>
> % git add *.txt <TAB>
> this does not complete a\ b.txt anymore.
>
> Or there are something I've overlooked.
With just ${(Q)line[...}}, you lose the ability to distinguish «git add
f\[a-z\]o <TAB>» from «git add f[a-z]o <TAB>». It took me a while to
find an incantation that works, but I think «${(bQ)line[...}}» is it:
% ls -1
[f]oo
f[a-z]o
foo
% () { eval "echo ${(bQ)1}" } '\[f\]oo'
[f]oo
% () { eval "echo ${(bQ)1}" } '[f]oo'
foo
% () { eval "echo ${(bQ)1}" } 'f[a-z]o'
foo
% () { eval "echo ${(bQ)1}" } 'f\[a-z\]o'
f[a-z]o
% ls -1
bar.txt
baz.txt
% () { eval "echo ${(bQ)1}" } '*.txt'
bar.txt baz.txt
(The purpose of the "eval" is to force the result of parameter expansion
to be interpreted as a pattern, since elements of $ignored are
interpreted by compadd as patterns.)
With the argument to 'compadd -F' constructed using ${(bQ)}, all these
patterns do the right thing: they ignore exactly the files the anonymous
function prints and no others. For example, «git add f[a-z]o <TAB>»
offers «f\[a-z\]o» but not «foo», and so on.
So, three questions:
1. Should regression tests be added for these uses of ${(bQ)}?
2. The ignore-line currently uses the following escaping:
.
# Completion/Base/Core/_description
50 if zstyle -s ":completion:${curcontext}:$1" ignore-line hidden; then
51 local -a qwords
52 qwords=( ${words//(#m)[\[\]()\\*?#<>~\^\|]/\\$MATCH} )
.
Should the ignore-line style use ${(bQ)}?
3. Is the following patch correct?
diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 2dd9a80..e063a6f 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -4961,7 +4961,7 @@ __git_committish_range_last () {
(( $+functions[__git_pattern_escape] )) ||
__git_pattern_escape () {
- print -r -n ${1//(#m)[\[\]()\\*?#<>~\^]/\\$MATCH}
+ print -r -n - ${(b)1}
}
(( $+functions[__git_is_type] )) ||
@@ -5053,8 +5053,8 @@ __git_ignore_line () {
declare -a ignored
ignored=()
((CURRENT > 1)) &&
- ignored+=(${line[1,CURRENT-1]//(#m)[\[\]()\\*?#<>~\^]/\\$MATCH})
+ ignored+=(${(bQ)line[1,CURRENT-1]})
((CURRENT < $#line)) &&
- ignored+=(${line[CURRENT+1,-1]//(#m)[\[\]()\\*?#<>~\^]/\\$MATCH})
+ ignored+=(${(bQ)line[CURRENT+1,-1]})
$* -F ignored
}
?
Cheers,
Daniel
P.S. I suppose a tree-wide grep for more of those hand-rolled
implementations of ${(b)}/${(q)} wouldn't be a bad thing...
Messages sorted by:
Reverse Date,
Date,
Thread,
Author