# [PATCH 1/2] completion: rename _git and _gitk 14 messages from 2012-05-19 to 2012-05-22. Participants: Felipe Contreras, Jeff King, Carsten Mattner, Junio C Hamano, SZEDER Gábor. Thread: https://gitlist.dev/t/30563 ## Felipe Contreras, 2012-05-19 02:41 Subject: [PATCH 0/2] completion: backwards compatibility fix Message-ID: <1337395295-29162-1-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1337395295-29162-1-git-send-email-felipe.contreras%40gmail.com ``` 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 Contreras, 2012-05-19 02:41 Subject: [PATCH 1/2] completion: rename _git and _gitk Message-ID: <1337395295-29162-2-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1337395295-29162-2-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1337395295-29162-1-git-send-email-felipe.contreras@gmail.com> ``` Would be useful to provide backwards compatibility for _git. Also, zsh completion uses _git, and it cannot be changed. Signed-off-by: Felipe Contreras --- 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 () { 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 ``` ## Felipe Contreras, 2012-05-19 02:41 Subject: [PATCH 2/2] completion: add support for backwards compatibility Message-ID: <1337395295-29162-3-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1337395295-29162-3-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1337395295-29162-1-git-send-email-felipe.contreras@gmail.com> ``` 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 --- 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 King, 2012-05-19 02:45 Subject: Re: [PATCH 0/2] completion: backwards compatibility fix Message-ID: <20120519024503.GA2801@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120519024503.GA2801%40sigill.intra.peff.net In-Reply-To: <1337395295-29162-1-git-send-email-felipe.contreras@gmail.com> ``` 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 ``` ## Carsten Mattner, 2012-05-19 07:54 Subject: Re: [PATCH 0/2] completion: backwards compatibility fix Message-ID: URL: https://gitlist.dev/e/CACY%2BHvp-G4odbftiFeu1u%2B1icROD36M%3DCzSn3DKu_TEaqNB3OQ%40mail.gmail.com In-Reply-To: <20120519024503.GA2801@sigill.intra.peff.net> ``` On Sat, May 19, 2012 at 4:45 AM, Jeff King 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 King, 2012-05-19 15:31 Subject: Re: [PATCH 0/2] completion: backwards compatibility fix Message-ID: <20120519153134.GB6979@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120519153134.GB6979%40sigill.intra.peff.net In-Reply-To: ``` 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 ``` ## Carsten Mattner, 2012-05-19 17:45 Subject: Re: [PATCH 0/2] completion: backwards compatibility fix Message-ID: URL: https://gitlist.dev/e/CACY%2BHvpUXiK3_bmBZV7i_bgKOP7vSuYb2k4ARtqPGXzdpJfbPg%40mail.gmail.com In-Reply-To: <20120519153134.GB6979@sigill.intra.peff.net> ``` On Sat, May 19, 2012 at 5:31 PM, Jeff King 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 Hamano, 2012-05-20 05:11 Subject: Re: [PATCH 0/2] completion: backwards compatibility fix Message-ID: <7vaa13bfms.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vaa13bfms.fsf%40alter.siamese.dyndns.org In-Reply-To: <1337395295-29162-1-git-send-email-felipe.contreras@gmail.com> ``` Felipe Contreras 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. ``` ## Carsten Mattner, 2012-05-20 09:02 Subject: Re: [PATCH 0/2] completion: backwards compatibility fix Message-ID: URL: https://gitlist.dev/e/CACY%2BHvqhJf6ywTaDgT5ie-3iBQAGwngTYPiCvKgw6mJB%3D1%2BfOg%40mail.gmail.com In-Reply-To: <1337395295-29162-1-git-send-email-felipe.contreras@gmail.com> ``` On Sat, May 19, 2012 at 4:41 AM, Felipe Contreras 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 Contreras, 2012-05-20 10:28 Subject: Re: [PATCH 0/2] completion: backwards compatibility fix Message-ID: URL: https://gitlist.dev/e/CAMP44s2sJaW-n7EGTu%2BhNNkaEnGkk7wKDQD56bgq2%3DqeeFqWrw%40mail.gmail.com In-Reply-To: ``` On Sun, May 20, 2012 at 11:02 AM, Carsten Mattner wrote: > On Sat, May 19, 2012 at 4:41 AM, Felipe Contreras > 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 ``` ## Carsten Mattner, 2012-05-20 17:27 Subject: Re: [PATCH 0/2] completion: backwards compatibility fix Message-ID: URL: https://gitlist.dev/e/CACY%2BHvrj_out4Lm%2B8Wi3egdc%2Bbs4MWLpVy2X98Uo__r3mSpBOQ%40mail.gmail.com In-Reply-To: ``` On Sun, May 20, 2012 at 12:28 PM, Felipe Contreras wrote: > On Sun, May 20, 2012 at 11:02 AM, Carsten Mattner > wrote: >> On Sat, May 19, 2012 at 4:41 AM, Felipe Contreras >> 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. ``` ## SZEDER Gábor, 2012-05-22 08:24 Subject: Re: [PATCH 1/2] completion: rename _git and _gitk Message-ID: <20120522082406.GA6001@goldbirke> URL: https://gitlist.dev/e/20120522082406.GA6001%40goldbirke In-Reply-To: <1337395295-29162-2-git-send-email-felipe.contreras@gmail.com> ``` 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 > --- > 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 Hamano, 2012-05-22 18:00 Subject: Re: [PATCH 1/2] completion: rename _git and _gitk Message-ID: <7vsjes3xl9.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsjes3xl9.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120522082406.GA6001@goldbirke> ``` SZEDER Gábor 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 Contreras, 2012-05-22 18:52 Subject: Re: [PATCH 1/2] completion: rename _git and _gitk Message-ID: URL: https://gitlist.dev/e/CAMP44s3yErr97pFy%3Dizy3ydKd%3DxwPH%2Bar89mwJeu9pP7SOxgig%40mail.gmail.com In-Reply-To: <7vsjes3xl9.fsf@alter.siamese.dyndns.org> ``` On Tue, May 22, 2012 at 8:00 PM, Junio C Hamano wrote: > SZEDER Gábor 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 ```