Re: [PATCH v2 1/2] help: use list_aliases() for alias listing and lookup
From: Junio C Hamano <hidden>
Date: 2026-02-10 19:28:02
Jonatan Holmgren [off-list ref] writes:
quoted hunk
help.c has its own get_alias() config callback that duplicates the parsing logic in alias.c. Consolidate by teaching list_aliases() to also store the alias values (via the string_list util field), then use it in list_all_cmds_help_aliases() instead of the private callback. While at it, switch git_unknown_cmd_config() from skip_prefix() to parse_config_key() for alias parsing, which properly handles the config key structure and prepares for multi-level alias config keys in a subsequent commit. No functional change intended. Signed-off-by: Jonatan Holmgren <redacted> --- alias.c | 4 +++- help.c | 26 ++++++++------------------ 2 files changed, 11 insertions(+), 19 deletions(-)diff --git a/alias.c b/alias.c index 1a1a141a0a..c66a6095bb 100644 --- a/alias.c +++ b/alias.c@@ -29,7 +29,9 @@ static int config_alias_cb(const char *key, const char *value, key, value); } } else if (data->list) { - string_list_append(data->list, p); + if (value) + string_list_append(data->list, p)->util = + xstrdup(value); }
If !value, the original still added p to data->list, but the updated
code discards p when value is not there. Is that an intended change?
If not,
} else if (data->list) {
struct string_list_item *item;
item = string_list_append(data->list, p);
if (value)
item->util = xstrdup(value);
}
perhaps.
-static int get_alias(const char *var, const char *value,
- const struct config_context *ctx UNUSED, void *data)
-{
- struct string_list *list = data;
-
- if (skip_prefix(var, "alias.", &var)) {
- if (!value)
- return config_error_nonbool(var);
- string_list_append(list, var)->util = xstrdup(value);
- }
-
- return 0;
-}
-A value-less [alias] foo used to get an configuuration error with a friendly message from help.c:get_alias(), which was removed. The config_alias_cb() called by alias.c:list_aliases() either silently ignores foo altogether (the posted patch) or creates an entry for 'foo' but leaves its expansion to NULL (the above "silent ignore fix"). Either way, there needs some new code to compensate for the loss of the error detection somehow.
quoted hunk
@@ -501,7 +488,7 @@ static void list_all_cmds_help_aliases(int longest) struct cmdname_help *aliases; int i; - repo_config(the_repository, get_alias, &alias_list); + list_aliases(&alias_list); string_list_sort(&alias_list); for (i = 0; i < alias_list.nr; i++) {
OK.
quoted hunk
@@ -586,7 +573,8 @@ static int git_unknown_cmd_config(const char *var, const char *value, void *cb) { struct help_unknown_cmd_config *cfg = cb; - const char *p; + const char *subsection, *key; + size_t subsection_len; if (!strcmp(var, "help.autocorrect")) { int v = parse_autocorrect(value);@@ -601,8 +589,10 @@ static int git_unknown_cmd_config(const char *var, const char *value, } /* Also use aliases for command lookup */ - if (skip_prefix(var, "alias.", &p)) - add_cmdname(&cfg->aliases, p, strlen(p)); + if (!parse_config_key(var, "alias", &subsection, &subsection_len, &key)) { + if (!subsection) + add_cmdname(&cfg->aliases, key, strlen(key)); + } return 0; }
Arguably, the last two hunks are about preparing for three-level
alias.*.command support.
It is a bit unfortunate that with
[alias "foo"]
command = !date
bar = !echo bar
in your configuration,
$ git foo.command
$ git foo.bar
used to invoke the alias 'foo.command' and 'foo.bar' just fine, but
now with these two preparatory hunks, it no longer is the case and
they are silently ignored. With the next patch, 'git foo' starts
working in place for 'git foo.command', but 'git foo.bar' has become
forever inaccessible. I wonder if we want to warn about foo.bar if
not foo.command, or if it is too much? It is conceivable that we
may add variables like alias.*.help so it may not be a great idea to
warn on anything alias.<subsection>.<key> where <key> is not "command"
Perhaps we can claim that we are fixing a bug that allowed aliases
with a dot in its name by mistake? I dunno. No matter what we
claim here, some people will be hit by this behaviour change and
complain about a regression X-<.