Re: [PATCH] completion: fix shell expansion of items
On Thu, Sep 20, 2012 at 8:21 PM, Jeff King wrote: > On Thu, Sep 20, 2012 at 08:11:52PM +0200, SZEDER Gábor wrote: > >> > > In order to achieve that I took bash-completion's quote() function, >> > > which is rather simple, and renamed it to __git_quote() as per Jeff >> > > King's suggestion. >> > > >> > > Solves the original problem for me. >> > >> > Me too. Thanks. >> >> While it solves the original problem, it seems to break refs >> completion, as demonstrated by the following POC test: >> >> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh >> index 92d7eb47..fab63b95 100755 >> --- a/t/t9902-completion.sh >> +++ b/t/t9902-completion.sh >> @@ -228,4 +228,11 @@ test_expect_success 'general options plus command' ' >> test_completion "git --no-replace-objects check" "checkout " >> ' >> >> +test_expect_success 'basic refs completion' ' >> + touch file && >> + git add file && >> + git commit -m initial && >> + test_completion "git branch m" "master " >> +' > > Hmm. I notice that Felipe's patch wraps the _whole_ input to > __gitcomp_nl in single quotes. Wasn't there a patch series that added tests for __gitcomp_nl to catch these issues? -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: fix shell expansion of items
On Thu, Sep 20, 2012 at 08:11:52PM +0200, SZEDER Gábor wrote: > > > In order to achieve that I took bash-completion's quote() function, > > > which is rather simple, and renamed it to __git_quote() as per Jeff > > > King's suggestion. > > > > > > Solves the original problem for me. > > > > Me too. Thanks. > > While it solves the original problem, it seems to break refs > completion, as demonstrated by the following POC test: > > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh > index 92d7eb47..fab63b95 100755 > --- a/t/t9902-completion.sh > +++ b/t/t9902-completion.sh > @@ -228,4 +228,11 @@ test_expect_success 'general options plus command' ' > test_completion "git --no-replace-objects check" "checkout " > ' > > +test_expect_success 'basic refs completion' ' > + touch file && > + git add file && > + git commit -m initial && > + test_completion "git branch m" "master " > +' Hmm. I notice that Felipe's patch wraps the _whole_ input to __gitcomp_nl in single quotes. So if there are multiple completions we would end up with: 'one two quo\'ted three' I wonder if that is OK to feed to compgen -W, or if it wants to expand it line-by-line. Just guessing at this point, though. -Peff > + > test_done > -- > 1.7.12.1.438.g7dfa67b > > > which fails with: > > --- expected2012-09-20 18:05:23.857752925 + > +++ out 2012-09-20 18:05:23.877752925 + > @@ -1 +1 @@ > -master > + > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: fix shell expansion of items
Hi,
On Wed, Sep 19, 2012 at 09:46:08PM -0400, Jeff King wrote:
> On Thu, Sep 20, 2012 at 04:15:15AM +0200, Felipe Contreras wrote:
>
> > As reported by Jeroen Meijer[1]; the current code doesn't deal properly
> > with items (tags, branches, etc.) that have ${} in them because they get
> > expaned by bash while using compgen.
> >
> > A simple solution is to quote the items so they get expanded properly
> > (\$\{\}).
> >
> > In order to achieve that I took bash-completion's quote() function,
> > which is rather simple, and renamed it to __git_quote() as per Jeff
> > King's suggestion.
> >
> > Solves the original problem for me.
>
> Me too. Thanks.
While it solves the original problem, it seems to break refs
completion, as demonstrated by the following POC test:
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 92d7eb47..fab63b95 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -228,4 +228,11 @@ test_expect_success 'general options plus command' '
test_completion "git --no-replace-objects check" "checkout "
'
+test_expect_success 'basic refs completion' '
+ touch file &&
+ git add file &&
+ git commit -m initial &&
+ test_completion "git branch m" "master "
+'
+
test_done
--
1.7.12.1.438.g7dfa67b
which fails with:
--- expected2012-09-20 18:05:23.857752925 +
+++ out 2012-09-20 18:05:23.877752925 +
@@ -1 +1 @@
-master
+
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: fix shell expansion of items
Jeff King writes:
> On Thu, Sep 20, 2012 at 04:15:15AM +0200, Felipe Contreras wrote:
>
>> As reported by Jeroen Meijer[1]; the current code doesn't deal properly
>> with items (tags, branches, etc.) that have ${} in them because they get
>> expaned by bash while using compgen.
>>
>> A simple solution is to quote the items so they get expanded properly
>> (\$\{\}).
>>
>> In order to achieve that I took bash-completion's quote() function,
>> which is rather simple, and renamed it to __git_quote() as per Jeff
>> King's suggestion.
>>
>> Solves the original problem for me.
>
> Me too. Thanks.
Thanks, both. Will queue.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: fix shell expansion of items
On Thu, Sep 20, 2012 at 04:15:15AM +0200, Felipe Contreras wrote:
> As reported by Jeroen Meijer[1]; the current code doesn't deal properly
> with items (tags, branches, etc.) that have ${} in them because they get
> expaned by bash while using compgen.
>
> A simple solution is to quote the items so they get expanded properly
> (\$\{\}).
>
> In order to achieve that I took bash-completion's quote() function,
> which is rather simple, and renamed it to __git_quote() as per Jeff
> King's suggestion.
>
> Solves the original problem for me.
Me too. Thanks.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] completion: fix shell expansion of items
As reported by Jeroen Meijer[1]; the current code doesn't deal properly
with items (tags, branches, etc.) that have ${} in them because they get
expaned by bash while using compgen.
A simple solution is to quote the items so they get expanded properly
(\$\{\}).
In order to achieve that I took bash-completion's quote() function,
which is rather simple, and renamed it to __git_quote() as per Jeff
King's suggestion.
Solves the original problem for me.
[1] http://article.gmane.org/gmane.comp.version-control.git/201596
Signed-off-by: Felipe Contreras
---
contrib/completion/git-completion.bash |9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index d743e56..5a5b5a0 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -225,6 +225,13 @@ _get_comp_words_by_ref ()
fi
fi
+# Quotes the argument for shell reuse
+__git_quote()
+{
+ local quoted=${1//\'/\'\\\'\'}
+ printf "'%s'" "$quoted"
+}
+
# Generates completion reply with compgen, appending a space to possible
# completion words, if necessary.
# It accepts 1 to 4 arguments:
@@ -261,7 +268,7 @@ __gitcomp ()
__gitcomp_nl ()
{
local IFS=$'\n'
- COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
+ COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$(__git_quote "$1")" --
"${3-$cur}"))
}
__git_heads ()
--
1.7.10.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
