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

From: Junio C Hamano <[email-masked]>
Date: Tue, 25 Feb 2014 15:22:24 -0800

Joel Nothman <[email-masked]> writes:

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

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

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

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