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@xxxxxxxxx> 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@xxxxxxxxx>
Signed-off-by: Jeff King <peff@xxxxxxxx>
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
