{"thread":{"id":"14330","subject":"[PATCH] completion: add branch options --contains --merged --no-merged","startedAt":"2008-07-07T20:41:54Z","lastAt":"2008-07-09T00:06:43Z","messageCount":12,"participants":["Eric Raible","Shawn O. Pearce","Junio C Hamano","Johannes Schindelin","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"82488","messageId":"279b37b20807071341k3551e61cl10c5969600ba8218@mail.gmail.com","threadId":"14330","inReplyTo":null,"subject":"[PATCH] completion: add branch options --contains --merged --no-merged","fromName":"Eric Raible","fromEmail":"raible@gmail.com","sentAt":"2008-07-07T20:41:54Z","receivedAt":"2008-07-07T20:41:54Z","isPatch":true,"sender":{"key":"raible@gmail.com","avatar":null},"body":"Signed-off-by: Eric Raible \n---\n contrib/completion/git-completion.bash | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash\nb/contrib/completion/git-completion.bash\nindex 0eb8df0..22e109d 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -546,7 +546,7 @@ _git_branch ()\n \t--*)\n \t\t__gitcomp \"\n \t\t\t--color --no-color --verbose --abbrev= --no-abbrev\n-\t\t\t--track --no-track\n+\t\t\t--track --no-track --contains --merged --no-merged\n \t\t\t\"\n \t\t;;\n \t*)\n-- \n1.5.6.1.1071.g76fb.dirty\n"},{"id":"82549","messageId":"20080708044922.GD2542@spearce.org","threadId":"14330","inReplyTo":"279b37b20807071341k3551e61cl10c5969600ba8218@mail.gmail.com","subject":"Re: [PATCH] completion: add branch options --contains --merged --no-merged","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-08T04:49:22Z","receivedAt":"2008-07-08T04:49:22Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Eric Raible wrote:\n> Signed-off-by: Eric Raible \n\nTrivially-Acked-by: Shawn O. Pearce \n\n;-)\n\nMore completion support that probably should go to maint, as the\nfunctionality in git-branch is in 1.5.6 but we (again) forgot to\nmake sure the completion was up-to-date prior to release.\n\n> ---\n> contrib/completion/git-completion.bash | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/contrib/completion/git-completion.bash\n> b/contrib/completion/git-completion.bash\n> index 0eb8df0..22e109d 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -546,7 +546,7 @@ _git_branch ()\n> \t--*)\n> \t\t__gitcomp \"\n> \t\t\t--color --no-color --verbose --abbrev= --no-abbrev\n> -\t\t\t--track --no-track\n> +\t\t\t--track --no-track --contains --merged --no-merged\n> \t\t\t\"\n> \t\t;;\n> \t*)\n> -- \n> 1.5.6.1.1071.g76fb.dirty\n\n-- \nShawn.\n"},{"id":"82554","messageId":"7vprppvt7a.fsf@gitster.siamese.dyndns.org","threadId":"14330","inReplyTo":"20080708044922.GD2542@spearce.org","subject":"Re: [PATCH] completion: add branch options --contains --merged --no-merged","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-08T05:30:17Z","receivedAt":"2008-07-08T05:30:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Eric Raible wrote:\n>> Signed-off-by: Eric Raible \n>\n> Trivially-Acked-by: Shawn O. Pearce \n>\n> ;-)\n>\n> More completion support that probably should go to maint, as the\n> functionality in git-branch is in 1.5.6 but we (again) forgot to\n> make sure the completion was up-to-date prior to release.\n\nI am actually getting more worried about completion code getting larger\nand larger without its performance impact not being looked at nor\naddressed adequately. In my regular working tree:\n\n\t$ echo Docu\n\ncompletes \"mentation/\" instantly, but:\n\n\t$ git log -- Docu\n\ntakes about 1.5 to 2 seconds to complete the same.\n"},{"id":"82590","messageId":"alpine.DEB.1.00.0807081335470.4319@eeepc-johanness","threadId":"14330","inReplyTo":"7vprppvt7a.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] completion: add branch options --contains --merged --no-merged","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-08T11:36:43Z","receivedAt":"2008-07-08T11:36:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 7 Jul 2008, Junio C Hamano wrote:\n\n> \"Shawn O. Pearce\" writes:\n> \n> > Eric Raible wrote:\n> >> Signed-off-by: Eric Raible \n> >\n> > Trivially-Acked-by: Shawn O. Pearce \n> >\n> > ;-)\n> >\n> > More completion support that probably should go to maint, as the\n> > functionality in git-branch is in 1.5.6 but we (again) forgot to\n> > make sure the completion was up-to-date prior to release.\n> \n> I am actually getting more worried about completion code getting larger\n> and larger without its performance impact not being looked at nor\n> addressed adequately. In my regular working tree:\n> \n> \t$ echo Docu\n> \n> completes \"mentation/\" instantly, but:\n> \n> \t$ git log -- Docu\n> \n> takes about 1.5 to 2 seconds to complete the same.\n\nI noticed that myself, but did not have time to look into it.\n\nIt shows two bugs, actually: completions do not care about \"--\", and \ncompleting refs takes way too long.\n\nCiao,\nDscho\n"},{"id":"82610","messageId":"20080708165614.GB8224@neumann","threadId":"14330","inReplyTo":"alpine.DEB.1.00.0807081335470.4319@eeepc-johanness","subject":"[PATCH] bash: offer only paths after '--'","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2008-07-08T16:56:14Z","receivedAt":"2008-07-08T16:56:14Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Many git commands use '--' to separate subcommands, options, and refs\nfrom paths. However, the programmable completion for several of these\ncommands does not respect the '--', and offer subcommands, options, or\nrefs after a '--', although only paths are permitted. e.g. 'git bisect\n-- ' offers subcommands, 'git log -- --' offers options and\n'git log -- git' offers all gitgui tags.\n\nThe completion for the following commands share this wrong behaviour:\n am add bisect commit diff log reset shortlog submodule gitk.\n\nTo avoid this, we check the presence of a '--' on the command line first\nand let the shell do filename completion, if one is found.\n\nSigned-off-by: SZEDER Gábor \n---\n\nOn Tue, Jul 08, 2008 at 01:36:43PM +0200, Johannes Schindelin wrote:\n> It shows two bugs, actually: completions do not care about \"--\", \nI think I have found and corrected all the places where '--' was not\nhandled properly, but might have overlooked something.\n\nHope that I got the commit message right (;\n\n\n contrib/completion/git-completion.bash | 30 ++++++++++++++++++++++++++++++\n 1 files changed, 30 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 6a15522..e7d8a75 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -451,6 +451,18 @@ __git_find_subcommand ()\n \tdone\n }\n \n+__git_has_doubledash ()\n+{\n+\tlocal c=1\n+\twhile [ $c -lt $COMP_CWORD ]; do\n+\t\tif [ \"--\" = \"${COMP_WORDS[c]}\" ]; then\n+\t\t\treturn 0\n+\t\tfi\n+\t\tc=$((++c))\n+\tdone\n+\treturn 1\n+}\n+\n __git_whitespacelist=\"nowarn warn error error-all strip\"\n \n _git_am ()\n@@ -497,6 +509,8 @@ _git_apply ()\n \n _git_add ()\n {\n+\t__git_has_doubledash && return\n+\n \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n \tcase \"$cur\" in\n \t--*)\n@@ -511,6 +525,8 @@ _git_add ()\n \n _git_bisect ()\n {\n+\t__git_has_doubledash && return\n+\n \tlocal subcommands=\"start bad good skip reset visualize replay log run\"\n \tlocal subcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n \tif [ -z \"$subcommand\" ]; then\n@@ -612,6 +628,8 @@ _git_cherry_pick ()\n \n _git_commit ()\n {\n+\t__git_has_doubledash && return\n+\n \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n \tcase \"$cur\" in\n \t--*)\n@@ -631,6 +649,8 @@ _git_describe ()\n \n _git_diff ()\n {\n+\t__git_has_doubledash && return\n+\n \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n \tcase \"$cur\" in\n \t--*)\n@@ -733,6 +753,8 @@ _git_ls_tree ()\n \n _git_log ()\n {\n+\t__git_has_doubledash && return\n+\n \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n \tcase \"$cur\" in\n \t--pretty=*)\n@@ -1088,6 +1110,8 @@ _git_remote ()\n \n _git_reset ()\n {\n+\t__git_has_doubledash && return\n+\n \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n \tcase \"$cur\" in\n \t--*)\n@@ -1100,6 +1124,8 @@ _git_reset ()\n \n _git_shortlog ()\n {\n+\t__git_has_doubledash && return\n+\n \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n \tcase \"$cur\" in\n \t--*)\n@@ -1157,6 +1183,8 @@ _git_stash ()\n \n _git_submodule ()\n {\n+\t__git_has_doubledash && return\n+\n \tlocal subcommands=\"add status init update\"\n \tif [ -z \"$(__git_find_subcommand \"$subcommands\")\" ]; then\n \t\tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n@@ -1362,6 +1390,8 @@ _git ()\n \n _gitk ()\n {\n+\t__git_has_doubledash && return\n+\n \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n \tlocal g=\"$(git rev-parse --git-dir 2>/dev/null)\"\n \tlocal merge=\"\"\n-- \n1.5.6.1.118.g82b2fef\n"},{"id":"82617","messageId":"loom.20080708T174319-960@post.gmane.org","threadId":"14330","inReplyTo":"20080708165614.GB8224@neumann","subject":"Re: [PATCH] bash: offer only paths after '--'","fromName":"Eric Raible","fromEmail":"raible@gmail.com","sentAt":"2008-07-08T17:46:08Z","receivedAt":"2008-07-08T17:46:08Z","isPatch":true,"sender":{"key":"raible@gmail.com","avatar":null},"body":"SZEDER Gábor ira.uka.de> writes:\n\n> \n> Many git commands use '--' to separate subcommands, options, and refs\n> from paths. However, the programmable completion for several of these\n> commands does not respect the '--', and offer subcommands, options, or\n> refs after a '--', although only paths are permitted. e.g. 'git bisect\n\nI like this change, but how about also offering a plain '--' as one\nof the completion choices as a way of reminding newbies that the\ncommand in question is one of the ones that takes filenames after\nall options?\n"},{"id":"82635","messageId":"7vtzf0rusw.fsf@gitster.siamese.dyndns.org","threadId":"14330","inReplyTo":"20080708165614.GB8224@neumann","subject":"Re: [PATCH] bash: offer only paths after '--'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-08T20:21:35Z","receivedAt":"2008-07-08T20:21:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> Hope that I got the commit message right (;\n\nIt was very readable. Thanks.\n\n> +__git_has_doubledash ()\n> +{\n> +\tlocal c=1\n> +\twhile [ $c -lt $COMP_CWORD ]; do\n> +\t\tif [ \"--\" = \"${COMP_WORDS[c]}\" ]; then\n> +\t\t\treturn 0\n> +\t\tfi\n> +\t\tc=$((++c))\n\nThis assignment is somewhat curious, although it should work as expected\neither way ;-)\n\nShawn?\n"},{"id":"82644","messageId":"20080708231837.GA16895@spearce.org","threadId":"14330","inReplyTo":"7vtzf0rusw.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] bash: offer only paths after '--'","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-08T23:18:37Z","receivedAt":"2008-07-08T23:18:37Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> SZEDER Gábor writes:\n> \n> > Hope that I got the commit message right (;\n> \n> It was very readable. Thanks.\n\nAcked-by: Shawn O. Pearce \n\n> > +__git_has_doubledash ()\n> > +{\n> > +\tlocal c=1\n> > +\twhile [ $c -lt $COMP_CWORD ]; do\n> > +\t\tif [ \"--\" = \"${COMP_WORDS[c]}\" ]; then\n> > +\t\t\treturn 0\n> > +\t\tfi\n> > +\t\tc=$((++c))\n> \n> This assignment is somewhat curious, although it should work as expected\n> either way ;-)\n\nI agree, its damned odd. But we already do this in the same\nsort of loop inside of _git_branch() (see around line 541 in\nnext). This new patch is only sticking with our current set\nof conventions in the script, so I say its fine.\n\n-- \nShawn.\n"},{"id":"82647","messageId":"7v7ibwq7u2.fsf@gitster.siamese.dyndns.org","threadId":"14330","inReplyTo":"20080708231837.GA16895@spearce.org","subject":"Re: [PATCH] bash: offer only paths after '--'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-08T23:23:01Z","receivedAt":"2008-07-08T23:23:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Junio C Hamano wrote:\n>> SZEDER Gábor writes:\n>> \n>> > Hope that I got the commit message right (;\n>> \n>> It was very readable. Thanks.\n>\n> Acked-by: Shawn O. Pearce \n>\n>> > +__git_has_doubledash ()\n>> > +{\n>> > +\tlocal c=1\n>> > +\twhile [ $c -lt $COMP_CWORD ]; do\n>> > +\t\tif [ \"--\" = \"${COMP_WORDS[c]}\" ]; then\n>> > +\t\t\treturn 0\n>> > +\t\tfi\n>> > +\t\tc=$((++c))\n>> \n>> This assignment is somewhat curious, although it should work as expected\n>> either way ;-)\n>\n> I agree, its damned odd. But we already do this in the same\n> sort of loop inside of _git_branch() (see around line 541 in\n> next). This new patch is only sticking with our current set\n> of conventions in the script, so I say its fine.\n\nChuckling... Thanks for sanity checking and an Ack.\n"},{"id":"82649","messageId":"20080708235153.GD8224@neumann","threadId":"14330","inReplyTo":"20080708231837.GA16895@spearce.org","subject":"Re: [PATCH] bash: offer only paths after '--'","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2008-07-08T23:51:53Z","receivedAt":"2008-07-08T23:51:53Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Jul 08, 2008 at 11:18:37PM +0000, Shawn O. Pearce wrote:\n> Junio C Hamano wrote:\n> > SZEDER Gábor writes:\n> > > +\t\tc=$((++c))\n> > \n> > This assignment is somewhat curious, although it should work as expected\n> > either way ;-)\n> \n> I agree, its damned odd. But we already do this in the same\n> sort of loop inside of _git_branch() (see around line 541 in\n> next). This new patch is only sticking with our current set\n> of conventions in the script, so I say its fine.\nWell, according to\n\n git blame contrib/completion/git-completion.bash |grep '++'\n\nyou started this convention back in 2006, I just copied and modified\nyour code (;\n\nMaybe an old C++ \"heritage\"? In C++ it matters for class types (e.g.\niterators), because the postfix operator might be slower than the\nprefix.\n\nBest,\nGábor\n"},{"id":"82650","messageId":"20080708235538.GB17263@spearce.org","threadId":"14330","inReplyTo":"20080708235153.GD8224@neumann","subject":"Re: [PATCH] bash: offer only paths after '--'","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-08T23:55:38Z","receivedAt":"2008-07-08T23:55:38Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"SZEDER GGGbor wrote:\n> On Tue, Jul 08, 2008 at 11:18:37PM +0000, Shawn O. Pearce wrote:\n> > Junio C Hamano wrote:\n> > > SZEDER Gábor writes:\n> > > > +\t\tc=$((++c))\n> > > \n> > > This assignment is somewhat curious, although it should work as expected\n> > > either way ;-)\n>\n> Well, according to\n> \n> git blame contrib/completion/git-completion.bash |grep '++'\n> \n> you started this convention back in 2006, I just copied and modified\n> your code (;\n\nYea, I don't doubt it was me that did this.\n \n> Maybe an old C++ \"heritage\"? In C++ it matters for class types (e.g.\n> iterators), because the postfix operator might be slower than the\n> prefix.\n\nUnlikely, but maybe. I'm not really a C++ programmer. I tend to\navoid C++ when/if I am given the chance to do so.\n\n-- \nShawn.\n"},{"id":"82653","messageId":"7vprpnq5t8.fsf@gitster.siamese.dyndns.org","threadId":"14330","inReplyTo":"20080708235153.GD8224@neumann","subject":"Re: [PATCH] bash: offer only paths after '--'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-09T00:06:43Z","receivedAt":"2008-07-09T00:06:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> On Tue, Jul 08, 2008 at 11:18:37PM +0000, Shawn O. Pearce wrote:\n>> Junio C Hamano wrote:\n>> > SZEDER Gábor writes:\n>> > > +\t\tc=$((++c))\n>> > \n>> > This assignment is somewhat curious, although it should work as expected\n>> > either way ;-)\n> ...\n> Maybe an old C++ \"heritage\"? In C++ it matters for class types (e.g.\n> iterators), because the postfix operator might be slower than the\n> prefix.\n\nHeh, I was not talking about prefix vs postfix but about the assignment\ninto the variable that is incremented as a side effect of evaluating the\nleft hand side. If you know the variable is incremented already there is\nno point in assigning the resulting value to it ;-)\n\n\tc=$(( $c + 1 ))\n\nwould have avoided such an uneasy feeling, and would have been more\nportable. Even though $((x)) and $(($x)) are supposed to evaluate the\nsame, some shells do not like dollar-less variable names in arithmetic\nexpansion, and prefix/postfix increment/decrement are not required to be\nsupported by POSIX.\n\nBut this script being bash completion, we can use as much bashism as we\nwant here; perhaps I would have written:\n\n\t: $((c++))\n \n"}]}