[PATCH] git help: promote 'git help -av'

[PATCH] git help: promote 'git help -av'

[Nguyễn Thái Ngọc Duy]

When you type "git help" (or just "git") you are greeted with a list
with commonly used commands and their short description and are
suggested to use "git help -a" or "git help -g" for more details.

"git help -av" would be more friendly and inline with what is shown
with "git help" since it shows list of commands with description as
well, and commands are properly grouped.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 git.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git.c b/git.c
index a6f4b44af5..69c21f378b 100644
--- a/git.c
+++ b/git.c
@@ -31,7 +31,7 @@ const char git_usage_string[] =
 	   "           <command> [<args>]");
 
 const char git_more_info_string[] =
-	N_("'git help -a' and 'git help -g' list available subcommands and some\n"
+	N_("'git help -av' and 'git help -g' list available subcommands and some\n"
 	   "concept guides. See 'git help <command>' or 'git help <concept>'\n"
 	   "to read about a specific subcommand or concept.");
 
-- 
2.19.0.647.gb9a6049235

Re: [PATCH] git help: promote 'git help -av'

[Ævar Arnfjörð Bjarmason]

On Sat, Sep 22 2018, Nguyễn Thái Ngọc Duy wrote:

> When you type "git help" (or just "git") you are greeted with a list
> with commonly used commands and their short description and are
> suggested to use "git help -a" or "git help -g" for more details.
>
> "git help -av" would be more friendly and inline with what is shown
> with "git help" since it shows list of commands with description as
> well, and commands are properly grouped.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  git.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git.c b/git.c
> index a6f4b44af5..69c21f378b 100644
> --- a/git.c
> +++ b/git.c
> @@ -31,7 +31,7 @@ const char git_usage_string[] =
>  	   "           <command> [<args>]");
>
>  const char git_more_info_string[] =
> -	N_("'git help -a' and 'git help -g' list available subcommands and some\n"
> +	N_("'git help -av' and 'git help -g' list available subcommands and some\n"
>  	   "concept guides. See 'git help <command>' or 'git help <concept>'\n"
>  	   "to read about a specific subcommand or concept.");

A side-effect of this not noted in your commit message is that we'll now
invoke the pager, perhaps we should just do:

    diff --git a/builtin/help.c b/builtin/help.c
    index 8d4f6dd301..1a3b174aaf 100644
    --- a/builtin/help.c
    +++ b/builtin/help.c
    @@ -436,9 +436,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
            parsed_help_format = help_format;

            if (show_all) {
    +               setup_pager();
                    git_config(git_help_config, NULL);
                    if (verbose) {
    -                       setup_pager();
                            list_all_cmds_help();
                            return 0;
                    }
    @@ -460,8 +460,10 @@ int cmd_help(int argc, const char **argv, const char *prefix)
                    return 0;
            }

    -       if (show_guides)
    +       if (show_guides) {
    +               setup_pager();
                    list_common_guides_help();
    +       }

            if (show_all || show_guides) {
                    printf("%s\n", _(git_more_info_string));

Or is there a good reason we shouldn't invoke the pager for e.g. -g when
the terminal is too small (per our default less config)?

Another thing I noticed: We don't list -v in the git-help manpage, but
since we use OPT_VERBOSE it's supported.

Re: [PATCH] git help: promote 'git help -av'

[Duy Nguyen]

On Sat, Sep 22, 2018 at 9:29 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Sat, Sep 22 2018, Nguyễn Thái Ngọc Duy wrote:
>
> > When you type "git help" (or just "git") you are greeted with a list
> > with commonly used commands and their short description and are
> > suggested to use "git help -a" or "git help -g" for more details.
> >
> > "git help -av" would be more friendly and inline with what is shown
> > with "git help" since it shows list of commands with description as
> > well, and commands are properly grouped.
> >
> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> > ---
> >  git.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/git.c b/git.c
> > index a6f4b44af5..69c21f378b 100644
> > --- a/git.c
> > +++ b/git.c
> > @@ -31,7 +31,7 @@ const char git_usage_string[] =
> >          "           <command> [<args>]");
> >
> >  const char git_more_info_string[] =
> > -     N_("'git help -a' and 'git help -g' list available subcommands and some\n"
> > +     N_("'git help -av' and 'git help -g' list available subcommands and some\n"
> >          "concept guides. See 'git help <command>' or 'git help <concept>'\n"
> >          "to read about a specific subcommand or concept.");
>
> A side-effect of this not noted in your commit message is that we'll now
> invoke the pager, perhaps we should just do:
>
>     diff --git a/builtin/help.c b/builtin/help.c
>     index 8d4f6dd301..1a3b174aaf 100644
>     --- a/builtin/help.c
>     +++ b/builtin/help.c
>     @@ -436,9 +436,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>             parsed_help_format = help_format;
>
>             if (show_all) {
>     +               setup_pager();
>                     git_config(git_help_config, NULL);
>                     if (verbose) {
>     -                       setup_pager();
>                             list_all_cmds_help();
>                             return 0;
>                     }
>     @@ -460,8 +460,10 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>                     return 0;
>             }
>
>     -       if (show_guides)
>     +       if (show_guides) {
>     +               setup_pager();
>                     list_common_guides_help();
>     +       }
>
>             if (show_all || show_guides) {
>                     printf("%s\n", _(git_more_info_string));
>
> Or is there a good reason we shouldn't invoke the pager for e.g. -g when
> the terminal is too small (per our default less config)?

Different pagers may behave differently (and so far "help -a" still
fits in my screen). So I don't think we should invoke pager more than
necessary.
-- 
Duy

Re: [PATCH] git help: promote 'git help -av'

[Jeff King]

On Sat, Sep 22, 2018 at 07:47:07PM +0200, Nguyễn Thái Ngọc Duy wrote:

> When you type "git help" (or just "git") you are greeted with a list
> with commonly used commands and their short description and are
> suggested to use "git help -a" or "git help -g" for more details.
> 
> "git help -av" would be more friendly and inline with what is shown
> with "git help" since it shows list of commands with description as
> well, and commands are properly grouped.

I agree that "help -av" is likely to be more friendly. I kind of wonder
if it should just be the default for "-a". Do we have any obligation not
to change the format of that output?

Assuming we want to keep "-a" and "-av" as-is, your patch seems quite
sensible to me.

-Peff

Re: [PATCH] git help: promote 'git help -av'

[Taylor Blau]

On Mon, Sep 24, 2018 at 02:19:28PM -0400, Jeff King wrote:
> On Sat, Sep 22, 2018 at 07:47:07PM +0200, Nguyễn Thái Ngọc Duy wrote:
>
> > When you type "git help" (or just "git") you are greeted with a list
> > with commonly used commands and their short description and are
> > suggested to use "git help -a" or "git help -g" for more details.
> >
> > "git help -av" would be more friendly and inline with what is shown
> > with "git help" since it shows list of commands with description as
> > well, and commands are properly grouped.
>
> I agree that "help -av" is likely to be more friendly. I kind of wonder
> if it should just be the default for "-a". Do we have any obligation not
> to change the format of that output?

I agree, though I'd like to clarify what you said before doing so
wholeheartedly.

Did you mean that all existing uses of 'git help -a' should instead mean
'git help -av' (i.e., that '-a' after your proposed patch means the same
as '-av' in revisions prior to this one?)

If so, I agree. I can't imagine a case where I'd like to provide '-a'
and _not_ '-v', so certainly the later should come as a two-for-one
deal. Less, more well-intentioned knobs seems a positive trend to me, so
I am for this idea.

Thanks,
Taylor

Re: [PATCH] git help: promote 'git help -av'

[Jeff King]

On Mon, Sep 24, 2018 at 03:20:00PM -0500, Taylor Blau wrote:

> On Mon, Sep 24, 2018 at 02:19:28PM -0400, Jeff King wrote:
> > On Sat, Sep 22, 2018 at 07:47:07PM +0200, Nguyễn Thái Ngọc Duy wrote:
> >
> > > When you type "git help" (or just "git") you are greeted with a list
> > > with commonly used commands and their short description and are
> > > suggested to use "git help -a" or "git help -g" for more details.
> > >
> > > "git help -av" would be more friendly and inline with what is shown
> > > with "git help" since it shows list of commands with description as
> > > well, and commands are properly grouped.
> >
> > I agree that "help -av" is likely to be more friendly. I kind of wonder
> > if it should just be the default for "-a". Do we have any obligation not
> > to change the format of that output?
> 
> I agree, though I'd like to clarify what you said before doing so
> wholeheartedly.
> 
> Did you mean that all existing uses of 'git help -a' should instead mean
> 'git help -av' (i.e., that '-a' after your proposed patch means the same
> as '-av' in revisions prior to this one?)

Yes, exactly. I think the vast majority of uses would prefer the
categorized list.

The obvious exceptions are:

 - you can't remember the name of the command so an alphabetized list is
   easier for sifting through (without having to re-sift for each
   category).

 - you need a machine-readable version of the list (e.g., for
   programmable completion). We have "git --list-cmds", but we may need
   to advertise it better and mark it non-experimental.

I dunno. Maybe it is not worth the effort. Duy's existing patch is an
easy one liner. ;)

-Peff

Re: [PATCH] git help: promote 'git help -av'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I agree that "help -av" is likely to be more friendly. I kind of wonder
> if it should just be the default for "-a". Do we have any obligation not
> to change the format of that output?

I know that at least older versions of git-completion.bash (I think
it was up to 2.17.x series, but do not quote me on that) have parsed
"git help -a" output to obtain the list of commands.  So such a
change would impact those minority users who keep stale completion
script in their $HOME/.bashrc without ever update it, thinking it is
good enough.

I personally find "help -av" a bit too loud to my taste than plain
"-a", and more importantly, I look at "help -a" primarily to check
the last section "avaialble from elsewhere on your $PATH" to find
things like "clang-format", which I do not think is available
anywhere in "help -av", so I do not think "-av" can be promoted as
an upward-compatible replacement in its current form.

I probably am not a part of the target audience, though.

Re: [PATCH] git help: promote 'git help -av'

[Duy Nguyen]

On Mon, Sep 24, 2018 at 10:58 PM Junio C Hamano <gitster@pobox.com> wrote:
> I personally find "help -av" a bit too loud to my taste than plain
> "-a", and more importantly, I look at "help -a" primarily to check
> the last section "avaialble from elsewhere on your $PATH" to find
> things like "clang-format", which I do not think is available
> anywhere in "help -av", so I do not think "-av" can be promoted as
> an upward-compatible replacement in its current form.

Yep. I also thought "help -a" was denser but wasn't sure if it
actually helps or not. Whenever I look at that block of commands, I
end up searching anyway. For my use case, "help -a" could be better
served with something like "git apropos".

I think adding another section about external commands in "help -av"
would address the "clang-format" stuff. With that, it's probably good
enough to completely replace "help -a". It may also be good to list
aliases there too in a separate section so you have "all you can type"
in one (big) list.
--
Duy

Re: [PATCH] git help: promote 'git help -av'

[Duy Nguyen]

On Tue, Sep 25, 2018 at 05:15:38PM +0200, Duy Nguyen wrote:
> On Mon, Sep 24, 2018 at 10:58 PM Junio C Hamano <gitster@pobox.com> wrote:
> > I personally find "help -av" a bit too loud to my taste than plain
> > "-a", and more importantly, I look at "help -a" primarily to check
> > the last section "avaialble from elsewhere on your $PATH" to find
> > things like "clang-format", which I do not think is available
> > anywhere in "help -av", so I do not think "-av" can be promoted as
> > an upward-compatible replacement in its current form.
> 
> Yep. I also thought "help -a" was denser but wasn't sure if it
> actually helps or not. Whenever I look at that block of commands, I
> end up searching anyway. For my use case, "help -a" could be better
> served with something like "git apropos".
> 
> I think adding another section about external commands in "help -av"
> would address the "clang-format" stuff. With that, it's probably good
> enough to completely replace "help -a". It may also be good to list
> aliases there too in a separate section so you have "all you can type"
> in one (big) list.

Here's the patch that adds that external commands and aliases
sections. I feel that external commands section is definitely good to
have even if we don't replace "help -a". Aliases are more
subjective...

-- 8< --
diff --git a/builtin/help.c b/builtin/help.c
index 8d4f6dd301..23a34b36e7 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -38,7 +38,6 @@ static const char *html_path;
 static int show_all = 0;
 static int show_guides = 0;
 static int show_config;
-static int verbose;
 static unsigned int colopts;
 static enum help_format help_format = HELP_FORMAT_NONE;
 static int exclude_guides;
@@ -53,7 +52,6 @@ static struct option builtin_help_options[] = {
 			HELP_FORMAT_WEB),
 	OPT_SET_INT('i', "info", &help_format, N_("show info page"),
 			HELP_FORMAT_INFO),
-	OPT__VERBOSE(&verbose, N_("print command description")),
 	OPT_END(),
 };
 
@@ -437,14 +435,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	if (show_all) {
 		git_config(git_help_config, NULL);
-		if (verbose) {
-			setup_pager();
-			list_all_cmds_help();
-			return 0;
-		}
-		printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
-		load_command_list("git-", &main_cmds, &other_cmds);
-		list_commands(colopts, &main_cmds, &other_cmds);
+		setup_pager();
+		list_all_cmds_help();
+		return 0;
 	}
 
 	if (show_config) {
diff --git a/help.c b/help.c
index 96f6d221ed..4a168230dc 100644
--- a/help.c
+++ b/help.c
@@ -98,7 +98,8 @@ static int cmd_name_cmp(const void *elem1, const void *elem2)
 	return strcmp(e1->name, e2->name);
 }
 
-static void print_cmd_by_category(const struct category_description *catdesc)
+static void print_cmd_by_category(const struct category_description *catdesc,
+				  int *longest_p)
 {
 	struct cmdname_help *cmds;
 	int longest = 0;
@@ -124,6 +125,8 @@ static void print_cmd_by_category(const struct category_description *catdesc)
 		print_command_list(cmds, mask, longest);
 	}
 	free(cmds);
+	if (longest_p)
+		*longest_p = longest;
 }
 
 void add_cmdname(struct cmdnames *cmds, const char *name, int len)
