On Fri, Mar 01, 2019 at 12:34:40PM -0500, Todd Zullinger wrote:
> Jeff King wrote:
> > I can reproduce your problem, though the test you included passes for me
> > even with the current tip of master.
>
> Oh, hrm. I think the issue is that completion.commands needs to be
> set in the global (or system-wide) config, via test_config_global
> rather than the local repo config which test_config sets.
>
> In hindsight, that seems obvious. But it's probably worth noting
> that where completion.commands is documented, for anyone who might
> spend a few cycles trying to configure it on a per-repo basis before
> realizing it doesn't work.
I think this is actually a bug. Normally code that is checking config
before we've decide to do setup_git_directory() would use
read_early_config(), which uses discover_git_directory() to tentatively
see if we're in a repo, and if so to add it to the config sequence.
But this code uses the caching configset mechanism. And that code
(rightly) does not use read_early_config(), because it has no idea if
it's being called early or what.
I think we probably ought to be doing something like this:
diff --git a/git.c b/git.c
index 2dd588674f..ba3690245e 100644
--- a/git.c
+++ b/git.c
@@ -62,6 +62,13 @@ static int list_cmds(const char *spec)
{
struct string_list list = STRING_LIST_INIT_DUP;
int i;
+ int nongit;
+
+ /*
+ * Set up the repository so we can pick up any repo-level config (like
+ * completion.commands).
+ */
+ setup_git_directory_gently(&nongit);
while (*spec) {
const char *sep = strchrnul(spec, ',');
> I'll leave it to those who know better to follow up with a change to
> use string_list_split().
It's less of an improvement than I'd hoped, since most of the code is
actually handling the "-" thing. So I don't think it's really worth the
churn. But here's what it looks like for reference:
---
help.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/help.c b/help.c
index 026f881715..9e4d258cb8 100644
--- a/help.c
+++ b/help.c
@@ -373,7 +373,9 @@ void list_cmds_by_category(struct string_list *list,
void list_cmds_by_config(struct string_list *list)
{
- const char *cmd_list;
+ const char *cmd_str;
+ struct string_list cmd_list = STRING_LIST_INIT_DUP;
+ int i;
/*
* There's no actual repository setup at this point (and even
@@ -382,26 +384,22 @@ void list_cmds_by_config(struct string_list *list)
* too since the caller (git --list-cmds=) should exit shortly
* anyway.
*/
- if (git_config_get_string_const("completion.commands", &cmd_list))
+ if (git_config_get_string_const("completion.commands", &cmd_str))
return;
string_list_sort(list);
string_list_remove_duplicates(list, 0);
- while (*cmd_list) {
- struct strbuf sb = STRBUF_INIT;
- const char *p = strchrnul(cmd_list, ' ');
+ string_list_split(&cmd_list, cmd_str, ' ', -1);
+ for (i = 0; i < cmd_list.nr; i++) {
+ const char *cmd = cmd_list.items[0].string;
- strbuf_add(&sb, cmd_list, p - cmd_list);
- if (sb.buf[0] == '-')
- string_list_remove(list, sb.buf + 1, 0);
+ if (*cmd == '-')
+ string_list_remove(list, cmd + 1, 0);
else
- string_list_insert(list, sb.buf);
- strbuf_release(&sb);
- while (*p == ' ')
- p++;
- cmd_list = p;
+ string_list_insert(list, cmd);
}
+ string_list_clear(&cmd_list, 0);
}
void list_common_guides_help(void)
