[PATCH] completion: add branch options --contains --merged --no-merged

[PATCH] completion: add branch options --contains --merged --no-merged

[Eric Raible]

Signed-off-by: Eric Raible <raible@gmail.com>
---
 contrib/completion/git-completion.bash |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 0eb8df0..22e109d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -546,7 +546,7 @@ _git_branch ()
 	--*)
 		__gitcomp "
 			--color --no-color --verbose --abbrev= --no-abbrev
-			--track --no-track
+			--track --no-track --contains --merged --no-merged
 			"
 		;;
 	*)
-- 
1.5.6.1.1071.g76fb.dirty

Re: [PATCH] completion: add branch options --contains --merged --no-merged

[Shawn O. Pearce]

Eric Raible <raible@gmail.com> wrote:
> Signed-off-by: Eric Raible <raible@gmail.com>

Trivially-Acked-by: Shawn O. Pearce <spearce@spearce.org>

;-)

More completion support that probably should go to maint, as the
functionality in git-branch is in 1.5.6 but we (again) forgot to
make sure the completion was up-to-date prior to release.

> ---
>  contrib/completion/git-completion.bash |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 0eb8df0..22e109d 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -546,7 +546,7 @@ _git_branch ()
>  	--*)
>  		__gitcomp "
>  			--color --no-color --verbose --abbrev= --no-abbrev
> -			--track --no-track
> +			--track --no-track --contains --merged --no-merged
>  			"
>  		;;
>  	*)
> -- 
> 1.5.6.1.1071.g76fb.dirty

-- 
Shawn.

Re: [PATCH] completion: add branch options --contains --merged --no-merged

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Eric Raible <raible@gmail.com> wrote:
>> Signed-off-by: Eric Raible <raible@gmail.com>
>
> Trivially-Acked-by: Shawn O. Pearce <spearce@spearce.org>
>
> ;-)
>
> More completion support that probably should go to maint, as the
> functionality in git-branch is in 1.5.6 but we (again) forgot to
> make sure the completion was up-to-date prior to release.

I am actually getting more worried about completion code getting larger
and larger without its performance impact not being looked at nor
addressed adequately.  In my regular working tree:

	$ echo Docu<TAB>

completes "mentation/" instantly, but:

	$ git log -- Docu<TAB>

takes about 1.5 to 2 seconds to complete the same.

Re: [PATCH] completion: add branch options --contains --merged --no-merged

[Johannes Schindelin]

Hi,

On Mon, 7 Jul 2008, Junio C Hamano wrote:

> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > Eric Raible <raible@gmail.com> wrote:
> >> Signed-off-by: Eric Raible <raible@gmail.com>
> >
> > Trivially-Acked-by: Shawn O. Pearce <spearce@spearce.org>
> >
> > ;-)
> >
> > More completion support that probably should go to maint, as the
> > functionality in git-branch is in 1.5.6 but we (again) forgot to
> > make sure the completion was up-to-date prior to release.
> 
> I am actually getting more worried about completion code getting larger
> and larger without its performance impact not being looked at nor
> addressed adequately.  In my regular working tree:
> 
> 	$ echo Docu<TAB>
> 
> completes "mentation/" instantly, but:
> 
> 	$ git log -- Docu<TAB>
> 
> takes about 1.5 to 2 seconds to complete the same.

I noticed that myself, but did not have time to look into it.

It shows two bugs, actually: completions do not care about "--", and 
completing refs takes way too long.

Ciao,
Dscho

[PATCH] bash: offer only paths after '--'

[SZEDER Gábor]

Many git commands use '--' to separate subcommands, options, and refs
from paths.  However, the programmable completion for several of these
commands does not respect the '--', and offer subcommands, options, or
refs after a '--', although only paths are permitted.  e.g. 'git bisect
-- <TAB>' offers subcommands, 'git log -- --<TAB>' offers options and
'git log -- git<TAB>' offers all gitgui tags.

The completion for the following commands share this wrong behaviour:
  am add bisect commit diff log reset shortlog submodule gitk.

To avoid this, we check the presence of a '--' on the command line first
and let the shell do filename completion, if one is found.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---

