{"thread":{"id":"40974","subject":"[PATCH] completion: fix completing unstuck email alias arguments","startedAt":"2015-12-12T00:18:18Z","lastAt":"2015-12-13T01:00:38Z","messageCount":3,"participants":["SZEDER Gábor","Jacob Keller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"274307","messageId":"1449879498-1516-1-git-send-email-szeder@ira.uka.de","threadId":"40974","inReplyTo":null,"subject":"[PATCH] completion: fix completing unstuck email alias arguments","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-12-12T00:18:18Z","receivedAt":"2015-12-12T00:18:18Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Completing unstuck form of email aliases doesn't quite work:\n\n $ git send-email --to \n alice bob cecil\n $ git send-email --to a\n alice bob cecil\n\nWhile listing email aliases works as expected, the second case should\njust complete to 'alice', but it keeps offering all email aliases\ninstead.\n\nThe cause for this behavior is that in this case we mistakenly tell\n__gitcomp() explicitly that the current word to be completed is empty,\nwhile in reality it is not. As a result __gitcomp() doesn't filter\nout non-matching aliases, so all aliases end up being offered over and\nover again.\n\nFix this by not passing the current word to be completed to\n__gitcomp() and letting it go the default route and grab it from the\n'$cur' variable. Don't pass empty prefix either, because it's assumed\nto be empty when unspecified, so it's not necessary.\n\nSigned-off-by: SZEDER Gábor \n---\n\nOn top of the recently merged dfbe5eeb32 (completion: add support for\ncompleting email aliases, 2015-11-19).\n\n contrib/completion/git-completion.bash | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 111b05302b..d9b995799c 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1716,7 +1716,7 @@ _git_send_email ()\n \t--to|--cc|--bcc|--from)\n \t\t__gitcomp \"\n \t\t$(git --git-dir=\"$(__gitdir)\" send-email --dump-aliases 2>/dev/null)\n-\t\t\" \"\" \"\"\n+\t\t\"\n \t\treturn\n \t\t;;\n \tesac\n-- \n2.7.0.rc0.37.g77d69b9\n"},{"id":"274311","messageId":"CA+P7+xrHhWHp8WA8EF7jzsit50uW13=-NEzf0DwNfMyu2dckhA@mail.gmail.com","threadId":"40974","inReplyTo":"1449879498-1516-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH] completion: fix completing unstuck email alias arguments","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-12-12T01:56:46Z","receivedAt":"2015-12-12T01:56:46Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Dec 11, 2015 at 4:18 PM, SZEDER Gábor wrote:\n> Completing unstuck form of email aliases doesn't quite work:\n>\n> $ git send-email --to \n> alice bob cecil\n> $ git send-email --to a\n> alice bob cecil\n>\n\nWoops. Is it possible to add a test for this case? I honestly don't\nknow how the tests for the completion works.\n\n> While listing email aliases works as expected, the second case should\n> just complete to 'alice', but it keeps offering all email aliases\n> instead.\n>\n> The cause for this behavior is that in this case we mistakenly tell\n> __gitcomp() explicitly that the current word to be completed is empty,\n> while in reality it is not. As a result __gitcomp() doesn't filter\n> out non-matching aliases, so all aliases end up being offered over and\n> over again.\n>\n> Fix this by not passing the current word to be completed to\n> __gitcomp() and letting it go the default route and grab it from the\n> '$cur' variable. Don't pass empty prefix either, because it's assumed\n> to be empty when unspecified, so it's not necessary.\n>\n> Signed-off-by: SZEDER Gábor \n> ---\n>\n> On top of the recently merged dfbe5eeb32 (completion: add support for\n> completing email aliases, 2015-11-19).\n>\n> contrib/completion/git-completion.bash | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 111b05302b..d9b995799c 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1716,7 +1716,7 @@ _git_send_email ()\n> --to|--cc|--bcc|--from)\n> __gitcomp \"\n> $(git --git-dir=\"$(__gitdir)\" send-email --dump-aliases 2>/dev/null)\n> - \" \"\" \"\"\n> + \"\n\nYes this looks reasonable to me.\n\n> return\n> ;;\n> esac\n> --\n> 2.7.0.rc0.37.g77d69b9\n>\n"},{"id":"274331","messageId":"20151213020038.Horde.rlO28-SYLxFX00yK-iahGJs@webmail.informatik.kit.edu","threadId":"40974","inReplyTo":"CA+P7+xrHhWHp8WA8EF7jzsit50uW13=-NEzf0DwNfMyu2dckhA@mail.gmail.com","subject":"Re: [PATCH] completion: fix completing unstuck email alias arguments","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-12-13T01:00:38Z","receivedAt":"2015-12-13T01:00:38Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Jacob Keller :\n\n> On Fri, Dec 11, 2015 at 4:18 PM, SZEDER Gábor wrote:\n>> Completing unstuck form of email aliases doesn't quite work:\n>>\n>> $ git send-email --to \n>> alice bob cecil\n>> $ git send-email --to a\n>> alice bob cecil\n>>\n>\n> Woops. Is it possible to add a test for this case? I honestly don't\n> know how the tests for the completion works.\n\nIt's possible, but I'm not sure we should.\n\nIt's a common pattern throughout the completion script to call\n__gitcomp() (or __gitcomp_nl()) this way, out of the almost 200\ncallsites many look like this one. We don't test all those, because\nit would require a lot of tests for not that much gain in coverage\nin my opinion. The interesting thing that's definitely worth testing\nis what happens inside the command substitution that supplies the\npossible completion words, but your tests added to the send-email\ntestsuite already cover that thoroughly.\n\nGábor\n"}]}