Re: [PATCH v2 1/2] help: use list_aliases() for alias listing and lookup

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 10, 2026, 19:27 UTC
Message-ID: <xmqq3438z9ds.fsf@gitster.g>
In-Reply-To: <20260210183110.1151072-2-jonatan@jontes.page>

Jonatan Holmgren <jonatan@jontes.page> writes:

Show 32 quoted lines

> 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 <jonatan@jontes.page>
> ---
>  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.

Show 14 quoted lines

> -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.

Show 9 quoted lines

> @@ -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.

Show 23 quoted lines

> @@ -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-<.