@@ -193,26 +196,6 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
 	cmds->cnt = cj;
 }
 
-static void pretty_print_cmdnames(struct cmdnames *cmds, unsigned int colopts)
-{
-	struct string_list list = STRING_LIST_INIT_NODUP;
-	struct column_options copts;
-	int i;
-
-	for (i = 0; i < cmds->cnt; i++)
-		string_list_append(&list, cmds->names[i]->name);
-	/*
-	 * always enable column display, we only consult column.*
-	 * about layout strategy and stuff
-	 */
-	colopts = (colopts & ~COL_ENABLE_MASK) | COL_ENABLED;
-	memset(&copts, 0, sizeof(copts));
-	copts.indent = "  ";
-	copts.padding = 2;
-	print_columns(&list, colopts, &copts);
-	string_list_clear(&list, 0);
-}
-
 static void list_commands_in_dir(struct cmdnames *cmds,
 					 const char *path,
 					 const char *prefix)
@@ -285,29 +268,10 @@ void load_command_list(const char *prefix,
 	exclude_cmds(other_cmds, main_cmds);
 }
 
-void list_commands(unsigned int colopts,
-		   struct cmdnames *main_cmds, struct cmdnames *other_cmds)
-{
-	if (main_cmds->cnt) {
-		const char *exec_path = git_exec_path();
-		printf_ln(_("available git commands in '%s'"), exec_path);
-		putchar('\n');
-		pretty_print_cmdnames(main_cmds, colopts);
-		putchar('\n');
-	}
-
-	if (other_cmds->cnt) {
-		printf_ln(_("git commands available from elsewhere on your $PATH"));
-		putchar('\n');
-		pretty_print_cmdnames(other_cmds, colopts);
-		putchar('\n');
-	}
-}
-
 void list_common_cmds_help(void)
 {
 	puts(_("These are common Git commands used in various situations:"));
-	print_cmd_by_category(common_categories);
+	print_cmd_by_category(common_categories, NULL);
 }
 
 void list_all_main_cmds(struct string_list *list)
