Re: [PATCH] help: make option --help open man pages only for Git commands

[Junio C Hamano]

"Philip Oakley"  writes:

> But does it cope with the Guides? Should it cope if spelt that way?
>
> git help revisions
> git revisions --help

Hmph.  Ralf's patch is not just "I wonder if we could do a bit
better" but is also a regression.  I do not particularly care
if the latter stops working, but the former definitely should,
as "git help -g" encourages readers to type that, but with the
change "git help " seems to stop working.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] help: make option --help open man pages only for Git commands

[Philip Oakley]

From: "Junio C Hamano" <gits...@pobox.com>
To: "Ralf Thielow" <ralf.thie...@gmail.com>
Cc: <git@vger.kernel.org>; <larsxschnei...@gmail.com>; <m...@jnm2.com>
Sent: Friday, August 12, 2016 11:53 PM
Subject: Re: [PATCH] help: make option --help open man pages only for Git 
commands




Junio C Hamano <gits...@pobox.com> writes:


I love it when I say "This shouldn't be too hard; somebody may want
to do a patch", with just a vague implemention idea in my head, and
a patch magically appears with even a better design than I had in
mind when I said it [*1*] ;-)


Having said that, I wonder if we could do a bit better.

   $ git -c help.autocorrect=1 whatchange --help
   WARNING: You called a Git command named 'whatchange', which does not 
exist.

   Continuing under the assumption that you meant 'whatchanged'
   in 0.1 seconds automatically...
   No manual entry for gitwhatchange

We are guessing that the user meant "whatchanged"; shouldn't we be
able to feed the corrected name of the command to the machinery to
drive the manpage viewer?


But does it cope with the Guides? Should it cope if spelt that way?

git help revisions
git revisions --help

?

I suspect that the former should work, but not the latter, as a reasonable 
approach. I just haven't checked.


The other moderately low hanging fruit is to do the (full) list of guides as 
part of 'help'.


Philip 


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] help: make option --help open man pages only for Git commands

[Junio C Hamano]

Junio C Hamano  writes:

> I love it when I say "This shouldn't be too hard; somebody may want
> to do a patch", with just a vague implemention idea in my head, and
> a patch magically appears with even a better design than I had in
> mind when I said it [*1*] ;-)

Having said that, I wonder if we could do a bit better.

$ git -c help.autocorrect=1 whatchange --help
WARNING: You called a Git command named 'whatchange', which does not exist.
Continuing under the assumption that you meant 'whatchanged'
in 0.1 seconds automatically...
No manual entry for gitwhatchange

We are guessing that the user meant "whatchanged"; shouldn't we be
able to feed the corrected name of the command to the machinery to
drive the manpage viewer?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] help: make option --help open man pages only for Git commands

[Junio C Hamano]

Ralf Thielow  writes:

> If option --help is passed to a Git command, we try to open
> the man page of that command. However, we do it even for commands
> we don't know.  Make sure the command is known to Git before try
> to open the man page.  If we don't know the command, give the
> usual advice.
>
> Signed-off-by: Ralf Thielow 
> ---

I love it when I say "This shouldn't be too hard; somebody may want
to do a patch", with just a vague implemention idea in my head, and
a patch magically appears with even a better design than I had in
mind when I said it [*1*] ;-)

>  builtin/help.c  | 21 ++---
>  t/t0012-help.sh | 15 +++
>  2 files changed, 29 insertions(+), 7 deletions(-)
>  create mode 100755 t/t0012-help.sh
>
> diff --git a/builtin/help.c b/builtin/help.c
> index 8848013..55d45de 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -433,10 +433,22 @@ static void list_common_guides_help(void)
>   putchar('\n');
>  }
>  
> +static void check_git_cmd(const char* cmd) {
> + char *alias = alias_lookup(cmd);
> +
> + if (!is_git_command(cmd)) {
> + if (alias) {
> + printf_ln(_("`git %s' is aliased to `%s'"), cmd, alias);
> + free(alias);
> + exit(0);
> + } else
> + help_unknown_cmd(cmd);
> + }
> +}

Looks quite reasonable to reuse help_unknown_cmd() there.

Thanks, will queue.


[Footnote]

*1* The vague thing I had in my mind was to use is_git_command() and
alias_lookup() to prevent the "git foo --help" -> "git help foo" 
magic from triggering for 'foo' that is not known.  Your solution
is MUCH cleaner and more straight-forward.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] help: make option --help open man pages only for Git commands

[Ralf Thielow]

If option --help is passed to a Git command, we try to open
the man page of that command. However, we do it even for commands
we don't know.  Make sure the command is known to Git before try
to open the man page.  If we don't know the command, give the
usual advice.

Signed-off-by: Ralf Thielow 
---
 builtin/help.c  | 21 ++---
 t/t0012-help.sh | 15 +++
 2 files changed, 29 insertions(+), 7 deletions(-)
 create mode 100755 t/t0012-help.sh

diff --git a/builtin/help.c b/builtin/help.c
index 8848013..55d45de 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -433,10 +433,22 @@ static void list_common_guides_help(void)
putchar('\n');
 }
 
+static void check_git_cmd(const char* cmd) {
+   char *alias = alias_lookup(cmd);
+
+   if (!is_git_command(cmd)) {
+   if (alias) {
+   printf_ln(_("`git %s' is aliased to `%s'"), cmd, alias);
+   free(alias);
+   exit(0);
+   } else
+   help_unknown_cmd(cmd);
+   }
+}
+
 int cmd_help(int argc, const char **argv, const char *prefix)
 {
int nongit;
-   char *alias;
enum help_format parsed_help_format;
 
argc = parse_options(argc, argv, prefix, builtin_help_options,
@@ -476,12 +488,7 @@ int cmd_help(int argc, const char **argv, const char 
*prefix)
if (help_format == HELP_FORMAT_NONE)
help_format = parse_help_format(DEFAULT_HELP_FORMAT);
 
-   alias = alias_lookup(argv[0]);
-   if (alias && !is_git_command(argv[0])) {
-   printf_ln(_("`git %s' is aliased to `%s'"), argv[0], alias);
-   free(alias);
-   return 0;
-   }
+   check_git_cmd(argv[0]);
 
switch (help_format) {
case HELP_FORMAT_NONE:
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
new file mode 100755
index 000..0dab88d
--- /dev/null
+++ b/t/t0012-help.sh
@@ -0,0 +1,15 @@
+#!/bin/sh
+
+test_description='help'
+
+. ./test-lib.sh
+
+test_expect_success "pass --help to unknown command" "
+   cat <<-EOF >expected &&
+   git: '123' is not a git command. See 'git --help'.
+   EOF
+   (git 123 --help 2>actual || true) &&
+   test_i18ncmp expected actual
+"
+
+test_done
-- 
2.9.2.911.g31804cd.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html