Re: [PATCH v3 1/8] git: add `deprecated` category to --list-cmds

From: Patrick Steinhardt <ps@pks.im>
Date: Sep 9, 2025, 06:43 UTC
Message-ID: <aL_MdiBLx9sssBwc@pks.im>
In-Reply-To: <bdc683a92b38884e9428cd4bade1f86960ef432b.1757345711.git.code@khaugsbakk.name>

On Mon, Sep 08, 2025 at 05:36:12PM +0200, kristofferhaugsbakk@fastmail.com wrote:

Show 9 quoted lines

> Notes (series):
>     v3 (new):
>     
>     This is something I wanted to submit independently until the point about
>     aliasing builtins was brought up.[1]  It will help (in a small way) with
>     the upcoming patch “git: allow alias-shadowing deprecated builtins”.
>     
>     By the way: should `command-list.txt` be updated in some way (I didn’t
>     know what way?)

I think it would be a good idea to introduce a new category for deprecated commands. After all, next to planning for the removal we should also actively discourage the use of such commands. But if they are still prominently featured in git(1) then people might be misled and use them, only to get a deprecation warning thrown at them.

Show 16 quoted lines

> diff --git a/git.c b/git.c
> index 83eac0aeab7..87d61f12594 100644
> --- a/git.c
> +++ b/git.c
> @@ -51,7 +52,13 @@ const char git_more_info_string[] =
>  
>  static int use_pager = -1;
>  
> -static void list_builtins(struct string_list *list, unsigned int exclude_option);
> +/*
> + * 'include_option' and 'exclude_option' are mutually exclusive.
> + *
> + * The default ('!include_option') is to include everything
> + * except those filtered out by 'exclude_option'.
> + */
> +static void list_builtins(struct string_list *list, unsigned int include_option, unsigned int exclude_option);

Nit: let's wrap this overly long line.

Show 6 quoted lines

> @@ -668,13 +677,20 @@ int is_builtin(const char *s)
>  	return !!get_builtin(s);
>  }
>  
> -static void list_builtins(struct string_list *out, unsigned int exclude_option)
> +static void list_builtins(struct string_list *out, unsigned int include_option, unsigned int exclude_option)

Same here.

Show 20 quoted lines

>  {
> -	for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
> -		if (exclude_option &&
> -		    (commands[i].option & exclude_option))
> -			continue;
> -		string_list_append(out, commands[i].cmd);
> +	if (include_option && exclude_option)
> +		BUG("'include_option' and 'exclude_option' are mutually exclusive");
> +	if (include_option) {
> +		for (size_t i = 0; i < ARRAY_SIZE(commands); i++)
> +			if (commands[i].option & include_option)
> +				string_list_append(out, commands[i].cmd);
> +	} else {
> +		for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
> +			if (commands[i].option & exclude_option)
> +				continue;
> +			string_list_append(out, commands[i].cmd);
> +		}
>  	}
>  }

We could combine these two loops into one:

    for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
        if (include_option && !(commands[i].option & include_option))
            continue;
        if (exclude_option && (commands[i].option & exclude_option))
            continue;
        string_list_append(out, commands[i].cmd);
    }

Which results in a bit less code duplication.

patrick