On Tue, Jul 08, 2008 at 01:36:43PM +0200, Johannes Schindelin wrote:
> It shows two bugs, actually: completions do not care about "--", 
I think I have found and corrected all the places where '--' was not
handled properly, but might have overlooked something.

Hope that I got the commit message right (;


 contrib/completion/git-completion.bash |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6a15522..e7d8a75 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -451,6 +451,18 @@ __git_find_subcommand ()
 	done
 }
 
+__git_has_doubledash ()
+{
+	local c=1
+	while [ $c -lt $COMP_CWORD ]; do
+		if [ "--" = "${COMP_WORDS[c]}" ]; then
+			return 0
+		fi
+		c=$((++c))
+	done
+	return 1
+}
+
 __git_whitespacelist="nowarn warn error error-all strip"
 
 _git_am ()
@@ -497,6 +509,8 @@ _git_apply ()
 
 _git_add ()
 {
+	__git_has_doubledash && return
+
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--*)
@@ -511,6 +525,8 @@ _git_add ()
 
 _git_bisect ()
 {
+	__git_has_doubledash && return
+
 	local subcommands="start bad good skip reset visualize replay log run"
 	local subcommand="$(__git_find_subcommand "$subcommands")"
 	if [ -z "$subcommand" ]; then
@@ -612,6 +628,8 @@ _git_cherry_pick ()
 
 _git_commit ()
 {
+	__git_has_doubledash && return
+
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--*)
@@ -631,6 +649,8 @@ _git_describe ()
 
 _git_diff ()
 {
+	__git_has_doubledash && return
+
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--*)
@@ -733,6 +753,8 @@ _git_ls_tree ()
 
 _git_log ()
 {
+	__git_has_doubledash && return
+
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--pretty=*)
@@ -1088,6 +1110,8 @@ _git_remote ()
 
 _git_reset ()
 {
+	__git_has_doubledash && return
+
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--*)
@@ -1100,6 +1124,8 @@ _git_reset ()
 
 _git_shortlog ()
 {
+	__git_has_doubledash && return
+
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--*)
@@ -1157,6 +1183,8 @@ _git_stash ()
 
 _git_submodule ()
 {
+	__git_has_doubledash && return
+
 	local subcommands="add status init update"
 	if [ -z "$(__git_find_subcommand "$subcommands")" ]; then
 		local cur="${COMP_WORDS[COMP_CWORD]}"
@@ -1362,6 +1390,8 @@ _git ()
 
 _gitk ()
 {
+	__git_has_doubledash && return
+
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	local g="$(git rev-parse --git-dir 2>/dev/null)"
 	local merge=""
-- 
1.5.6.1.118.g82b2fef

Re: [PATCH] bash: offer only paths after '--'

[Eric Raible]

SZEDER Gábor <szeder <at> ira.uka.de> writes:

> 
> Many git commands use '--' to separate subcommands, options, and refs
> from paths.  However, the programmable completion for several of these
> commands does not respect the '--', and offer subcommands, options, or
> refs after a '--', although only paths are permitted.  e.g. 'git bisect

I like this change, but how about also offering a plain '--' as one
of the completion choices as a way of reminding newbies that the
command in question is one of the ones that takes filenames after
all options?

Re: [PATCH] bash: offer only paths after '--'

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> Hope that I got the commit message right (;

It was very readable.  Thanks.

> +__git_has_doubledash ()
> +{
> +	local c=1
> +	while [ $c -lt $COMP_CWORD ]; do
> +		if [ "--" = "${COMP_WORDS[c]}" ]; then
> +			return 0
> +		fi
> +		c=$((++c))

This assignment is somewhat curious, although it should work as expected
either way ;-)

Shawn?

Re: [PATCH] bash: offer only paths after '--'

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > Hope that I got the commit message right (;
> 
> It was very readable.  Thanks.

Acked-by: Shawn O. Pearce <spearce@spearce.org>

> > +__git_has_doubledash ()
> > +{
> > +	local c=1
> > +	while [ $c -lt $COMP_CWORD ]; do
> > +		if [ "--" = "${COMP_WORDS[c]}" ]; then
> > +			return 0
> > +		fi
> > +		c=$((++c))
> 
> This assignment is somewhat curious, although it should work as expected
> either way ;-)

I agree, its damned odd.  But we already do this in the same
sort of loop inside of _git_branch() (see around line 541 in
next).  This new patch is only sticking with our current set
of conventions in the script, so I say its fine.

-- 
Shawn.

Re: [PATCH] bash: offer only paths after '--'

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Junio C Hamano <gitster@pobox.com> wrote:
>> SZEDER Gábor <szeder@ira.uka.de> writes:
>> 
>> > Hope that I got the commit message right (;
>> 
>> It was very readable.  Thanks.
>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
>
>> > +__git_has_doubledash ()
>> > +{
>> > +	local c=1
>> > +	while [ $c -lt $COMP_CWORD ]; do
>> > +		if [ "--" = "${COMP_WORDS[c]}" ]; then
>> > +			return 0
>> > +		fi
>> > +		c=$((++c))
>> 
>> This assignment is somewhat curious, although it should work as expected
>> either way ;-)
>
> I agree, its damned odd.  But we already do this in the same
> sort of loop inside of _git_branch() (see around line 541 in
> next).  This new patch is only sticking with our current set
> of conventions in the script, so I say its fine.

Chuckling...  Thanks for sanity checking and an Ack.

Re: [PATCH] bash: offer only paths after '--'

[SZEDER Gábor]

On Tue, Jul 08, 2008 at 11:18:37PM +0000, Shawn O. Pearce wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
> > SZEDER Gábor <szeder@ira.uka.de> writes:
> > > +		c=$((++c))
> > 
> > This assignment is somewhat curious, although it should work as expected
> > either way ;-)
> 
> I agree, its damned odd.  But we already do this in the same
> sort of loop inside of _git_branch() (see around line 541 in
> next).  This new patch is only sticking with our current set
> of conventions in the script, so I say its fine.
Well, according to

  git blame contrib/completion/git-completion.bash  |grep '++'

you started this convention back in 2006, I just copied and modified
your code (;

Maybe an old C++ "heritage"?  In C++ it matters for class types (e.g.
iterators), because the postfix operator might be slower than the
prefix.

Best,
Gábor

Re: [PATCH] bash: offer only paths after '--'

[Shawn O. Pearce]

SZEDER GGGbor <szeder@ira.uka.de> wrote:
> On Tue, Jul 08, 2008 at 11:18:37PM +0000, Shawn O. Pearce wrote:
> > Junio C Hamano <gitster@pobox.com> wrote:
> > > SZEDER Gábor <szeder@ira.uka.de> writes:
> > > > +		c=$((++c))
> > > 
> > > This assignment is somewhat curious, although it should work as expected
> > > either way ;-)
>
> Well, according to
> 
>   git blame contrib/completion/git-completion.bash  |grep '++'
> 
> you started this convention back in 2006, I just copied and modified
> your code (;

Yea, I don't doubt it was me that did this.
 
> Maybe an old C++ "heritage"?  In C++ it matters for class types (e.g.
> iterators), because the postfix operator might be slower than the
> prefix.

Unlikely, but maybe.  I'm not really a C++ programmer.  I tend to
avoid C++ when/if I am given the chance to do so.

-- 
Shawn.

Re: [PATCH] bash: offer only paths after '--'

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> On Tue, Jul 08, 2008 at 11:18:37PM +0000, Shawn O. Pearce wrote:
>> Junio C Hamano <gitster@pobox.com> wrote:
>> > SZEDER Gábor <szeder@ira.uka.de> writes:
>> > > +		c=$((++c))
>> > 
>> > This assignment is somewhat curious, although it should work as expected
>> > either way ;-)
> ...
> Maybe an old C++ "heritage"?  In C++ it matters for class types (e.g.
> iterators), because the postfix operator might be slower than the
> prefix.

Heh, I was not talking about prefix vs postfix but about the assignment
into the variable that is incremented as a side effect of evaluating the
left hand side.  If you know the variable is incremented already there is
no point in assigning the resulting value to it ;-)

	c=$(( $c + 1 ))

would have avoided such an uneasy feeling, and would have been more
portable.  Even though $((x)) and $(($x)) are supposed to evaluate the
same, some shells do not like dollar-less variable names in arithmetic
expansion, and prefix/postfix increment/decrement are not required to be
supported by POSIX.

But this script being bash completion, we can use as much bashism as we
want here; perhaps I would have written:

	: $((c++))