Re: [PATCH] squash! completion: fill COMPREPLY directly when completing refs

[Junio C Hamano]

SZEDER Gábor  writes:

> On Fri, Feb 10, 2017 at 10:44 PM, Junio C Hamano  wrote:
>
>> Should I expect a reroll to come, or is this the only fix-up to the
>> series that begins at <20170203025405.8242-1-szeder@gmail.com>?
>>
>> No hurries.
>
> Yes, definitely.
>
> I found a minor bug in the middle of the series, and haven't quite
> made up my mind about it and its fix yet.  Perhaps in a day or three.
>
> Regarding the 'strip' -> 'lstrip' 'for-each-ref' format modifier
> rename that broke pu: I should keep using 'strip', right?

Right.  I view the removal of 'strip' as an accident when 'lstrip'
was introduced, not an intended change.

Re: [PATCH] squash! completion: fill COMPREPLY directly when completing refs

[SZEDER Gábor]

On Fri, Feb 10, 2017 at 10:44 PM, Junio C Hamano  wrote:

> Should I expect a reroll to come, or is this the only fix-up to the
> series that begins at <20170203025405.8242-1-szeder@gmail.com>?
>
> No hurries.

Yes, definitely.

I found a minor bug in the middle of the series, and haven't quite
made up my mind about it and its fix yet.  Perhaps in a day or three.

Regarding the 'strip' -> 'lstrip' 'for-each-ref' format modifier
rename that broke pu: I should keep using 'strip', right?

Gábor

Re: [PATCH] squash! completion: fill COMPREPLY directly when completing refs

[Junio C Hamano]

SZEDER Gábor  writes:

> Care should be taken, though, because that prefix might contain
> 'for-each-ref' format specifiers as part of the left hand side of a
> '..' range or '...' symmetric difference notation or fetch/push/etc.
> refspec, e.g. 'git log "evil-%(refname)..br'.  Doubling every '%'
> in the prefix will prevent 'git for-each-ref' from interpolating any
> of those contained format specifiers.
> ---
>
> This is really pathological, and I'm sure this has nothing to do
> with whatever breakage Jacob experienced.  The shell
> metacharacters '(' and ')' still cause us trouble in various ways,
> but that's nothing new and has been the case for quite a while
> (always?).
>
> It's already incorporated into (the rewritten)
>
>   https://github.com/szeder/git completion-refs-speedup

Should I expect a reroll to come, or is this the only fix-up to the
series that begins at <20170203025405.8242-1-szeder@gmail.com>?

No hurries.

[PATCH] squash! completion: fill COMPREPLY directly when completing refs

[SZEDER Gábor]

Care should be taken, though, because that prefix might contain
'for-each-ref' format specifiers as part of the left hand side of a
'..' range or '...' symmetric difference notation or fetch/push/etc.
refspec, e.g. 'git log "evil-%(refname)..br'.  Doubling every '%'
in the prefix will prevent 'git for-each-ref' from interpolating any
of those contained format specifiers.
---

This is really pathological, and I'm sure this has nothing to do with
whatever breakage Jacob experienced.
The shell metacharacters '(' and ')' still cause us trouble in various
ways, but that's nothing new and has been the case for quite a while
(always?).

It's already incorporated into (the rewritten)

  https://github.com/szeder/git completion-refs-speedup

 contrib/completion/git-completion.bash |  8 +---
 t/t9902-completion.sh  | 11 +++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index dbbb62f5f..058f1d0ee 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -381,6 +381,7 @@ __git_refs ()
local list_refs_from=path remote="${1-}"
local format refs
local pfx="${3-}" cur_="${4-$cur}" sfx="${5-}"
+   local fer_pfx="${pfx//\%/%%}"
 
__git_find_repo_path
dir="$__git_repo_path"
@@ -406,6 +407,7 @@ __git_refs ()
if [ "$list_refs_from" = path ]; then
if [[ "$cur_" == ^* ]]; then
pfx="$pfx^"
+   fer_pfx="$fer_pfx^"
cur_=${cur_#^}
fi
case "$cur_" in
@@ -429,13 +431,13 @@ __git_refs ()
"refs/remotes/$cur_*" "refs/remotes/$cur_*/**")
;;
esac
-   __git_dir="$dir" __git for-each-ref 
--format="$pfx%($format)$sfx" \
+   __git_dir="$dir" __git for-each-ref 
--format="$fer_pfx%($format)$sfx" \
"${refs[@]}"
if [ -n "$track" ]; then
# employ the heuristic used by git checkout
# Try to find a remote branch that matches the 
completion word
# but only output if the branch name is unique
-   __git for-each-ref 
--format="$pfx%(refname:strip=3)$sfx" \
+   __git for-each-ref 
--format="$fer_pfx%(refname:strip=3)$sfx" \
--sort="refname:strip=3" \
"refs/remotes/*/$cur_*" 
"refs/remotes/*/$cur_*/**" | \
uniq -u
@@ -457,7 +459,7 @@ __git_refs ()
case "HEAD" in
$cur_*) echo "${pfx}HEAD$sfx" ;;
esac
-   __git for-each-ref 
--format="$pfx%(refname:strip=3)$sfx" \
+   __git for-each-ref 
--format="$fer_pfx%(refname:strip=3)$sfx" \
"refs/remotes/$remote/$cur_*" \
"refs/remotes/$remote/$cur_*/**"
else
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 1206a38ed..be584c069 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -951,6 +951,17 @@ test_expect_success 'teardown after filtering matching 
refs' '
git update-ref -d refs/remotes/other/matching/branch-in-other
 '
 
+test_expect_success '__git_refs - for-each-ref format specifiers in prefix' '
+   cat >expected <<-EOF &&
+   evil-%%-%42-%(refname)..master
+   EOF
+   (
+   cur="evil-%%-%42-%(refname)..mas" &&
+   __git_refs "" "" "evil-%%-%42-%(refname).." mas >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
 test_expect_success '__git_complete_refs - simple' '
sed -e "s/Z$//g" >expected <<-EOF &&
HEAD Z
-- 
2.11.1.499.gb6fcc8b3a