threads / patch / 30563
Subject: [PATCH 1/2] completion: rename _git and _gitk
Felipe Contreras[PATCH 0/2] completion: backwards compatibility fix
Hi,
These two patches are meant to fix the backwards compatibility of _git, and _gitk. This also helps my zsh's wrapper.
Felipe Contreras (2): completion: rename _git and _gitk completion: add support for backwards compatibilit
contrib/completion/git-completion.bash | 22 +++++++++++++++++----- t/t9902-completion.sh | 2 +- 2 files changed, 18 insertions(+), 6 deletions(-)
-- 1.7.10.2
Felipe ContrerasWould be useful to provide backwards compatibility for _git. Also, zsh completion uses _git, and it cannot be changed.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- contrib/completion/git-completion.bash | 10 +++++----- t/t9902-completion.sh | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-)
contrib/completion/git-completion.bash, t/t9902-completion.sh
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index cd92322..3cb106e 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2597,7 +2597,7 @@ _git_whatchanged () _git_log } -_git () +_main_git () { local i c=1 command __git_dir @@ -2648,7 +2648,7 @@ _git () fi } -_gitk () +_main_gitk () { __git_has_doubledash && return @@ -2700,13 +2700,13 @@ __git_complete () || complete -o default -o nospace -F $wrapper $1 } -__git_complete git _git -__git_complete gitk _gitk +__git_complete git _main_git +__git_complete gitk _main_gitk # The following are necessary only for Cygwin, and only are needed # when the user has tab-completed the executable name and consequently # included the '.exe' suffix. # if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then -__git_complete git.exe _git +__git_complete git.exe _main_git fi diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 0f09fd6..9a80c60 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -63,7 +63,7 @@ run_completion () local _cword _words=( $1 ) (( _cword = ${#_words[@]} - 1 )) - __git_wrap_git && print_comp + __git_wrap_main_git && print_comp } test_completion ()
-- 1.7.10.2
SZEDER GáborRe: [PATCH 1/2] completion: rename _git and _gitk
Hi,
On Sat, May 19, 2012 at 04:41:34AM +0200, Felipe Contreras wrote:
> Would be useful to provide backwards compatibility for _git. Also, zsh > completion uses _git, and it cannot be changed. > > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> > --- > contrib/completion/git-completion.bash | 10 +++++----- > t/t9902-completion.sh | 2 +- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index cd92322..3cb106e 100755 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -2597,7 +2597,7 @@ _git_whatchanged () > _git_log > } > > -_git () > +_main_git ()
> -_gitk () > +_main_gitk ()
After all those namespace discussions the names of these functions should start with _git or __git.
Gábor
Junio C HamanoRe: [PATCH 1/2] completion: rename _git and _gitk
SZEDER Gábor <szeder@ira.uka.de> writes:
>> -_git () >> +_main_git () > >> -_gitk () >> +_main_gitk () > > After all those namespace discussions the names of these functions > should start with _git or __git.
As these are pure implementation internal details that are called from very limited places, I do not think Felipe minds a patch to update them.
Personally I would find _git_git or __git_git even more mysterious and begs one-liner comments above their definition, though.
Thanks.
Felipe ContrerasRe: [PATCH 1/2] completion: rename _git and _gitk
On Tue, May 22, 2012 at 8:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes: > >>> -_git () >>> +_main_git () >> >>> -_gitk () >>> +_main_gitk () >> >> After all those namespace discussions the names of these functions >> should start with _git or __git. > > As these are pure implementation internal details that are called from > very limited places, I do not think Felipe minds a patch to update them.
I don't mind updating these to whatever you decide is the namespace for internal functions, just avoid _git ().
Cheers.
-- Felipe Contreras
Felipe Contreras[PATCH 2/2] completion: add support for backwards compatibility
Some people might be relying on _git and _gitk to define custom aliases, unfortunately, commit 6b179ad (completion: add new __git_complete helper) broke that support.
"bash: [: 1: unary operator expected"
This can be easily fixed by using __git_complete, but it's not meant to be public.
Although _git and _gitk are probably not meant to be public, it's easy to keep having support for them by having a wrapper to the proper new function that is fully functional.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- contrib/completion/git-completion.bash | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 3cb106e..1689f99 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2700,6 +2700,18 @@ __git_complete () || complete -o default -o nospace -F $wrapper $1 } +# wrapper for backwards compatibility +_git () +{ + __git_wrap_main_git +} + +# wrapper for backwards compatibility +_gitk () +{ + __git_wrap_main_gitk +} + __git_complete git _main_git __git_complete gitk _main_gitk
-- 1.7.10.2
Jeff KingRe: [PATCH 0/2] completion: backwards compatibility fix
On Sat, May 19, 2012 at 04:41:33AM +0200, Felipe Contreras wrote:
> These two patches are meant to fix the backwards compatibility of _git, and > _gitk. This also helps my zsh's wrapper. > > Felipe Contreras (2): > completion: rename _git and _gitk > completion: add support for backwards compatibilit
Thanks. This looks like the obviously correct solution. Even if we end up with a public _GIT_complete or whatever, the backwards-compatibility is worth it.
-Peff
Re: [PATCH 0/2] completion: backwards compatibility fix
On Sat, May 19, 2012 at 4:45 AM, Jeff King <peff@peff.net> wrote:
> On Sat, May 19, 2012 at 04:41:33AM +0200, Felipe Contreras wrote: > >> These two patches are meant to fix the backwards compatibility of _git, and >> _gitk. This also helps my zsh's wrapper. >> >> Felipe Contreras (2): >> completion: rename _git and _gitk >> completion: add support for backwards compatibilit > > Thanks. This looks like the obviously correct solution. Even if we end > up with a public _GIT_complete or whatever, the backwards-compatibility > is worth it.
Cool. Just though the patches now. Felipe, which variant should I use to define _main_git completion for the alias g with the 2 patches applied to git_completion.bash? Using bash-4.2.28.
Jeff KingRe: [PATCH 0/2] completion: backwards compatibility fix
On Sat, May 19, 2012 at 09:54:28AM +0200, Carsten Mattner wrote:
> >> Felipe Contreras (2): > >> completion: rename _git and _gitk > >> completion: add support for backwards compatibilit > > > > Thanks. This looks like the obviously correct solution. Even if we end > > up with a public _GIT_complete or whatever, the backwards-compatibility > > is worth it. > > Cool. Just though the patches now. Felipe, which variant should I use > to define _main_git completion for the alias g with the 2 patches > applied to git_completion.bash? Using bash-4.2.28.
I think the point of the patches is that you can continue to use your same "complete ... -F _git g" line as always (with these patches you could also say "__git_complete g _main_git", but that is not backwards compatible to older versions, if you use the same bashrc on multiple machines).
-Peff
Re: [PATCH 0/2] completion: backwards compatibility fix
On Sat, May 19, 2012 at 5:31 PM, Jeff King <peff@peff.net> wrote:
> On Sat, May 19, 2012 at 09:54:28AM +0200, Carsten Mattner wrote: > >> >> Felipe Contreras (2): >> >> completion: rename _git and _gitk >> >> completion: add support for backwards compatibilit >> > >> > Thanks. This looks like the obviously correct solution. Even if we end >> > up with a public _GIT_complete or whatever, the backwards-compatibility >> > is worth it. >> >> Cool. Just though the patches now. Felipe, which variant should I use >> to define _main_git completion for the alias g with the 2 patches >> applied to git_completion.bash? Using bash-4.2.28. > > I think the point of the patches is that you can continue to use your > same "complete ... -F _git g" line as always (with these patches you > could also say "__git_complete g _main_git", but that is not backwards > compatible to older versions, if you use the same bashrc on multiple > machines).
Great, is that short form always enough with the patches applied and the non-internal function to use for future compatibility?
Junio C HamanoRe: [PATCH 0/2] completion: backwards compatibility fix
Felipe Contreras <felipe.contreras@gmail.com> writes:
> These two patches are meant to fix the backwards compatibility of _git, and > _gitk.
Thanks.
Even though I think these two should be squashed into one "fix regression that made _git and _gitk behave differently" commit (because otherwise the result of applying the first one makes _git/_gitk disappear, shifting the breakage from "older usage fails inside _git" to "older usage fails to even find _git", the end result looks good.
Re: [PATCH 0/2] completion: backwards compatibility fix
On Sat, May 19, 2012 at 4:41 AM, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> Hi, > > These two patches are meant to fix the backwards compatibility of _git, and > _gitk. This also helps my zsh's wrapper. > > Felipe Contreras (2): > completion: rename _git and _gitk > completion: add support for backwards compatibilit > > contrib/completion/git-completion.bash | 22 +++++++++++++++++----- > t/t9902-completion.sh | 2 +- > 2 files changed, 18 insertions(+), 6 deletions(-) > > -- > 1.7.10.2 >
Thanks Felipe. With the 2 patches applied all the following completion definitions work.
complete -o bashdefault -o default -o nospace -F _git g 2>/dev/null \ || complete -o default -o nospace -F _git g __git_complete g _git __git_complete g _main_git
Ignoring backwards compatibility, which one would you suggest I use? Keep in mind that I originally copied the first long completion defintion from git-completion.bash a long time ago.
Felipe ContrerasRe: [PATCH 0/2] completion: backwards compatibility fix
On Sun, May 20, 2012 at 11:02 AM, Carsten Mattner <carstenmattner@googlemail.com> wrote:
> On Sat, May 19, 2012 at 4:41 AM, Felipe Contreras > <felipe.contreras@gmail.com> wrote: >> Hi, >> >> These two patches are meant to fix the backwards compatibility of _git, and >> _gitk. This also helps my zsh's wrapper. >> >> Felipe Contreras (2): >> completion: rename _git and _gitk >> completion: add support for backwards compatibilit >> >> contrib/completion/git-completion.bash | 22 +++++++++++++++++----- >> t/t9902-completion.sh | 2 +- >> 2 files changed, 18 insertions(+), 6 deletions(-) >> >> -- >> 1.7.10.2 >> > > Thanks Felipe. With the 2 patches applied all the following > completion definitions work. > > complete -o bashdefault -o default -o nospace -F _git g 2>/dev/null \ > || complete -o default -o nospace -F _git g > __git_complete g _git > __git_complete g _main_git > > Ignoring backwards compatibility, which one would you suggest > I use? Keep in mind that I originally copied the first > long completion defintion from git-completion.bash a long time > ago.
I would use '__git_complete g _git', but keep in mind that none of these forms have a promise to stay. Eventually the final one would be '_GIT_complete g _git' (hopefully), but if you want to be safe from changes maybe the first (original) would be best; even though there's no promise it won't break, we would probably try our best not to break it (again).
BTW. If your system has the '-o bashdefault' option you probably don't need the full form, just: % complete -o bashdefault -o default -o nospace -F _git g
Or if your system doesn't: % complete -o default -o nospace -F _git g
Cheers.
-- Felipe Contreras
Re: [PATCH 0/2] completion: backwards compatibility fix
On Sun, May 20, 2012 at 12:28 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> On Sun, May 20, 2012 at 11:02 AM, Carsten Mattner > <carstenmattner@googlemail.com> wrote: >> On Sat, May 19, 2012 at 4:41 AM, Felipe Contreras >> <felipe.contreras@gmail.com> wrote: >>> Hi, >>> >>> These two patches are meant to fix the backwards compatibility of _git, and >>> _gitk. This also helps my zsh's wrapper. >>> >>> Felipe Contreras (2): >>> completion: rename _git and _gitk >>> completion: add support for backwards compatibilit >>> >>> contrib/completion/git-completion.bash | 22 +++++++++++++++++----- >>> t/t9902-completion.sh | 2 +- >>> 2 files changed, 18 insertions(+), 6 deletions(-) >>> >>> -- >>> 1.7.10.2 >>> >> >> Thanks Felipe. With the 2 patches applied all the following >> completion definitions work. >> >> complete -o bashdefault -o default -o nospace -F _git g 2>/dev/null \ >> || complete -o default -o nospace -F _git g >> __git_complete g _git >> __git_complete g _main_git >> >> Ignoring backwards compatibility, which one would you suggest >> I use? Keep in mind that I originally copied the first >> long completion defintion from git-completion.bash a long time >> ago. > > I would use '__git_complete g _git', but keep in mind that none of > these forms have a promise to stay. Eventually the final one would be > '_GIT_complete g _git' (hopefully), but if you want to be safe from > changes maybe the first (original) would be best; even though there's > no promise it won't break, we would probably try our best not to break > it (again).
What about adding a small note at the top of the completion script together with the other documentation? Does it make sense to keep the known to work and public invocation in there?
> BTW. If your system has the '-o bashdefault' option you probably don't > need the full form, just: > % complete -o bashdefault -o default -o nospace -F _git g > > Or if your system doesn't: > % complete -o default -o nospace -F _git g
I like the short versions. They seem to work.