Re: [PATCH v4 1/3] help: use list_aliases() for alias listing
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 11, 2026, 22:29 UTC
- Message-ID
- <xmqqldgyrk24.fsf@gitster.g>
- In-Reply-To
- <20260211211810.278806-2-jonatan@jontes.page>
Jonatan Holmgren <jonatan@jontes.page> writes:
Show 17 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. > > This preserves the existing error checking for value-less alias > definitions by checking in alias.c rather than help.c. > > No functional change intended. > > Signed-off-by: Jonatan Holmgren <jonatan@jontes.page> > --- > alias.c | 8 +++++++- > help.c | 17 ++--------------- > t/t0014-alias.sh | 10 ++++++++++ > 3 files changed, 19 insertions(+), 16 deletions(-)
Looks good. There is a small functional change not on the help.c:get_alias() side (i.e., "git help --all") but on the alias.c:list_aliases() side. "git --list-cmds=alias", used by command line completion, used to include such a broken alias, but it no longer does (and gets an error).
I think it is fine to call it a bugfix ;-)
Show 80 quoted lines
> diff --git a/alias.c b/alias.c
> index 1a1a141a0a..271acb9bf1 100644
> --- a/alias.c
> +++ b/alias.c
> @@ -29,7 +29,13 @@ static int config_alias_cb(const char *key, const char *value,
> key, value);
> }
> } else if (data->list) {
> - string_list_append(data->list, p);
> + struct string_list_item *item;
> +
> + if (!value)
> + return config_error_nonbool(key);
> +
> + item = string_list_append(data->list, p);
> + item->util = xstrdup(value);
> }
>
> return 0;
> diff --git a/help.c b/help.c
> index fefd811f7a..0bdb7ca10f 100644
> --- a/help.c
> +++ b/help.c
> @@ -20,6 +20,7 @@
> #include "prompt.h"
> #include "fsmonitor-ipc.h"
> #include "repository.h"
> +#include "alias.h"
>
> #ifndef NO_CURL
> #include "git-curl-compat.h" /* For LIBCURL_VERSION only */
> @@ -468,20 +469,6 @@ void list_developer_interfaces_help(void)
> putchar('\n');
> }
>
> -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;
> -}
> -
> static void list_all_cmds_help_external_commands(void)
> {
> struct string_list others = STRING_LIST_INIT_DUP;
> @@ -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++) {
> diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh
> index 07a53e7366..a13d2be8ca 100755
> --- a/t/t0014-alias.sh
> +++ b/t/t0014-alias.sh
> @@ -112,4 +112,14 @@ test_expect_success 'cannot alias-shadow a sample of regular builtins' '
> done
> '
>
> +test_expect_success 'alias without value reports error' '
> + test_when_finished "git config --unset alias.noval" &&
> + cat >>.git/config <<-\EOF &&
> + [alias]
> + noval
> + EOF
> + test_must_fail git noval 2>error &&
> + test_grep "alias.noval" error
> +'
> +
> test_done