{"thread":{"id":"30563","subject":"[PATCH 1/2] completion: rename _git and _gitk","startedAt":"2012-05-19T02:41:33Z","lastAt":"2012-05-22T18:52:32Z","messageCount":14,"participants":["Felipe Contreras","Jeff King","Carsten Mattner","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"191712","messageId":"1337395295-29162-1-git-send-email-felipe.contreras@gmail.com","threadId":"30563","inReplyTo":null,"subject":"[PATCH 0/2] completion: backwards compatibility fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-05-19T02:41:33Z","receivedAt":"2012-05-19T02:41:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nThese two patches are meant to fix the backwards compatibility of _git, and\n_gitk. This also helps my zsh's wrapper.\n\nFelipe Contreras (2):\n completion: rename _git and _gitk\n completion: add support for backwards compatibilit\n\n contrib/completion/git-completion.bash | 22 +++++++++++++++++-----\n t/t9902-completion.sh | 2 +-\n 2 files changed, 18 insertions(+), 6 deletions(-)\n\n-- \n1.7.10.2\n"},{"id":"191711","messageId":"1337395295-29162-2-git-send-email-felipe.contreras@gmail.com","threadId":"30563","inReplyTo":"1337395295-29162-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/2] completion: rename _git and _gitk","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-05-19T02:41:34Z","receivedAt":"2012-05-19T02:41:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Would be useful to provide backwards compatibility for _git. Also, zsh\ncompletion uses _git, and it cannot be changed.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/completion/git-completion.bash | 10 +++++-----\n t/t9902-completion.sh | 2 +-\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex cd92322..3cb106e 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2597,7 +2597,7 @@ _git_whatchanged ()\n \t_git_log\n }\n \n-_git ()\n+_main_git ()\n {\n \tlocal i c=1 command __git_dir\n \n@@ -2648,7 +2648,7 @@ _git ()\n \tfi\n }\n \n-_gitk ()\n+_main_gitk ()\n {\n \t__git_has_doubledash && return\n \n@@ -2700,13 +2700,13 @@ __git_complete ()\n \t\t|| complete -o default -o nospace -F $wrapper $1\n }\n \n-__git_complete git _git\n-__git_complete gitk _gitk\n+__git_complete git _main_git\n+__git_complete gitk _main_gitk\n \n # The following are necessary only for Cygwin, and only are needed\n # when the user has tab-completed the executable name and consequently\n # included the '.exe' suffix.\n #\n if [ Cygwin = \"$(uname -o 2>/dev/null)\" ]; then\n-__git_complete git.exe _git\n+__git_complete git.exe _main_git\n fi\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 0f09fd6..9a80c60 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -63,7 +63,7 @@ run_completion ()\n \tlocal _cword\n \t_words=( $1 )\n \t(( _cword = ${#_words[@]} - 1 ))\n-\t__git_wrap_git && print_comp\n+\t__git_wrap_main_git && print_comp\n }\n \n test_completion ()\n-- \n1.7.10.2\n"},{"id":"191713","messageId":"1337395295-29162-3-git-send-email-felipe.contreras@gmail.com","threadId":"30563","inReplyTo":"1337395295-29162-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/2] completion: add support for backwards compatibility","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-05-19T02:41:35Z","receivedAt":"2012-05-19T02:41:35Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Some people might be relying on _git and _gitk to define custom aliases,\nunfortunately, commit 6b179ad (completion: add new __git_complete\nhelper) broke that support.\n\n \"bash: [: 1: unary operator expected\"\n\nThis can be easily fixed by using __git_complete, but it's not meant to\nbe public.\n\nAlthough _git and _gitk are probably not meant to be public, it's easy\nto keep having support for them by having a wrapper to the proper\nnew function that is fully functional.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/completion/git-completion.bash | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 3cb106e..1689f99 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2700,6 +2700,18 @@ __git_complete ()\n \t\t|| complete -o default -o nospace -F $wrapper $1\n }\n \n+# wrapper for backwards compatibility\n+_git ()\n+{\n+\t__git_wrap_main_git\n+}\n+\n+# wrapper for backwards compatibility\n+_gitk ()\n+{\n+\t__git_wrap_main_gitk\n+}\n+\n __git_complete git _main_git\n __git_complete gitk _main_gitk\n \n-- \n1.7.10.2\n"},{"id":"191715","messageId":"20120519024503.GA2801@sigill.intra.peff.net","threadId":"30563","inReplyTo":"1337395295-29162-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 0/2] completion: backwards compatibility fix","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-19T02:45:03Z","receivedAt":"2012-05-19T02:45:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 19, 2012 at 04:41:33AM +0200, Felipe Contreras wrote:\n\n> These two patches are meant to fix the backwards compatibility of _git, and\n> _gitk. This also helps my zsh's wrapper.\n>\n> Felipe Contreras (2):\n> completion: rename _git and _gitk\n> completion: add support for backwards compatibilit\n\nThanks. This looks like the obviously correct solution. Even if we end\nup with a public _GIT_complete or whatever, the backwards-compatibility\nis worth it.\n\n-Peff\n"},{"id":"191730","messageId":"CACY+Hvp-G4odbftiFeu1u+1icROD36M=CzSn3DKu_TEaqNB3OQ@mail.gmail.com","threadId":"30563","inReplyTo":"20120519024503.GA2801@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] completion: backwards compatibility fix","fromName":"Carsten Mattner","fromEmail":"carstenmattner@googlemail.com","sentAt":"2012-05-19T07:54:28Z","receivedAt":"2012-05-19T07:54:28Z","isPatch":true,"sender":{"key":"carstenmattner@googlemail.com","avatar":null},"body":"On Sat, May 19, 2012 at 4:45 AM, Jeff King wrote:\n> On Sat, May 19, 2012 at 04:41:33AM +0200, Felipe Contreras wrote:\n>\n>> These two patches are meant to fix the backwards compatibility of _git, and\n>> _gitk. This also helps my zsh's wrapper.\n>>\n>> Felipe Contreras (2):\n>>   completion: rename _git and _gitk\n>>   completion: add support for backwards compatibilit\n>\n> Thanks. This looks like the obviously correct solution. Even if we end\n> up with a public _GIT_complete or whatever, the backwards-compatibility\n> is worth it.\n\nCool. Just though the patches now. Felipe, which variant should I use\nto define _main_git completion for the alias g with the 2 patches\napplied to git_completion.bash? Using bash-4.2.28.\n"},{"id":"191740","messageId":"20120519153134.GB6979@sigill.intra.peff.net","threadId":"30563","inReplyTo":"CACY+Hvp-G4odbftiFeu1u+1icROD36M=CzSn3DKu_TEaqNB3OQ@mail.gmail.com","subject":"Re: [PATCH 0/2] completion: backwards compatibility fix","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-19T15:31:34Z","receivedAt":"2012-05-19T15:31:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 19, 2012 at 09:54:28AM +0200, Carsten Mattner wrote:\n\n> >> Felipe Contreras (2):\n> >>   completion: rename _git and _gitk\n> >>   completion: add support for backwards compatibilit\n> >\n> > Thanks. This looks like the obviously correct solution. Even if we end\n> > up with a public _GIT_complete or whatever, the backwards-compatibility\n> > is worth it.\n> \n> Cool. Just though the patches now. Felipe, which variant should I use\n> to define _main_git completion for the alias g with the 2 patches\n> applied to git_completion.bash? Using bash-4.2.28.\n\nI think the point of the patches is that you can continue to use your\nsame \"complete ... -F _git g\" line as always (with these patches you\ncould also say \"__git_complete g _main_git\", but that is not backwards\ncompatible to older versions, if you use the same bashrc on multiple\nmachines).\n\n-Peff\n"},{"id":"191741","messageId":"CACY+HvpUXiK3_bmBZV7i_bgKOP7vSuYb2k4ARtqPGXzdpJfbPg@mail.gmail.com","threadId":"30563","inReplyTo":"20120519153134.GB6979@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] completion: backwards compatibility fix","fromName":"Carsten Mattner","fromEmail":"carstenmattner@googlemail.com","sentAt":"2012-05-19T17:45:47Z","receivedAt":"2012-05-19T17:45:47Z","isPatch":true,"sender":{"key":"carstenmattner@googlemail.com","avatar":null},"body":"On Sat, May 19, 2012 at 5:31 PM, Jeff King wrote:\n> On Sat, May 19, 2012 at 09:54:28AM +0200, Carsten Mattner wrote:\n>\n>> >> Felipe Contreras (2):\n>> >>   completion: rename _git and _gitk\n>> >>   completion: add support for backwards compatibilit\n>> >\n>> > Thanks. This looks like the obviously correct solution. Even if we end\n>> > up with a public _GIT_complete or whatever, the backwards-compatibility\n>> > is worth it.\n>>\n>> Cool. Just though the patches now. Felipe, which variant should I use\n>> to define _main_git completion for the alias g with the 2 patches\n>> applied to git_completion.bash? Using bash-4.2.28.\n>\n> I think the point of the patches is that you can continue to use your\n> same \"complete ... -F _git g\" line as always (with these patches you\n> could also say \"__git_complete g _main_git\", but that is not backwards\n> compatible to older versions, if you use the same bashrc on multiple\n> machines).\n\nGreat, is that short form always enough with the patches applied\nand the non-internal function to use for future compatibility?\n"},{"id":"191752","messageId":"7vaa13bfms.fsf@alter.siamese.dyndns.org","threadId":"30563","inReplyTo":"1337395295-29162-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 0/2] completion: backwards compatibility fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-20T05:11:55Z","receivedAt":"2012-05-20T05:11:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> These two patches are meant to fix the backwards compatibility of _git, and\n> _gitk.\n\nThanks.\n\nEven though I think these two should be squashed into one \"fix regression\nthat made _git and _gitk behave differently\" commit (because otherwise the\nresult of applying the first one makes _git/_gitk disappear, shifting the\nbreakage from \"older usage fails inside _git\" to \"older usage fails to\neven find _git\", the end result looks good.\n"},{"id":"191755","messageId":"CACY+HvqhJf6ywTaDgT5ie-3iBQAGwngTYPiCvKgw6mJB=1+fOg@mail.gmail.com","threadId":"30563","inReplyTo":"1337395295-29162-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 0/2] completion: backwards compatibility fix","fromName":"Carsten Mattner","fromEmail":"carstenmattner@googlemail.com","sentAt":"2012-05-20T09:02:35Z","receivedAt":"2012-05-20T09:02:35Z","isPatch":true,"sender":{"key":"carstenmattner@googlemail.com","avatar":null},"body":"On Sat, May 19, 2012 at 4:41 AM, Felipe Contreras\n wrote:\n> Hi,\n>\n> These two patches are meant to fix the backwards compatibility of _git, and\n> _gitk. This also helps my zsh's wrapper.\n>\n> Felipe Contreras (2):\n>  completion: rename _git and _gitk\n>  completion: add support for backwards compatibilit\n>\n>  contrib/completion/git-completion.bash |   22 +++++++++++++++++-----\n>  t/t9902-completion.sh                  |    2 +-\n>  2 files changed, 18 insertions(+), 6 deletions(-)\n>\n> --\n> 1.7.10.2\n>\n\nThanks Felipe. With the 2 patches applied all the following\ncompletion definitions work.\n\ncomplete -o bashdefault -o default -o nospace -F _git g 2>/dev/null \\\n || complete -o default -o nospace -F _git g\n__git_complete g _git\n__git_complete g _main_git\n\nIgnoring backwards compatibility, which one would you suggest\nI use? Keep in mind that I originally copied the first\nlong completion defintion from git-completion.bash a long time\nago.\n"},{"id":"191758","messageId":"CAMP44s2sJaW-n7EGTu+hNNkaEnGkk7wKDQD56bgq2=qeeFqWrw@mail.gmail.com","threadId":"30563","inReplyTo":"CACY+HvqhJf6ywTaDgT5ie-3iBQAGwngTYPiCvKgw6mJB=1+fOg@mail.gmail.com","subject":"Re: [PATCH 0/2] completion: backwards compatibility fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-05-20T10:28:35Z","receivedAt":"2012-05-20T10:28:35Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, May 20, 2012 at 11:02 AM, Carsten Mattner\n wrote:\n> On Sat, May 19, 2012 at 4:41 AM, Felipe Contreras\n> wrote:\n>> Hi,\n>>\n>> These two patches are meant to fix the backwards compatibility of _git, and\n>> _gitk. This also helps my zsh's wrapper.\n>>\n>> Felipe Contreras (2):\n>>  completion: rename _git and _gitk\n>>  completion: add support for backwards compatibilit\n>>\n>>  contrib/completion/git-completion.bash |   22 +++++++++++++++++-----\n>>  t/t9902-completion.sh                  |    2 +-\n>>  2 files changed, 18 insertions(+), 6 deletions(-)\n>>\n>> --\n>> 1.7.10.2\n>>\n>\n> Thanks Felipe. With the 2 patches applied all the following\n> completion definitions work.\n>\n> complete -o bashdefault -o default -o nospace -F _git g 2>/dev/null \\\n>   || complete -o default -o nospace -F _git g\n> __git_complete g _git\n> __git_complete g _main_git\n>\n> Ignoring backwards compatibility, which one would you suggest\n> I use? Keep in mind that I originally copied the first\n> long completion defintion from git-completion.bash a long time\n> ago.\n\nI would use '__git_complete g _git', but keep in mind that none of\nthese forms have a promise to stay. Eventually the final one would be\n'_GIT_complete g _git' (hopefully), but if you want to be safe from\nchanges maybe the first (original) would be best; even though there's\nno promise it won't break, we would probably try our best not to break\nit (again).\n\nBTW. If your system has the '-o bashdefault' option you probably don't\nneed the full form, just:\n% complete -o bashdefault -o default -o nospace -F _git g\n\nOr if your system doesn't:\n% complete -o default -o nospace -F _git g\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"191765","messageId":"CACY+Hvrj_out4Lm+8Wi3egdc+bs4MWLpVy2X98Uo__r3mSpBOQ@mail.gmail.com","threadId":"30563","inReplyTo":"CAMP44s2sJaW-n7EGTu+hNNkaEnGkk7wKDQD56bgq2=qeeFqWrw@mail.gmail.com","subject":"Re: [PATCH 0/2] completion: backwards compatibility fix","fromName":"Carsten Mattner","fromEmail":"carstenmattner@googlemail.com","sentAt":"2012-05-20T17:27:20Z","receivedAt":"2012-05-20T17:27:20Z","isPatch":true,"sender":{"key":"carstenmattner@googlemail.com","avatar":null},"body":"On Sun, May 20, 2012 at 12:28 PM, Felipe Contreras\n wrote:\n> On Sun, May 20, 2012 at 11:02 AM, Carsten Mattner\n> wrote:\n>> On Sat, May 19, 2012 at 4:41 AM, Felipe Contreras\n>> wrote:\n>>> Hi,\n>>>\n>>> These two patches are meant to fix the backwards compatibility of _git, and\n>>> _gitk. This also helps my zsh's wrapper.\n>>>\n>>> Felipe Contreras (2):\n>>>  completion: rename _git and _gitk\n>>>  completion: add support for backwards compatibilit\n>>>\n>>>  contrib/completion/git-completion.bash |   22 +++++++++++++++++-----\n>>>  t/t9902-completion.sh                  |    2 +-\n>>>  2 files changed, 18 insertions(+), 6 deletions(-)\n>>>\n>>> --\n>>> 1.7.10.2\n>>>\n>>\n>> Thanks Felipe. With the 2 patches applied all the following\n>> completion definitions work.\n>>\n>> complete -o bashdefault -o default -o nospace -F _git g 2>/dev/null \\\n>>   || complete -o default -o nospace -F _git g\n>> __git_complete g _git\n>> __git_complete g _main_git\n>>\n>> Ignoring backwards compatibility, which one would you suggest\n>> I use? Keep in mind that I originally copied the first\n>> long completion defintion from git-completion.bash a long time\n>> ago.\n>\n> I would use '__git_complete g _git', but keep in mind that none of\n> these forms have a promise to stay. Eventually the final one would be\n> '_GIT_complete g _git' (hopefully), but if you want to be safe from\n> changes maybe the first (original) would be best; even though there's\n> no promise it won't break, we would probably try our best not to break\n> it (again).\n\nWhat about adding a small note at the top of the completion script\ntogether with the other documentation? Does it make sense to keep\nthe known to work and public invocation in there?\n\n> BTW. If your system has the '-o bashdefault' option you probably don't\n> need the full form, just:\n> % complete -o bashdefault -o default -o nospace -F _git g\n>\n> Or if your system doesn't:\n> % complete -o default -o nospace -F _git g\n\nI like the short versions. They seem to work.\n"},{"id":"191886","messageId":"20120522082406.GA6001@goldbirke","threadId":"30563","inReplyTo":"1337395295-29162-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 1/2] completion: rename _git and _gitk","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-05-22T08:24:06Z","receivedAt":"2012-05-22T08:24:06Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Sat, May 19, 2012 at 04:41:34AM +0200, Felipe Contreras wrote:\n> Would be useful to provide backwards compatibility for _git. Also, zsh\n> completion uses _git, and it cannot be changed.\n> \n> Signed-off-by: Felipe Contreras \n> ---\n> contrib/completion/git-completion.bash | 10 +++++-----\n> t/t9902-completion.sh | 2 +-\n> 2 files changed, 6 insertions(+), 6 deletions(-)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index cd92322..3cb106e 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2597,7 +2597,7 @@ _git_whatchanged ()\n> \t_git_log\n> }\n> \n> -_git ()\n> +_main_git ()\n\n> -_gitk ()\n> +_main_gitk ()\n\nAfter all those namespace discussions the names of these functions\nshould start with _git or __git.\n\n\nGábor\n"},{"id":"191914","messageId":"7vsjes3xl9.fsf@alter.siamese.dyndns.org","threadId":"30563","inReplyTo":"20120522082406.GA6001@goldbirke","subject":"Re: [PATCH 1/2] completion: rename _git and _gitk","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-22T18:00:18Z","receivedAt":"2012-05-22T18:00:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n>> -_git ()\n>> +_main_git ()\n>\n>> -_gitk ()\n>> +_main_gitk ()\n>\n> After all those namespace discussions the names of these functions\n> should start with _git or __git.\n\nAs these are pure implementation internal details that are called from\nvery limited places, I do not think Felipe minds a patch to update them.\n\nPersonally I would find _git_git or __git_git even more mysterious and\nbegs one-liner comments above their definition, though.\n\nThanks.\n"},{"id":"191924","messageId":"CAMP44s3yErr97pFy=izy3ydKd=xwPH+ar89mwJeu9pP7SOxgig@mail.gmail.com","threadId":"30563","inReplyTo":"7vsjes3xl9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] completion: rename _git and _gitk","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-05-22T18:52:32Z","receivedAt":"2012-05-22T18:52:32Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, May 22, 2012 at 8:00 PM, Junio C Hamano wrote:\n> SZEDER Gábor writes:\n>\n>>> -_git ()\n>>> +_main_git ()\n>>\n>>> -_gitk ()\n>>> +_main_gitk ()\n>>\n>> After all those namespace discussions the names of these functions\n>> should start with _git or __git.\n>\n> As these are pure implementation internal details that are called from\n> very limited places, I do not think Felipe minds a patch to update them.\n\nI don't mind updating these to whatever you decide is the namespace\nfor internal functions, just avoid _git ().\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}