Re: [PATCH] help: include list of aliases in git-help --all

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 25, 2014, 23:22 UTC
Message-ID: <xmqqmwhe4w27.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1393289315-28982-1-git-send-email-joel.nothman@gmail.com>

Joel Nothman <joel.nothman@gmail.com> writes:

Show 6 quoted lines

> Git help --all had listed all git commands, but no configured aliases.
> This includes aliases as a separate listing, after commands in the main
> git directory and other $PATH directories.
>
> Signed-off-by: Joel Nothman <joel.nothman <at> gmail.com>
> ---

Thanks.

Show 10 quoted lines

> diff --git a/help.c b/help.c
> index df7d16d..3c14af4 100644
> --- a/help.c
> +++ b/help.c
> @@ -86,7 +86,7 @@ static void pretty_print_string_list(struct cmdnames *cmds,
>  	int i;
>  
>  	for (i = 0; i < cmds->cnt; i++)
> -		string_list_append(&list, cmds->names[i]->name);
> +	    string_list_append(&list, cmds->names[i]->name);

Why?

Show 5 quoted lines

> @@ -202,8 +202,48 @@ void load_command_list(const char *prefix,
>  	exclude_cmds(other_cmds, main_cmds);
>  }
>  
> +static struct cmdnames aliases;

Instead of using a static global variable, perhaps make this an on-stack variable in load_commands_and_aliases() that is passed as a callback parameter to load_aliases_cb() thru git_config()?

> +static int load_aliases_cb(const char *var, const char *value, void *cb)
> +{

That is, cb here is the second parameter you gave to git_config().

Show 14 quoted lines

>  void list_commands(unsigned int colopts,
> -		   struct cmdnames *main_cmds, struct cmdnames *other_cmds)
> +		   struct cmdnames *main_cmds, struct cmdnames *other_cmds,
> +		   struct cmdnames *alias_cmds)
>  {
>  	if (main_cmds->cnt) {
>  		const char *exec_path = git_exec_path();
> @@ -219,6 +259,13 @@ void list_commands(unsigned int colopts,
>  		pretty_print_string_list(other_cmds, colopts);
>  		putchar('\n');
>  	}
> +
> +	if (alias_cmds->cnt) {
> +		printf_ln(_("aliases defined in git configuration"));

This will not break the use of "git help -a" in our completion script, because it ignores anything that does not begin with two SP followed by alphanumerics.

It may however break scripts that read from "help -a" done by other people that may remove the lines in the output that are known to them as not names of commands (i.e. "available git commands..." and "git commands avaliable elsewhere...")---they haven't seen this new string and would not know that this line must be skipped.

Other than that, looks reasonably cleanly done. We'd need a test to cover this so that other people will not break it in future patches.