threads / patch / 40974

patchcompletion: fix completing unstuck email alias arguments

Subject: [PATCH] completion: fix completing unstuck email alias arguments

## tl;dr

3 messages between Dec 12, 2015 and Dec 13, 2015. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

SZEDER Gábor· Dec 12, 2015, 00:18 UTC · lore

Completing unstuck form of email aliases doesn't quite work:

  $ git send-email --to <TAB>
  alice   bob     cecil
  $ git send-email --to a<TAB>
  alice   bob     cecil

While listing email aliases works as expected, the second case should just complete to 'alice', but it keeps offering all email aliases instead.

The cause for this behavior is that in this case we mistakenly tell __gitcomp() explicitly that the current word to be completed is empty, while in reality it is not. As a result __gitcomp() doesn't filter out non-matching aliases, so all aliases end up being offered over and over again.

Fix this by not passing the current word to be completed to __gitcomp() and letting it go the default route and grab it from the '$cur' variable. Don't pass empty prefix either, because it's assumed to be empty when unspecified, so it's not necessary.

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

On top of the recently merged dfbe5eeb32 (completion: add support for completing email aliases, 2015-11-19).

 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to contrib/completion/git-completion.bash +1 −1

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 111b05302b..d9b995799c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1716,7 +1716,7 @@ _git_send_email ()
 	--to|--cc|--bcc|--from)
 		__gitcomp "
 		$(git --git-dir="$(__gitdir)" send-email --dump-aliases 2>/dev/null)
-		" "" ""
+		"
 		return
 		;;
 	esac

-- 
2.7.0.rc0.37.g77d69b9

Jacob Keller· Dec 12, 2015, 01:56 UTC · re: SZEDER Gábor · lore

Re: [PATCH] completion: fix completing unstuck email alias arguments

On Fri, Dec 11, 2015 at 4:18 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:

Show 7 quoted lines

> Completing unstuck form of email aliases doesn't quite work:
>
>   $ git send-email --to <TAB>
>   alice   bob     cecil
>   $ git send-email --to a<TAB>
>   alice   bob     cecil
>

Woops. Is it possible to add a test for this case? I honestly don't know how the tests for the completion works.

Show 34 quoted lines

> While listing email aliases works as expected, the second case should
> just complete to 'alice', but it keeps offering all email aliases
> instead.
>
> The cause for this behavior is that in this case we mistakenly tell
> __gitcomp() explicitly that the current word to be completed is empty,
> while in reality it is not.  As a result __gitcomp() doesn't filter
> out non-matching aliases, so all aliases end up being offered over and
> over again.
>
> Fix this by not passing the current word to be completed to
> __gitcomp() and letting it go the default route and grab it from the
> '$cur' variable.  Don't pass empty prefix either, because it's assumed
> to be empty when unspecified, so it's not necessary.
>
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> ---
>
> On top of the recently merged dfbe5eeb32 (completion: add support for
> completing email aliases, 2015-11-19).
>
>  contrib/completion/git-completion.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 111b05302b..d9b995799c 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1716,7 +1716,7 @@ _git_send_email ()
>         --to|--cc|--bcc|--from)
>                 __gitcomp "
>                 $(git --git-dir="$(__gitdir)" send-email --dump-aliases 2>/dev/null)
> -               " "" ""
> +               "

Yes this looks reasonable to me.

Show 6 quoted lines

>                 return
>                 ;;
>         esac
> --
> 2.7.0.rc0.37.g77d69b9
>

SZEDER Gábor· Dec 13, 2015, 01:00 UTC · re: Jacob Keller · lore

Re: [PATCH] completion: fix completing unstuck email alias arguments

Quoting Jacob Keller <jacob.keller@gmail.com>:

Show 11 quoted lines

> On Fri, Dec 11, 2015 at 4:18 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>> Completing unstuck form of email aliases doesn't quite work:
>>
>> $ git send-email --to <TAB>
>> alice   bob     cecil
>> $ git send-email --to a<TAB>
>> alice   bob     cecil
>>
>
> Woops. Is it possible to add a test for this case? I honestly don't
> know how the tests for the completion works.

It's possible, but I'm not sure we should.

It's a common pattern throughout the completion script to call __gitcomp() (or __gitcomp_nl()) this way, out of the almost 200 callsites many look like this one. We don't test all those, because it would require a lot of tests for not that much gain in coverage in my opinion. The interesting thing that's definitely worth testing is what happens inside the command substitution that supplies the possible completion words, but your tests added to the send-email testsuite already cover that thoroughly.

Gábor