@@ -405,7 +369,7 @@ void list_common_guides_help(void)
 		{ CAT_guide, N_("The common Git guides are:") },
 		{ 0, NULL }
 	};
-	print_cmd_by_category(catdesc);
+	print_cmd_by_category(catdesc, NULL);
 	putchar('\n');
 }
 
@@ -494,9 +458,48 @@ void list_config_help(int for_human)
 	string_list_clear(&keys, 0);
 }
 
+static int get_alias(const char *var, const char *value, void *data)
+{
+	struct string_list *list = data;
+
+	if (skip_prefix(var, "alias.", &var))
+		string_list_append(list, var)->util = xstrdup(value);
+
+	return 0;
+}
+
 void list_all_cmds_help(void)
 {
-	print_cmd_by_category(main_categories);
+	struct string_list others = STRING_LIST_INIT_DUP;
+	struct string_list alias_list = STRING_LIST_INIT_DUP;
+	struct cmdname_help *aliases;
+	int i, longest;
+
+	printf_ln(_("See 'git help <command>' to read about a specific subcommand"));
+	print_cmd_by_category(main_categories, &longest);
+
+	list_all_other_cmds(&others);
+	if (others.nr)
+		printf("\n%s\n", _("External commands"));
+	for (i = 0; i < others.nr; i++)
+		printf("   %s\n", others.items[i].string);
+	string_list_clear(&others, 0);
+
+	git_config(get_alias, &alias_list);
+	string_list_sort(&alias_list);
+	if (alias_list.nr) {
+		printf("\n%s\n", _("Command aliases"));
+		ALLOC_ARRAY(aliases, alias_list.nr + 1);
+		for (i = 0; i < alias_list.nr; i++) {
+			aliases[i].name = alias_list.items[i].string;
+			aliases[i].help = alias_list.items[i].util;
+			aliases[i].category = 1;
+		}
+		aliases[alias_list.nr].name = NULL;
+		print_command_list(aliases, 1, longest);
+		free(aliases);
+	}
+	string_list_clear(&alias_list, 1);
 }
 
 int is_in_cmdlist(struct cmdnames *c, const char *s)
