diff mbox series

[3/3] completion: fix multiple command removals

Message ID 20190301173443.16429-4-tmz@pobox.com (mailing list archive)
State New, archived
Headers show
Series [1/3] doc: note config file restrictions for completion.commands | expand

Commit Message

Todd Zullinger March 1, 2019, 5:34 p.m. UTC 
From: Jeff King <peff@peff.net>

6532f3740b ("completion: allow to customize the completable
command list", 2018-05-20) added the completion.commands config
variable.

The documentation states multiple commands may be added,
separated by spaces.  Adding multiple commands to remove fails,
only removing the last command in the config.

Fix multiple command removals.

Signed-off-by: Jeff King <peff@peff.net>
---
Jeff,

The commit message could probably be worded better, particularly since it's
forged in your name.

 help.c                | 4 ++--
 t/t9902-completion.sh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Jeff King March 1, 2019, 6:16 p.m. UTC | #1 
On Fri, Mar 01, 2019 at 12:34:43PM -0500, Todd Zullinger wrote:

> The commit message could probably be worded better, particularly since it's
> forged in your name.

What you have is OK, but I'd have probably written it like this:

-- >8 --
Subject: [PATCH] completion: fix multiple command removals

Commit 6532f3740b ("completion: allow to customize the completable
command list", 2018-05-20) tried to allow multiple space-separated
entries in completion.commands. To do this, it copies each parsed token
into a strbuf so that the result is NUL-terminated.

However, for tokens starting with "-", it accidentally passes the
original non-terminated string, meaning that only the final one worked.
Switch to using the strbuf.

Reported-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 help.c                | 4 ++--
 t/t9902-completion.sh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/help.c b/help.c
index 520c9080e8..026f881715 100644
--- a/help.c
+++ b/help.c
@@ -393,8 +393,8 @@ void list_cmds_by_config(struct string_list *list)
 		const char *p = strchrnul(cmd_list, ' ');
 
 		strbuf_add(&sb, cmd_list, p - cmd_list);
-		if (*cmd_list == '-')
-			string_list_remove(list, cmd_list + 1, 0);
+		if (sb.buf[0] == '-')
+			string_list_remove(list, sb.buf + 1, 0);
 		else
 			string_list_insert(list, sb.buf);
 		strbuf_release(&sb);
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index dd11bb660d..d7daa1ca92 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1483,7 +1483,7 @@ test_expect_success 'git --help completion' '
 	test_completion "git --help core" "core-tutorial "
 '
 
-test_expect_failure 'completion.commands removes multiple commands' '
+test_expect_success 'completion.commands removes multiple commands' '
 	echo cherry-pick >expected &&
 	test_config_global completion.commands "-cherry -mergetool" &&
 	git --list-cmds=list-mainporcelain,list-complete,config |
diff mbox series

Patch

diff --git a/help.c b/help.c
index 520c9080e8..026f881715 100644
--- a/help.c
+++ b/help.c
@@ -393,8 +393,8 @@  void list_cmds_by_config(struct string_list *list)
 		const char *p = strchrnul(cmd_list, ' ');
 
 		strbuf_add(&sb, cmd_list, p - cmd_list);
-		if (*cmd_list == '-')
-			string_list_remove(list, cmd_list + 1, 0);
+		if (sb.buf[0] == '-')
+			string_list_remove(list, sb.buf + 1, 0);
 		else
 			string_list_insert(list, sb.buf);
 		strbuf_release(&sb);
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index dd11bb660d..d7daa1ca92 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1483,7 +1483,7 @@  test_expect_success 'git --help completion' '
 	test_completion "git --help core" "core-tutorial "
 '
 
-test_expect_failure 'completion.commands removes multiple commands' '
+test_expect_success 'completion.commands removes multiple commands' '
 	echo cherry-pick >expected &&
 	test_config_global completion.commands "-cherry -mergetool" &&
 	git --list-cmds=list-mainporcelain,list-complete,config |