From: Todd Zullinger <tmz@pobox.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
"Jeff King" <peff@peff.net>,
"SZEDER Gábor" <szeder.dev@gmail.com>,
"Eric Sunshine" <sunshine@sunshineco.com>
Subject: [PATCH v2 0/4] completion.commands: fix multiple command removals
Date: Sun, 17 Mar 2019 14:16:16 -0400 [thread overview]
Message-ID: <20190317181620.26727-1-tmz@pobox.com> (raw)
In-Reply-To: <CACsJy8BuR=syjT1gjTxXXKKaevzpbdRGp+je+rsX6jV96F3sbA@mail.gmail.com>
Hi,
Duy Nguyen wrote:
> Poking this thread before it goes completely dead...
I've been meaning to send out a re-rolled version. I didn't
realize 2 weeks had slipped by already. :/
> On Sat, Mar 2, 2019 at 12:34 AM Todd Zullinger <tmz@pobox.com> wrote:
>>
>> The completion.commands config var must be set in either the system-wide
>> or global config file. The completion script does not read a local
>> repository config.
>
> After the last discussion, I think the consensus was it's ok to allow
> per-repo config so we can just drop this and update the code to read
> from any config file, right?
Yeah. Here's an updated series which I hope sums up the changes
from the discussion.
Changes since v1:
- Change --list-commands to respect local config and use
test_config rather than test_config_global
- Avoid git upstream of pipes
- Check that both cherry and mergetool are removed
- Use __git in completion calls which use --list-cmds, to
ensure the proper git repo config is checked with git -C
/some/other/repo
Jeff King (2):
git: read local config in --list-cmds
completion: fix multiple command removals
Todd Zullinger (2):
t9902: test multiple removals via completion.commands
completion: use __git when calling --list-cmds
contrib/completion/git-completion.bash | 8 ++++----
git.c | 7 +++++++
help.c | 4 ++--
t/t9902-completion.sh | 6 ++++++
4 files changed, 19 insertions(+), 6 deletions(-)
Range-diff against v1:
1: 385c596510 < -: ---------- doc: note config file restrictions for completion.commands
-: ---------- > 1: e51bdea6d3 git: read local config in --list-cmds
2: ffa5ed9780 ! 2: 2f5e9da9de t9902: test multiple removals via completion.commands
@@ -18,11 +18,9 @@
'
+test_expect_failure 'completion.commands removes multiple commands' '
-+ echo cherry-pick >expected &&
-+ test_config_global completion.commands "-cherry -mergetool" &&
-+ git --list-cmds=list-mainporcelain,list-complete,config |
-+ grep ^cherry >actual &&
-+ test_cmp expected actual
++ test_config completion.commands "-cherry -mergetool" &&
++ git --list-cmds=list-mainporcelain,list-complete,config >out &&
++ ! grep -E "^(cherry|mergetool)$" out
+'
+
test_expect_success 'setup for integration tests' '
3: af029e908d ! 3: 7548dcc23f completion: fix multiple command removals
@@ -2,16 +2,16 @@
completion: fix multiple command removals
- 6532f3740b ("completion: allow to customize the completable
- command list", 2018-05-20) added the completion.commands config
- variable.
+ 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.
- 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.
+ 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>
diff --git a/help.c b/help.c
@@ -38,6 +38,6 @@
-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 |
+ test_config completion.commands "-cherry -mergetool" &&
+ git --list-cmds=list-mainporcelain,list-complete,config >out &&
+ ! grep -E "^(cherry|mergetool)$" out
-: ---------- > 4: 26bef0b2af completion: use __git when calling --list-cmds
next prev parent reply other threads:[~2019-03-17 18:16 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-28 22:31 [BUG] completion.commands does not remove multiple commands Todd Zullinger
2019-02-28 23:05 ` 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 ` Todd Zullinger [this message]
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=20190317181620.26727-1-tmz@pobox.com \
--to=tmz@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
--cc=sunshine@sunshineco.com \
--cc=szeder.dev@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).