diff --git a/help.h b/help.h
index 9eab6a3f89..105de6195a 100644
--- a/help.h
+++ b/help.h
@@ -37,7 +37,6 @@ extern void add_cmdname(struct cmdnames *cmds, const char *name, int len);
 /* Here we require that excludes is a sorted list. */
 extern void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes);
 extern int is_in_cmdlist(struct cmdnames *cmds, const char *name);
-extern void list_commands(unsigned int colopts, struct cmdnames *main_cmds, struct cmdnames *other_cmds);
 
 /*
  * call this to die(), when it is suspected that the user mistyped a
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index bc27df7f38..964615de2f 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -30,8 +30,7 @@ test_expect_success "setup" '
 test_expect_success 'basic help commands' '
 	git help >/dev/null &&
 	git help -a >/dev/null &&
-	git help -g >/dev/null &&
-	git help -av >/dev/null
+	git help -g >/dev/null
 '
 
 test_expect_success "works for commands and guides by default" '
-- 8< --

--
Duy

Re: [PATCH] git help: promote 'git help -av'

[Jeff King]

On Tue, Sep 25, 2018 at 07:44:51PM +0200, Duy Nguyen wrote:

> > I think adding another section about external commands in "help -av"
> > would address the "clang-format" stuff. With that, it's probably good
> > enough to completely replace "help -a". It may also be good to list
> > aliases there too in a separate section so you have "all you can type"
> > in one (big) list.
> 
> Here's the patch that adds that external commands and aliases
> sections. I feel that external commands section is definitely good to
> have even if we don't replace "help -a". Aliases are more
> subjective...

Just eyeballing the output, it looks pretty reasonable to me. The lack
of explanation for external commands really stands out, but there's not
much we can do.

That part of the output is not very compact, and we _could_ put it in
columns, but that might be confusing since the rest of the output is
one-per-line.

Mentioning aliases seems reasonable to me. The definitions of some of
mine are pretty nasty bits of shell, but I guess that people either
don't have any ugly aliases, or are comfortable enough with them that
they won't be scared away. :)

> -- 8< --
> @@ -53,7 +52,6 @@ static struct option builtin_help_options[] = {
>  			HELP_FORMAT_WEB),
>  	OPT_SET_INT('i', "info", &help_format, N_("show info page"),
>  			HELP_FORMAT_INFO),
> -	OPT__VERBOSE(&verbose, N_("print command description")),
>  	OPT_END(),
>  };

Would we want to continue respecting "-v" as a noop? I admit I did not
even know it existed until this thread, but if people have trained
themselves to run "git help -av", we should probably continue to give
them this output.

-Peff

Re: [PATCH] git help: promote 'git help -av'

[Duy Nguyen]

On Tue, Sep 25, 2018 at 10:54 PM Jeff King <peff@peff.net> wrote:
> Mentioning aliases seems reasonable to me. The definitions of some of
> mine are pretty nasty bits of shell, but I guess that people either
> don't have any ugly aliases, or are comfortable enough with them that
> they won't be scared away. :)

I have one with a very long pretty format for git-log. Well, you wrote
your aliases you learn to live with them :) We could certainly cut too
long aliased commands to keep the listing pretty, but I don't think we
should do that until somebody asks for it.

> > -- 8< --
> > @@ -53,7 +52,6 @@ static struct option builtin_help_options[] = {
> >                       HELP_FORMAT_WEB),
> >       OPT_SET_INT('i', "info", &help_format, N_("show info page"),
> >                       HELP_FORMAT_INFO),
> > -     OPT__VERBOSE(&verbose, N_("print command description")),
> >       OPT_END(),
> >  };
>
> Would we want to continue respecting "-v" as a noop? I admit I did not
> even know it existed until this thread, but if people have trained
> themselves to run "git help -av", we should probably continue to give
> them this output.

-v was recently added just for the new "help -a" in May 2018. I think
it's ok to get rid of it. Memory muscles probably take a couple more
months to kick in.
-- 
Duy

Re: [PATCH] git help: promote 'git help -av'

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> -v was recently added just for the new "help -a" in May 2018. I think
> it's ok to get rid of it. Memory muscles probably take a couple more
> months to kick in.

If it is not hurting, keeping it lets people say "--no-verbose" to
get a less verbose output to help those who do not like the new
default.

Unless you are removing code to show the current "help -a" output,
that is.

Re: [PATCH] git help: promote 'git help -av'

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> Here's the patch that adds that external commands and aliases
> sections. I feel that external commands section is definitely good to
> have even if we don't replace "help -a". Aliases are more
> subjective...

I didn't apply this (so I didn't try running it), but a quick
eyeballing tells me that the listing of external commands in -av
output done in this patch seems reasonable.

I do not think removing "-v" and making the current "help -a" output
unavailable is a wise idea, though.

Re: [PATCH] git help: promote 'git help -av'

[Taylor Blau]

On Wed, Sep 26, 2018 at 10:28:31AM -0700, Junio C Hamano wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
> > Here's the patch that adds that external commands and aliases
> > sections. I feel that external commands section is definitely good to
> > have even if we don't replace "help -a". Aliases are more
> > subjective...
>
> I didn't apply this (so I didn't try running it), but a quick
> eyeballing tells me that the listing of external commands in -av
> output done in this patch seems reasonable.
>
> I do not think removing "-v" and making the current "help -a" output
> unavailable is a wise idea, though.

I think that your point about having to react in a split-second in order
to ^C before we open the manual page for a command is a good one. So, I
agree with this.

Thanks,
Taylor

Re: [PATCH] git help: promote 'git help -av'

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> On Wed, Sep 26, 2018 at 10:28:31AM -0700, Junio C Hamano wrote:
>> Duy Nguyen <pclouds@gmail.com> writes:
>>
>> > Here's the patch that adds that external commands and aliases
>> > sections. I feel that external commands section is definitely good to
>> > have even if we don't replace "help -a". Aliases are more
>> > subjective...
>>
>> I didn't apply this (so I didn't try running it), but a quick
>> eyeballing tells me that the listing of external commands in -av
>> output done in this patch seems reasonable.
>>
>> I do not think removing "-v" and making the current "help -a" output
>> unavailable is a wise idea, though.
>
> I think that your point about having to react in a split-second in order
> to ^C before we open the manual page for a command is a good one. So, I
> agree with this.

Responding to a wrong thread?  

I thought "now I need ^C within a handful of deciseconds if I want
only alias?" was not about the change to make "-v" default when
"help -a" is asked, but about what to do with "git foo --help" that
only gives "foo is aliased to ...".

Re: [PATCH] git help: promote 'git help -av'

[Taylor Blau]

On Fri, Sep 28, 2018 at 09:30:51AM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > On Wed, Sep 26, 2018 at 10:28:31AM -0700, Junio C Hamano wrote:
> >> Duy Nguyen <pclouds@gmail.com> writes:
> >>
> >> > Here's the patch that adds that external commands and aliases
> >> > sections. I feel that external commands section is definitely good to
> >> > have even if we don't replace "help -a". Aliases are more
> >> > subjective...
> >>
> >> I didn't apply this (so I didn't try running it), but a quick
> >> eyeballing tells me that the listing of external commands in -av
> >> output done in this patch seems reasonable.
> >>
> >> I do not think removing "-v" and making the current "help -a" output
> >> unavailable is a wise idea, though.
> >
> > I think that your point about having to react in a split-second in order
> > to ^C before we open the manual page for a command is a good one. So, I
> > agree with this.
>
> Responding to a wrong thread?

Ah, I certainly am. Thanks for catching my mistake :-).

> I thought "now I need ^C within a handful of deciseconds if I want
> only alias?" was not about the change to make "-v" default when
> "help -a" is asked, but about what to do with "git foo --help" that
> only gives "foo is aliased to ...".

Thanks,
Taylor

[PATCH v2] help -a: improve and make --verbose default

[Nguyễn Thái Ngọc Duy]

When you type "git help" (or just "git") you are greeted with a list
with commonly used commands and their short description and are
suggested to use "git help -a" or "git help -g" for more details.

"git help -av" would be more friendly and inline with what is shown
with "git help" since it shows list of commands with description as
well, and commands are properly grouped.

"help -av" does not show everything "help -a" shows though. Add
external command section in "help -av" for this. While at there, add a
section for aliases as well (until now aliases have no UI, just "git
config").

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 v2 makes 'help -av' default and the user would need to type 'help -a
 --no-verbose' to get the old printout back. 'help -av' also has
 external commands and aliases.

 Documentation/git-help.txt |  8 +++---
 builtin/help.c             |  2 +-
 help.c                     | 50 +++++++++++++++++++++++++++++++++++---
 t/t0012-help.sh            |  4 +--
 4 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index 83d25d825a..206e3aef64 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -8,7 +8,7 @@ git-help - Display help information about Git
 SYNOPSIS
 --------
 [verse]
-'git help' [-a|--all [--verbose]] [-g|--guide]
+'git help' [-a|--all [--[no-]verbose]] [-g|--guide]
 	   [-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]
 
 DESCRIPTION
@@ -42,8 +42,10 @@ OPTIONS
 --all::
 	Prints all the available commands on the standard output. This
 	option overrides any given command or guide name.
-	When used with `--verbose` print description for all recognized
-	commands.
+
+--verbose::
+	When used with `--all` print description for all recognized
+	commands. This is the default.
 
 -c::
 --config::
diff --git a/builtin/help.c b/builtin/help.c
index 8d4f6dd301..d83dac2839 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -38,7 +38,7 @@ static const char *html_path;
 static int show_all = 0;
 static int show_guides = 0;
 static int show_config;
-static int verbose;
+static int verbose = 1;
 static unsigned int colopts;
 static enum help_format help_format = HELP_FORMAT_NONE;
 static int exclude_guides;
diff --git a/help.c b/help.c
index 96f6d221ed..4745b32299 100644
--- a/help.c
+++ b/help.c
@@ -98,7 +98,8 @@ static int cmd_name_cmp(const void *elem1, const void *elem2)
 	return strcmp(e1->name, e2->name);
 }
 
-static void print_cmd_by_category(const struct category_description *catdesc)
+static void print_cmd_by_category(const struct category_description *catdesc,
+				  int *longest_p)
 {
 	struct cmdname_help *cmds;
 	int longest = 0;
@@ -124,6 +125,8 @@ static void print_cmd_by_category(const struct category_description *catdesc)
 		print_command_list(cmds, mask, longest);
 	}
 	free(cmds);
+	if (longest_p)
+		*longest_p = longest;
 }
 
 void add_cmdname(struct cmdnames *cmds, const char *name, int len)
@@ -307,7 +310,7 @@ void list_commands(unsigned int colopts,
 void list_common_cmds_help(void)
 {
 	puts(_("These are common Git commands used in various situations:"));
-	print_cmd_by_category(common_categories);
+	print_cmd_by_category(common_categories, NULL);
 }
 
 void list_all_main_cmds(struct string_list *list)
@@ -405,7 +408,7 @@ void list_common_guides_help(void)
 		{ CAT_guide, N_("The common Git guides are:") },
 		{ 0, NULL }
 	};
-	print_cmd_by_category(catdesc);
+	print_cmd_by_category(catdesc, NULL);
 	putchar('\n');
 }
 
@@ -494,9 +497,48 @@ void list_config_help(int for_human)
 	string_list_clear(&keys, 0);
 }
 
+static int get_alias(const char *var, const char *value, void *data)
+{
+	struct string_list *list = data;
+
+	if (skip_prefix(var, "alias.", &var))
+		string_list_append(list, var)->util = xstrdup(value);
+
+	return 0;
+}
+
 void list_all_cmds_help(void)
 {
-	print_cmd_by_category(main_categories);
+	struct string_list others = STRING_LIST_INIT_DUP;
+	struct string_list alias_list = STRING_LIST_INIT_DUP;
+	struct cmdname_help *aliases;
+	int i, longest;
+
+	printf_ln(_("See 'git help <command>' to read about a specific subcommand"));
+	print_cmd_by_category(main_categories, &longest);
+
+	list_all_other_cmds(&others);
+	if (others.nr)
+		printf("\n%s\n", _("External commands"));
+	for (i = 0; i < others.nr; i++)
+		printf("   %s\n", others.items[i].string);
+	string_list_clear(&others, 0);
+
+	git_config(get_alias, &alias_list);
+	string_list_sort(&alias_list);
+	if (alias_list.nr) {
+		printf("\n%s\n", _("Command aliases"));
+		ALLOC_ARRAY(aliases, alias_list.nr + 1);
+		for (i = 0; i < alias_list.nr; i++) {
+			aliases[i].name = alias_list.items[i].string;
+			aliases[i].help = alias_list.items[i].util;
+			aliases[i].category = 1;
+		}
+		aliases[alias_list.nr].name = NULL;
+		print_command_list(aliases, 1, longest);
+		free(aliases);
+	}
+	string_list_clear(&alias_list, 1);
 }
 
 int is_in_cmdlist(struct cmdnames *c, const char *s)
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index bc27df7f38..e8ef7300ec 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -29,9 +29,9 @@ test_expect_success "setup" '
 # to verify
 test_expect_success 'basic help commands' '
 	git help >/dev/null &&
-	git help -a >/dev/null &&
+	git help -a --no-verbose >/dev/null &&
 	git help -g >/dev/null &&
-	git help -av >/dev/null
+	git help -a >/dev/null
 '
 
 test_expect_success "works for commands and guides by default" '
-- 
2.19.0.341.g3acb95d729

Re: [PATCH v2] help -a: improve and make --verbose default

[Taylor Blau]

On Sat, Sep 29, 2018 at 08:08:14AM +0200, Nguyễn Thái Ngọc Duy wrote:
> When you type "git help" (or just "git") you are greeted with a list
> with commonly used commands and their short description and are
> suggested to use "git help -a" or "git help -g" for more details.
>
> "git help -av" would be more friendly and inline with what is shown
> with "git help" since it shows list of commands with description as
> well, and commands are properly grouped.
>
> "help -av" does not show everything "help -a" shows though. Add
> external command section in "help -av" for this. While at there, add a
> section for aliases as well (until now aliases have no UI, just "git
> config").
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  v2 makes 'help -av' default and the user would need to type 'help -a
>  --no-verbose' to get the old printout back. 'help -av' also has
>  external commands and aliases.

Thanks. This looks like what I would have expected based on my
recollection of the discussion earlier on v1, so this has my:

  Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks,
Taylor