git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Todd Zullinger <tmz@pobox.com>
To: git@vger.kernel.org
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [BUG] completion.commands does not remove multiple commands
Date: Thu, 28 Feb 2019 17:31:23 -0500	[thread overview]
Message-ID: <20190228223123.GZ16125@zaya.teonanacatl.net> (raw)

Hi,

I was playing with the completion.commands config added in
6532f3740b ("completion: allow to customize the completable
command list", 2018-05-20) and noticed an issue removing
multiple commands.

I wanted to remove completion for cherry and mergetool, so I
added them both to the config:

    git config completion.commands "-cherry -mergetool"

But git still completes cherry in this case, only removing
mergetool.  Swapping the items in the config yields the
opposite result (cherry is removed and mergetool is not).

With luck this will be a clear and easily resolved issue in
list_cmds_by_config() (in help.c).

Here's an example in test form:

-- 8< --
diff --git c/t/t9902-completion.sh w/t/t9902-completion.sh
index 3a2c6326d8..06cee36ae5 100755
--- c/t/t9902-completion.sh
+++ w/t/t9902-completion.sh
@@ -1483,6 +1483,14 @@ test_expect_success 'git --help completion' '
 	test_completion "git --help core" "core-tutorial "
 '
 
+test_expect_failure 'completion.commands removes multiple commands' '
+	test_config completion.commands "-cherry -mergetool" &&
+	git --list-cmds=list-mainporcelain,list-complete,config |
+		grep ^cherry >actual &&
+	echo cherry-pick >expected &&
+	test_cmp expected actual
+'
+
 test_expect_success 'setup for integration tests' '
 	echo content >file1 &&
 	echo more >file2 &&
-- 8< --

That's not quite normal form for t9902, but I was undable to
create a working test using the test_completion functions.
The tests set GIT_TESTING_PORCELAIN_COMMAND_LIST and
GIT_TESTING_ALL_COMMAND_LIST which override the settings in
the completion script.  I played a bit with adjusting those
to add cherry{,-pick} to the command lists, unsetting those
vars for the test, and such, to no avail.

In the end, I'm not really sure that calling --list-cmds
directly is such a bad way to go for testing this issue.

-- 
Todd

             reply	other threads:[~2019-02-28 22:31 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-28 22:31 Todd Zullinger [this message]
2019-02-28 23:05 ` [BUG] completion.commands does not remove multiple commands Jeff King
2019-03-01 17:34   ` Todd Zullinger
2019-03-01 18:30     ` Jeff King
2019-03-01 22:15       ` Todd Zullinger
2019-03-01 23:08         ` Jeff King
2019-03-02  1:08           ` Duy Nguyen
2019-03-02  1:18         ` Junio C Hamano
2019-03-02  2:40           ` Todd Zullinger
2019-03-02  4:07             ` SZEDER Gábor
2019-03-03  1:34               ` Todd Zullinger
2019-03-03 17:06               ` Jeff King
2019-03-01 17:34   ` [PATCH 1/3] doc: note config file restrictions for completion.commands Todd Zullinger
2019-03-17 13:12     ` Duy Nguyen
2019-03-17 18:16       ` [PATCH v2 0/4] completion.commands: fix multiple command removals Todd Zullinger
2019-03-17 18:16       ` [PATCH v2 1/4] git: read local config in --list-cmds Todd Zullinger
2019-03-18  9:41         ` Duy Nguyen
2019-03-20 18:03           ` [PATCH v3 0/4] completion.commands: fix multiple command removals Todd Zullinger
2019-03-21  2:58             ` Junio C Hamano
2019-03-21 17:18               ` Todd Zullinger
2019-03-21  9:45             ` Duy Nguyen
2019-03-20 18:03           ` [PATCH v3 1/4] git: read local config in --list-cmds Todd Zullinger
2019-03-20 18:03           ` [PATCH v3 2/4] t9902: test multiple removals via completion.commands Todd Zullinger
2019-03-20 18:03           ` [PATCH v3 3/4] completion: fix multiple command removals Todd Zullinger
2019-03-20 18:03           ` [PATCH v3 4/4] completion: use __git when calling --list-cmds Todd Zullinger
2019-03-17 18:16       ` [PATCH v2 2/4] t9902: test multiple removals via completion.commands Todd Zullinger
2019-03-17 18:16       ` [PATCH v2 3/4] completion: fix multiple command removals Todd Zullinger
2019-03-17 18:16       ` [PATCH v2 4/4] completion: use __git when calling --list-cmds Todd Zullinger
2019-03-01 17:34   ` [PATCH 2/3] t9902: test multiple removals via completion.commands Todd Zullinger
2019-03-01 18:22     ` Eric Sunshine
2019-03-01 20:50       ` SZEDER Gábor
2019-03-01 21:56         ` Todd Zullinger
2019-03-01 17:34   ` [PATCH 3/3] completion: fix multiple command removals Todd Zullinger
2019-03-01 18:16     ` Jeff King
2019-02-28 23:05 ` [BUG] completion.commands does not remove multiple commands SZEDER Gábor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190228223123.GZ16125@zaya.teonanacatl.net \
    --to=tmz@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).