From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8460 invoked by alias); 10 Mar 2016 23:20:57 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 38129 Received: (qmail 8195 invoked from network); 10 Mar 2016 23:20:56 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham autolearn_force=no version=3.4.1 Date: Thu, 10 Mar 2016 23:20:54 +0000 From: Daniel Shahaf To: "Jun T." Cc: zsh-workers@zsh.org Subject: __git_ignore_line positional argument (un)escaping (was: Re: _git-reset doesn't complete newly staged files) Message-ID: <20160310232054.GA10995@tarsus.local2> References: <20160309132444.GA2428@tarsus.local2> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) 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 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 > this completes a\ b.txt, and > % git add a\ b.txt > 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 > 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 » from «git add f[a-z]o ». 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 » 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...