{"thread":{"id":"43792","subject":"`git stash --help` tries to pull up nonexistent file gitstack.html","startedAt":"2016-08-12T02:01:34Z","lastAt":"2016-08-26T20:39:57Z","messageCount":46,"participants":["Joseph Musser","Junio C Hamano","Lars Schneider","Jacob Keller","Ralf Thielow","Philip Oakley","John Keeping","Remi Galan Alfonso","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"298814","messageId":"CAKRjdd4WdVTgbT0gcR=a267+aEwD2Exztrc9gNau1nOXroC=ng@mail.gmail.com","threadId":"43792","inReplyTo":null,"subject":"`git stash --help` tries to pull up nonexistent file gitstack.html","fromName":"Joseph Musser","fromEmail":"me@jnm2.com","sentAt":"2016-08-12T02:00:56Z","receivedAt":"2016-08-12T02:01:34Z","isPatch":false,"sender":{"key":"me@jnm2.com","avatar":"https://gravatar.com/avatar/82a7d3ba27eefb8c496774b34d193877cb5a3eec5374a07a27506d32c9ba9d89?d=mp&s=160"},"body":"Looks like a simple typo.\n\n\nJoseph Musser\n"},{"id":"298845","messageId":"xmqqr39uxa33.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"CAKRjdd4WdVTgbT0gcR=a267+aEwD2Exztrc9gNau1nOXroC=ng@mail.gmail.com","subject":"Re: `git stash --help` tries to pull up nonexistent file gitstack.html","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-12T15:48:00Z","receivedAt":"2016-08-12T15:48:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joseph Musser writes:\n\n> Looks like a simple typo.\n\nUnfortunately this does not reproduce to me (built from source on\nUbuntu Linux).\n"},{"id":"298858","messageId":"A7A176B0-08CE-4D92-9756-51A59DF3B9D7@gmail.com","threadId":"43792","inReplyTo":"xmqqr39uxa33.fsf@gitster.mtv.corp.google.com","subject":"Re: `git stash --help` tries to pull up nonexistent file gitstack.html","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-08-12T16:03:58Z","receivedAt":"2016-08-12T16:04:32Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 12 Aug 2016, at 17:48, Junio C Hamano wrote:\n> \n> Joseph Musser writes:\n> \n>> Looks like a simple typo.\n> \n> Unfortunately this does not reproduce to me (built from source on\n> Ubuntu Linux).\n\nI tried it with the latest released version on Windows and OSX (2.9.2)\nand was not able to reproduce it, too.\n\n- Lars\n"},{"id":"298860","messageId":"CAKRjdd4V3OfDnzisxBofBUmtds7q7ejUtuV_-s96eUVf7fqwHA@mail.gmail.com","threadId":"43792","inReplyTo":"A7A176B0-08CE-4D92-9756-51A59DF3B9D7@gmail.com","subject":"Re: `git stash --help` tries to pull up nonexistent file gitstack.html","fromName":"Joseph Musser","fromEmail":"me@jnm2.com","sentAt":"2016-08-12T16:15:38Z","receivedAt":"2016-08-12T16:16:14Z","isPatch":false,"sender":{"key":"me@jnm2.com","avatar":"https://gravatar.com/avatar/82a7d3ba27eefb8c496774b34d193877cb5a3eec5374a07a27506d32c9ba9d89?d=mp&s=160"},"body":"Oh, I'm embarrassed. The typo was mine, I must have typed `git stack\n--help`. I would have expected a syntax error message or \"did you\nmean\" suggestions; it didn't even enter my mind that it would look up\nwhatever I typed before --help and assume it existed on disk.\n\nI'm sorry!\n\nOn Fri, Aug 12, 2016 at 12:03 PM, Lars Schneider\n wrote:\n>\n>> On 12 Aug 2016, at 17:48, Junio C Hamano wrote:\n>>\n>> Joseph Musser writes:\n>>\n>>> Looks like a simple typo.\n>>\n>> Unfortunately this does not reproduce to me (built from source on\n>> Ubuntu Linux).\n>\n> I tried it with the latest released version on Windows and OSX (2.9.2)\n> and was not able to reproduce it, too.\n>\n> - Lars\n"},{"id":"298861","messageId":"CAPc5daXicjUDi6B-MA8Sn=_UZ_jHvc8SE4ZXt2dHbbDQkD7=WA@mail.gmail.com","threadId":"43792","inReplyTo":"CAKRjdd4V3OfDnzisxBofBUmtds7q7ejUtuV_-s96eUVf7fqwHA@mail.gmail.com","subject":"Re: `git stash --help` tries to pull up nonexistent file gitstack.html","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-12T16:25:28Z","receivedAt":"2016-08-12T16:25:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Fri, Aug 12, 2016 at 9:15 AM, Joseph Musser wrote:\n> Oh, I'm embarrassed. The typo was mine, I must have typed `git stack\n> --help`. I would have expected a syntax error message or \"did you\n> mean\" suggestions; it didn't even enter my mind that it would look up\n> whatever I typed before --help and assume it existed on disk.\n\nI actually think you found an interesting (albeit minor) bug.\nI think whenever \"git\" sees any word followed by \"--help\" and nothing else,\nit blindly turns it into \"git help\" followed by that word. I think it\nis reasonable\nto expect that \"git foo --help\" responds with \"foo: no such subcommand\",\ninstead of \"No manual entry for gitfoo\".\n\nIt may not be too hard to arrange; this might be another low-hanging\nfruit if somebody wants to try a patch ;-)\n\nThanks.\n"},{"id":"298880","messageId":"CA+P7+xp7rpVRWgAXTe9sHN9=a+T+x0SnQPUCrR5_E9Qpufo6=Q@mail.gmail.com","threadId":"43792","inReplyTo":"CAPc5daXicjUDi6B-MA8Sn=_UZ_jHvc8SE4ZXt2dHbbDQkD7=WA@mail.gmail.com","subject":"Re: `git stash --help` tries to pull up nonexistent file gitstack.html","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-08-12T18:14:46Z","receivedAt":"2016-08-12T18:16:33Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Aug 12, 2016 at 9:25 AM, Junio C Hamano wrote:\n> On Fri, Aug 12, 2016 at 9:15 AM, Joseph Musser wrote:\n>> Oh, I'm embarrassed. The typo was mine, I must have typed `git stack\n>> --help`. I would have expected a syntax error message or \"did you\n>> mean\" suggestions; it didn't even enter my mind that it would look up\n>> whatever I typed before --help and assume it existed on disk.\n>\n> I actually think you found an interesting (albeit minor) bug.\n> I think whenever \"git\" sees any word followed by \"--help\" and nothing else,\n> it blindly turns it into \"git help\" followed by that word. I think it\n> is reasonable\n> to expect that \"git foo --help\" responds with \"foo: no such subcommand\",\n> instead of \"No manual entry for gitfoo\".\n>\n> It may not be too hard to arrange; this might be another low-hanging\n> fruit if somebody wants to try a patch ;-)\n>\n\nWhat about extension subcommands that aren't core? Wouldn't we prefer\nif it still tried to find help for those also? Just a thought to add\nto this.\n\nThanks,\nJake\n"},{"id":"298889","messageId":"20160812201011.20233-1-ralf.thielow@gmail.com","threadId":"43792","inReplyTo":"CAPc5daXicjUDi6B-MA8Sn=_UZ_jHvc8SE4ZXt2dHbbDQkD7=WA@mail.gmail.com","subject":"[PATCH] help: make option --help open man pages only for Git commands","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-12T20:10:11Z","receivedAt":"2016-08-12T20:10:20Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"If option --help is passed to a Git command, we try to open\nthe man page of that command. However, we do it even for commands\nwe don't know. Make sure the command is known to Git before try\nto open the man page. If we don't know the command, give the\nusual advice.\n\nSigned-off-by: Ralf Thielow \n---\n builtin/help.c | 21 ++++++++++++++-------\n t/t0012-help.sh | 15 +++++++++++++++\n 2 files changed, 29 insertions(+), 7 deletions(-)\n create mode 100755 t/t0012-help.sh\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex 8848013..55d45de 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -433,10 +433,22 @@ static void list_common_guides_help(void)\n \tputchar('\\n');\n }\n \n+static void check_git_cmd(const char* cmd) {\n+\tchar *alias = alias_lookup(cmd);\n+\n+\tif (!is_git_command(cmd)) {\n+\t\tif (alias) {\n+\t\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), cmd, alias);\n+\t\t\tfree(alias);\n+\t\t\texit(0);\n+\t\t} else\n+\t\t\thelp_unknown_cmd(cmd);\n+\t}\n+}\n+\n int cmd_help(int argc, const char **argv, const char *prefix)\n {\n \tint nongit;\n-\tchar *alias;\n \tenum help_format parsed_help_format;\n \n \targc = parse_options(argc, argv, prefix, builtin_help_options,\n@@ -476,12 +488,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)\n \tif (help_format == HELP_FORMAT_NONE)\n \t\thelp_format = parse_help_format(DEFAULT_HELP_FORMAT);\n \n-\talias = alias_lookup(argv[0]);\n-\tif (alias && !is_git_command(argv[0])) {\n-\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), argv[0], alias);\n-\t\tfree(alias);\n-\t\treturn 0;\n-\t}\n+\tcheck_git_cmd(argv[0]);\n \n \tswitch (help_format) {\n \tcase HELP_FORMAT_NONE:\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nnew file mode 100755\nindex 0000000..0dab88d\n--- /dev/null\n+++ b/t/t0012-help.sh\n@@ -0,0 +1,15 @@\n+#!/bin/sh\n+\n+test_description='help'\n+\n+. ./test-lib.sh\n+\n+test_expect_success \"pass --help to unknown command\" \"\n+\tcat <<-EOF >expected &&\n+\t\tgit: '123' is not a git command. See 'git --help'.\n+\tEOF\n+\t(git 123 --help 2>actual || true) &&\n+\ttest_i18ncmp expected actual\n+\"\n+\n+test_done\n-- \n2.9.2.911.g31804cd.dirty\n\n"},{"id":"298893","messageId":"xmqqk2flvfhb.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"20160812201011.20233-1-ralf.thielow@gmail.com","subject":"Re: [PATCH] help: make option --help open man pages only for Git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-12T21:34:24Z","receivedAt":"2016-08-12T21:34:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow writes:\n\n> If option --help is passed to a Git command, we try to open\n> the man page of that command. However, we do it even for commands\n> we don't know. Make sure the command is known to Git before try\n> to open the man page. If we don't know the command, give the\n> usual advice.\n>\n> Signed-off-by: Ralf Thielow \n> ---\n\nI love it when I say \"This shouldn't be too hard; somebody may want\nto do a patch\", with just a vague implemention idea in my head, and\na patch magically appears with even a better design than I had in\nmind when I said it [*1*] ;-)\n\n> builtin/help.c | 21 ++++++++++++++-------\n> t/t0012-help.sh | 15 +++++++++++++++\n> 2 files changed, 29 insertions(+), 7 deletions(-)\n> create mode 100755 t/t0012-help.sh\n>\n> diff --git a/builtin/help.c b/builtin/help.c\n> index 8848013..55d45de 100644\n> --- a/builtin/help.c\n> +++ b/builtin/help.c\n> @@ -433,10 +433,22 @@ static void list_common_guides_help(void)\n> \tputchar('\\n');\n> }\n> \n> +static void check_git_cmd(const char* cmd) {\n> +\tchar *alias = alias_lookup(cmd);\n> +\n> +\tif (!is_git_command(cmd)) {\n> +\t\tif (alias) {\n> +\t\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), cmd, alias);\n> +\t\t\tfree(alias);\n> +\t\t\texit(0);\n> +\t\t} else\n> +\t\t\thelp_unknown_cmd(cmd);\n> +\t}\n> +}\n\nLooks quite reasonable to reuse help_unknown_cmd() there.\n\nThanks, will queue.\n\n\n[Footnote]\n\n*1* The vague thing I had in my mind was to use is_git_command() and\n alias_lookup() to prevent the \"git foo --help\" -> \"git help foo\" \n magic from triggering for 'foo' that is not known. Your solution\n is MUCH cleaner and more straight-forward.\n\n"},{"id":"298935","messageId":"xmqq1t1tvbu8.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"xmqqk2flvfhb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] help: make option --help open man pages only for Git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-12T22:53:03Z","receivedAt":"2016-08-12T22:53:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I love it when I say \"This shouldn't be too hard; somebody may want\n> to do a patch\", with just a vague implemention idea in my head, and\n> a patch magically appears with even a better design than I had in\n> mind when I said it [*1*] ;-)\n\nHaving said that, I wonder if we could do a bit better.\n\n $ git -c help.autocorrect=1 whatchange --help\n WARNING: You called a Git command named 'whatchange', which does not exist.\n Continuing under the assumption that you meant 'whatchanged'\n in 0.1 seconds automatically...\n No manual entry for gitwhatchange\n\nWe are guessing that the user meant \"whatchanged\"; shouldn't we be\nable to feed the corrected name of the command to the machinery to\ndrive the manpage viewer?\n\n"},{"id":"298942","messageId":"F3BA479B58944083A62896E2F0504D48@PhilipOakley","threadId":"43792","inReplyTo":"xmqq1t1tvbu8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] help: make option --help open man pages only for Git commands","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-08-13T00:08:22Z","receivedAt":"2016-08-13T00:08:30Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \nTo: \"Ralf Thielow\" \nCc: ; ; \nSent: Friday, August 12, 2016 11:53 PM\nSubject: Re: [PATCH] help: make option --help open man pages only for Git \ncommands\n\n\n> Junio C Hamano writes:\n>\n>> I love it when I say \"This shouldn't be too hard; somebody may want\n>> to do a patch\", with just a vague implemention idea in my head, and\n>> a patch magically appears with even a better design than I had in\n>> mind when I said it [*1*] ;-)\n>\n> Having said that, I wonder if we could do a bit better.\n>\n> $ git -c help.autocorrect=1 whatchange --help\n> WARNING: You called a Git command named 'whatchange', which does not \n> exist.\n> Continuing under the assumption that you meant 'whatchanged'\n> in 0.1 seconds automatically...\n> No manual entry for gitwhatchange\n>\n> We are guessing that the user meant \"whatchanged\"; shouldn't we be\n> able to feed the corrected name of the command to the machinery to\n> drive the manpage viewer?\n>\nBut does it cope with the Guides? Should it cope if spelt that way?\n\ngit help revisions\ngit revisions --help\n\n?\n\nI suspect that the former should work, but not the latter, as a reasonable \napproach. I just haven't checked.\n\nThe other moderately low hanging fruit is to do the (full) list of guides as \npart of 'help'.\n\nPhilip \n\n"},{"id":"298972","messageId":"xmqqwpjku1mz.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"F3BA479B58944083A62896E2F0504D48@PhilipOakley","subject":"Re: [PATCH] help: make option --help open man pages only for Git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-13T15:31:00Z","receivedAt":"2016-08-13T15:31:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> But does it cope with the Guides? Should it cope if spelt that way?\n>\n> git help revisions\n> git revisions --help\n\nHmph. Ralf's patch is not just \"I wonder if we could do a bit\nbetter\" but is also a regression. I do not particularly care\nif the latter stops working, but the former definitely should,\nas \"git help -g\" encourages readers to type that, but with the\nchange \"git help \" seems to stop working.\n\n"},{"id":"299313","messageId":"20160815053628.3793-1-ralf.thielow@gmail.com","threadId":"43792","inReplyTo":"20160812201011.20233-1-ralf.thielow@gmail.com","subject":"[PATCH v2] help: make option --help open man pages only for Git commands","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-15T05:36:28Z","receivedAt":"2016-08-15T05:36:42Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"If option --help is passed to a Git command, we try to open\nthe man page of that command. However, we do it even for commands\nwe don't know. Make sure the command is known to Git before try\nto open the man page. If we don't know the command, give the\nusual advice.\n\nSigned-off-by: Ralf Thielow \n---\nChanges in v2:\n- not only check for commands but also for guides\n- use the command assumed by \"help_unknown_cmd\"\n\n builtin/help.c | 34 +++++++++++++++++++++++++++-------\n t/t0012-help.sh | 15 +++++++++++++++\n 2 files changed, 42 insertions(+), 7 deletions(-)\n create mode 100755 t/t0012-help.sh\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex 8848013..7d2110e 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -433,10 +433,35 @@ static void list_common_guides_help(void)\n \tputchar('\\n');\n }\n \n+static int is_common_guide(const char* cmd)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < ARRAY_SIZE(common_guides); i++)\n+\t\tif (!strcmp(cmd, common_guides[i].name))\n+\t\t\treturn 1;\n+\treturn 0;\n+}\n+\n+static const char* check_git_cmd(const char* cmd)\n+{\n+\tchar *alias;\n+\n+\tif (is_git_command(cmd) || is_common_guide(cmd))\n+\t\treturn cmd;\n+\n+\talias = alias_lookup(cmd);\n+\tif (alias) {\n+\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), cmd, alias);\n+\t\tfree(alias);\n+\t\texit(0);\n+\t} else\n+\t\treturn help_unknown_cmd(cmd);\n+}\n+\n int cmd_help(int argc, const char **argv, const char *prefix)\n {\n \tint nongit;\n-\tchar *alias;\n \tenum help_format parsed_help_format;\n \n \targc = parse_options(argc, argv, prefix, builtin_help_options,\n@@ -476,12 +501,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)\n \tif (help_format == HELP_FORMAT_NONE)\n \t\thelp_format = parse_help_format(DEFAULT_HELP_FORMAT);\n \n-\talias = alias_lookup(argv[0]);\n-\tif (alias && !is_git_command(argv[0])) {\n-\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), argv[0], alias);\n-\t\tfree(alias);\n-\t\treturn 0;\n-\t}\n+\targv[0] = check_git_cmd(argv[0]);\n \n \tswitch (help_format) {\n \tcase HELP_FORMAT_NONE:\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nnew file mode 100755\nindex 0000000..0dab88d\n--- /dev/null\n+++ b/t/t0012-help.sh\n@@ -0,0 +1,15 @@\n+#!/bin/sh\n+\n+test_description='help'\n+\n+. ./test-lib.sh\n+\n+test_expect_success \"pass --help to unknown command\" \"\n+\tcat <<-EOF >expected &&\n+\t\tgit: '123' is not a git command. See 'git --help'.\n+\tEOF\n+\t(git 123 --help 2>actual || true) &&\n+\ttest_i18ncmp expected actual\n+\"\n+\n+test_done\n-- \n2.9.2.912.g51c4565.dirty\n\n"},{"id":"299325","messageId":"D954CB3E6C3445AF9358C6941362B69D@PhilipOakley","threadId":"43792","inReplyTo":"20160815053628.3793-1-ralf.thielow@gmail.com","subject":"Re: [PATCH v2] help: make option --help open man pages only for Git commands","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-08-15T11:25:40Z","receivedAt":"2016-08-15T11:25:49Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Ralf Thielow\" \n> If option --help is passed to a Git command, we try to open\n> the man page of that command. However, we do it even for commands\n> we don't know. Make sure the command is known to Git before try\n> to open the man page. If we don't know the command, give the\n> usual advice.\n\nI'm still not sure this is enough. One of the problems back when I \nintroduced the --guides option (65f9835 (builtin/help.c: add --guide option, \n2013-04-02)) was that we had no easy way of determining what guides were \navailable, especially given the *nix/Windows split where the help defaults \nare different (--man/--html).\n\nAt the time[1] we (I) punted on trying to determine which guides were \nactually installed, and just created a short list of the important guides, \nwhich I believe you now check. However the less common guides are still \nthere (gitcvs-migration?), and others may be added locally.\n\nOne option may be to report that \"no command or common guide found, will \nsearch for other guide (may fail)\", which at least allows you to check the \ncommand list first, and then the common guide list, and only then warn \n(option?), and finally go on the rabbit hunt (possibly fruitless) for the \nmissing guide (we've already decided it can't be a command!)\n\n--\nPhilip\n\n[1] \nhttps://public-inbox.org/git/1364942392-576-1-git-send-email-philipoakley@iee.org/ \n(V3) plus previous discussions\nhttps://public-inbox.org/git/1362342072-1412-1-git-send-email-philipoakley@iee.org/ \n(V2) see note\nPatch 6 - 13:\nAll dropped.\nDrop the separate guide list.txt and extraction script, which was\ncopied from the common command list and script. If the guide usage\nlist is useful, extend the command-list.txt and generate-cmdlist.sh\nat a later \ndatehttps://public-inbox.org/git/1361660761-1932-1-git-send-email-philipoakley@iee.org/#t \n(V1) the original series\n\n>\n> Signed-off-by: Ralf Thielow \n> ---\n> Changes in v2:\n> - not only check for commands but also for guides\n> - use the command assumed by \"help_unknown_cmd\"\n>\n> builtin/help.c | 34 +++++++++++++++++++++++++++-------\n> t/t0012-help.sh | 15 +++++++++++++++\n> 2 files changed, 42 insertions(+), 7 deletions(-)\n> create mode 100755 t/t0012-help.sh\n>\n> diff --git a/builtin/help.c b/builtin/help.c\n> index 8848013..7d2110e 100644\n> --- a/builtin/help.c\n> +++ b/builtin/help.c\n> @@ -433,10 +433,35 @@ static void list_common_guides_help(void)\n> putchar('\\n');\n> }\n>\n> +static int is_common_guide(const char* cmd)\n> +{\n> + int i;\n> +\n> + for (i = 0; i < ARRAY_SIZE(common_guides); i++)\n> + if (!strcmp(cmd, common_guides[i].name))\n> + return 1;\n> + return 0;\n> +}\n> +\n> +static const char* check_git_cmd(const char* cmd)\n> +{\n> + char *alias;\n> +\n> + if (is_git_command(cmd) || is_common_guide(cmd))\n> + return cmd;\n> +\n> + alias = alias_lookup(cmd);\n> + if (alias) {\n> + printf_ln(_(\"`git %s' is aliased to `%s'\"), cmd, alias);\n> + free(alias);\n> + exit(0);\n> + } else\n> + return help_unknown_cmd(cmd);\n> +}\n> +\n> int cmd_help(int argc, const char **argv, const char *prefix)\n> {\n> int nongit;\n> - char *alias;\n> enum help_format parsed_help_format;\n>\n> argc = parse_options(argc, argv, prefix, builtin_help_options,\n> @@ -476,12 +501,7 @@ int cmd_help(int argc, const char **argv, const char \n> *prefix)\n> if (help_format == HELP_FORMAT_NONE)\n> help_format = parse_help_format(DEFAULT_HELP_FORMAT);\n>\n> - alias = alias_lookup(argv[0]);\n> - if (alias && !is_git_command(argv[0])) {\n> - printf_ln(_(\"`git %s' is aliased to `%s'\"), argv[0], alias);\n> - free(alias);\n> - return 0;\n> - }\n> + argv[0] = check_git_cmd(argv[0]);\n>\n> switch (help_format) {\n> case HELP_FORMAT_NONE:\n> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n> new file mode 100755\n> index 0000000..0dab88d\n> --- /dev/null\n> +++ b/t/t0012-help.sh\n> @@ -0,0 +1,15 @@\n> +#!/bin/sh\n> +\n> +test_description='help'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success \"pass --help to unknown command\" \"\n> + cat <<-EOF >expected &&\n> + git: '123' is not a git command. See 'git --help'.\n> + EOF\n> + (git 123 --help 2>actual || true) &&\n> + test_i18ncmp expected actual\n> +\"\n> +\n> +test_done\n> -- \n> 2.9.2.912.g51c4565.dirty\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n\n"},{"id":"299364","messageId":"xmqqr39phq3c.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"D954CB3E6C3445AF9358C6941362B69D@PhilipOakley","subject":"Re: [PATCH v2] help: make option --help open man pages only for Git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-15T17:57:59Z","receivedAt":"2016-08-15T17:58:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> I'm still not sure this is enough. One of the problems back when I\n> introduced the --guides option (65f9835 (builtin/help.c: add --guide\n> option, 2013-04-02)) was that we had no easy way of determining what\n> guides were available, especially given the *nix/Windows split where\n> the help defaults are different (--man/--html).\n>\n> At the time[1] we (I) punted on trying to determine which guides were\n> actually installed, and just created a short list of the important\n> guides, which I believe you now check. However the less common guides\n> are still there (gitcvs-migration?), and others may be added locally.\n\nI think we should do both; \"git help cvs-migration\" should keep the\nsame codeflow and behaviour as we have today (so that it would still\nwork), while \"git cvs-migration --help\" should say \"'cvs-migration'\nis not a git command\". That would be a good clean-up anyway.\n\nIt obviously cannot be done if git.c::handle_builtin() does the same\n\"swap --help to help \" hack, but we could improve that\npart (e.g. rewrite it to \"help --swapped \" to allow cmd_help()\nto notice). When the user said \" --help\", we don't do guides,\nwhen we swapped the word order, we check with guides, too.\n\n\n"},{"id":"299395","messageId":"C8DDA334A45E4B558FD1EFB191E047C9@PhilipOakley","threadId":"43792","inReplyTo":"xmqqr39phq3c.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] help: make option --help open man pages only for Git commands","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-08-15T20:40:54Z","receivedAt":"2016-08-15T20:41:01Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \n> \"Philip Oakley\" writes:\n>\n>> I'm still not sure this is enough. One of the problems back when I\n>> introduced the --guides option (65f9835 (builtin/help.c: add --guide\n>> option, 2013-04-02)) was that we had no easy way of determining what\n>> guides were available, especially given the *nix/Windows split where\n>> the help defaults are different (--man/--html).\n>>\n>> At the time[1] we (I) punted on trying to determine which guides were\n>> actually installed, and just created a short list of the important\n>> guides, which I believe you now check. However the less common guides\n>> are still there (gitcvs-migration?), and others may be added locally.\n>\n> I think we should do both; \"git help cvs-migration\" should keep the\n> same codeflow and behaviour as we have today (so that it would still\n> work), while \"git cvs-migration --help\" should say \"'cvs-migration'\n> is not a git command\". That would be a good clean-up anyway.\n>\n> It obviously cannot be done if git.c::handle_builtin() does the same\n> \"swap --help to help \" hack, but we could improve that\n> part (e.g. rewrite it to \"help --swapped \" to allow cmd_help()\n> to notice). When the user said \" --help\", we don't do guides,\n> when we swapped the word order, we check with guides, too.\n>\nThe other option is to simply build a guide-list in exactly the same format \nas the command list (which if it works can be merged later). Re-use the \nexisting code, etc.\n\nI did propose that in my very first patch series, but it was probably a step \ntoo far at the time, as it stepped on the toes of your (junio's) script for \nthe command list.\n\nThe link in my previous patch got mangled a possible start point for Ralf \nfor looking at building a guide list would be \nhttp://public-inbox.org/git/1361660761-1932-7-git-send-email-philipoakley@iee.org/ \n(which worked back then ;-)\n\nPhilip\n\n\n"},{"id":"299415","messageId":"xmqq8tvxfzeh.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"C8DDA334A45E4B558FD1EFB191E047C9@PhilipOakley","subject":"Re: [PATCH v2] help: make option --help open man pages only for Git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-15T22:19:50Z","receivedAt":"2016-08-15T22:19:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> The other option is to simply build a guide-list in exactly the same\n> format as the command list (which if it works can be merged\n> later). Re-use the existing code, etc.\n\nYeah, that sounds like a good way to go forward. To implement typo\ncorrection for \"git help \", having guide-list would be\nvery useful.\n\nA related tangent is that I think \"git --help\" shouldn't\nfall back to \"git help \", regardless of typo correction. It\nhappens to \"work\" only because we blindly turned \" --help\" to\n\"help \" without even checking what is. Making it stop\n\"working\" would be a bugfix.\n\nAnd having both command and guide list would be helpful to prevent\n\"git --help\" from falling back to \"git help \".\n"},{"id":"299445","messageId":"20160816100633.be55qsbnlmlm37dr@john.keeping.me.uk","threadId":"43792","inReplyTo":"C8DDA334A45E4B558FD1EFB191E047C9@PhilipOakley","subject":"Re: [PATCH v2] help: make option --help open man pages only for Git commands","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2016-08-16T10:06:33Z","receivedAt":"2016-08-16T10:06:52Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Mon, Aug 15, 2016 at 09:40:54PM +0100, Philip Oakley wrote:\n> From: \"Junio C Hamano\" \n> > \"Philip Oakley\" writes:\n> >\n> >> I'm still not sure this is enough. One of the problems back when I\n> >> introduced the --guides option (65f9835 (builtin/help.c: add --guide\n> >> option, 2013-04-02)) was that we had no easy way of determining what\n> >> guides were available, especially given the *nix/Windows split where\n> >> the help defaults are different (--man/--html).\n> >>\n> >> At the time[1] we (I) punted on trying to determine which guides were\n> >> actually installed, and just created a short list of the important\n> >> guides, which I believe you now check. However the less common guides\n> >> are still there (gitcvs-migration?), and others may be added locally.\n> >\n> > I think we should do both; \"git help cvs-migration\" should keep the\n> > same codeflow and behaviour as we have today (so that it would still\n> > work), while \"git cvs-migration --help\" should say \"'cvs-migration'\n> > is not a git command\". That would be a good clean-up anyway.\n> >\n> > It obviously cannot be done if git.c::handle_builtin() does the same\n> > \"swap --help to help \" hack, but we could improve that\n> > part (e.g. rewrite it to \"help --swapped \" to allow cmd_help()\n> > to notice). When the user said \" --help\", we don't do guides,\n> > when we swapped the word order, we check with guides, too.\n> >\n> The other option is to simply build a guide-list in exactly the same format \n> as the command list (which if it works can be merged later). Re-use the \n> existing code, etc.\n\nOne nice thing at the moment is that third-party Git commands can\ninstall documentation and have \"git help\" work correctly (shameless plug\nfor git-integration[1] which does this). I think Junio's suggestion\nabove keeps that working whereas having a hardcoded list of guides will\nbreak this.\n\n[1] https://github.com/johnkeeping/git-integration\n"},{"id":"299473","messageId":"20160816162030.27754-1-ralf.thielow@gmail.com","threadId":"43792","inReplyTo":"20160815053628.3793-1-ralf.thielow@gmail.com","subject":"[PATCH v3] help: make option --help open man pages only for Git commands","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-16T16:20:30Z","receivedAt":"2016-08-16T16:22:04Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"If option --help is passed to a Git command, we try to open\nthe man page of that command. However, we do it even for commands\nwe don't know. Make sure it is a Git command by using \"help_unknown_cmd\"\nwhich is even able to assume a command if the user made a typo.\n\nThis breaks \"git --help\" while \"git help \" still works.\n\nAs \" --help\" will internally be turned into \"help \",\nintroduce the hidden option \"--swapped\" in order to know which\nversion has been called.\n\nSigned-off-by: Ralf Thielow \n---\nThanks, all, for the help!\n\nChanges since v2:\n- don't check for common guides as the list is very incomplete\n- only check for git commands when called via --help (introduce\n option --swapped for that), as suggested by Junio\n- change test case to check for --help being passed to a concept\n used as a git command\n\n builtin/help.c | 30 +++++++++++++++++++++++-------\n git.c | 15 ++++++++++++++-\n t/t0012-help.sh | 15 +++++++++++++++\n 3 files changed, 52 insertions(+), 8 deletions(-)\n create mode 100755 t/t0012-help.sh\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex 8848013..76f07c7 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -37,7 +37,9 @@ static int show_all = 0;\n static int show_guides = 0;\n static unsigned int colopts;\n static enum help_format help_format = HELP_FORMAT_NONE;\n+static int swapped = 0;\n static struct option builtin_help_options[] = {\n+\tOPT_BOOL('s', \"swapped\", &swapped, \"mark as being called by --help\"),\n \tOPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n \tOPT_BOOL('g', \"guides\", &show_guides, N_(\"print list of useful guides\")),\n \tOPT_SET_INT('m', \"man\", &help_format, N_(\"show man page\"), HELP_FORMAT_MAN),\n@@ -433,10 +435,29 @@ static void list_common_guides_help(void)\n \tputchar('\\n');\n }\n \n+static const char* check_git_cmd(const char* cmd)\n+{\n+\tchar *alias;\n+\n+\tif (is_git_command(cmd))\n+\t\treturn cmd;\n+\n+\talias = alias_lookup(cmd);\n+\tif (alias) {\n+\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), cmd, alias);\n+\t\tfree(alias);\n+\t\texit(0);\n+\t}\n+\n+\tif (swapped)\n+\t\treturn help_unknown_cmd(cmd);\n+\n+\treturn cmd;\n+}\n+\n int cmd_help(int argc, const char **argv, const char *prefix)\n {\n \tint nongit;\n-\tchar *alias;\n \tenum help_format parsed_help_format;\n \n \targc = parse_options(argc, argv, prefix, builtin_help_options,\n@@ -476,12 +497,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)\n \tif (help_format == HELP_FORMAT_NONE)\n \t\thelp_format = parse_help_format(DEFAULT_HELP_FORMAT);\n \n-\talias = alias_lookup(argv[0]);\n-\tif (alias && !is_git_command(argv[0])) {\n-\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), argv[0], alias);\n-\t\tfree(alias);\n-\t\treturn 0;\n-\t}\n+\targv[0] = check_git_cmd(argv[0]);\n \n \tswitch (help_format) {\n \tcase HELP_FORMAT_NONE:\ndiff --git a/git.c b/git.c\nindex 0f1937f..71ea983 100644\n--- a/git.c\n+++ b/git.c\n@@ -528,10 +528,23 @@ static void handle_builtin(int argc, const char **argv)\n \tstrip_extension(argv);\n \tcmd = argv[0];\n \n-\t/* Turn \"git cmd --help\" into \"git help cmd\" */\n+\t/* Turn \"git cmd --help\" into \"git help --swapped cmd\" */\n \tif (argc > 1 && !strcmp(argv[1], \"--help\")) {\n+\t\tstruct argv_array args;\n+\t\tint i;\n+\n \t\targv[1] = argv[0];\n \t\targv[0] = cmd = \"help\";\n+\n+\t\targv_array_init(&args);\n+\t\tfor (i = 0; i < argc; i++) {\n+\t\t\targv_array_push(&args, argv[i]);\n+\t\t\tif (i == 0)\n+\t\t\t\targv_array_push(&args, \"--swapped\");\n+\t\t}\n+\n+\t\targc++;\n+\t\targv = argv_array_detach(&args);\n \t}\n \n \tbuiltin = get_builtin(cmd);\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nnew file mode 100755\nindex 0000000..6f700b1\n--- /dev/null\n+++ b/t/t0012-help.sh\n@@ -0,0 +1,15 @@\n+#!/bin/sh\n+\n+test_description='help'\n+\n+. ./test-lib.sh\n+\n+test_expect_success \"pass --help to common guide\" \"\n+\tcat <<-EOF >expected &&\n+\t\tgit: 'revisions' is not a git command. See 'git --help'.\n+\tEOF\n+\t(git revisions --help 2>actual || true) &&\n+\ttest_i18ncmp expected actual\n+\"\n+\n+test_done\n-- \n2.9.2.912.g69c5047\n\n"},{"id":"299477","messageId":"20160816163334.xkkuffjwzc6mw663@john.keeping.me.uk","threadId":"43792","inReplyTo":"20160816162030.27754-1-ralf.thielow@gmail.com","subject":"Re: [PATCH v3] help: make option --help open man pages only for Git commands","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2016-08-16T16:33:34Z","receivedAt":"2016-08-16T16:34:38Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Aug 16, 2016 at 06:20:30PM +0200, Ralf Thielow wrote:\n> If option --help is passed to a Git command, we try to open\n> the man page of that command. However, we do it even for commands\n> we don't know. Make sure it is a Git command by using \"help_unknown_cmd\"\n> which is even able to assume a command if the user made a typo.\n> \n> This breaks \"git --help\" while \"git help \" still works.\n> \n> As \" --help\" will internally be turned into \"help \",\n> introduce the hidden option \"--swapped\" in order to know which\n> version has been called.\n> \n> Signed-off-by: Ralf Thielow \n> ---\n> Thanks, all, for the help!\n> \n> Changes since v2:\n> - don't check for common guides as the list is very incomplete\n> - only check for git commands when called via --help (introduce\n> option --swapped for that), as suggested by Junio\n> - change test case to check for --help being passed to a concept\n> used as a git command\n> \n> builtin/help.c | 30 +++++++++++++++++++++++-------\n> git.c | 15 ++++++++++++++-\n> t/t0012-help.sh | 15 +++++++++++++++\n> 3 files changed, 52 insertions(+), 8 deletions(-)\n> create mode 100755 t/t0012-help.sh\n> \n> diff --git a/builtin/help.c b/builtin/help.c\n> index 8848013..76f07c7 100644\n> --- a/builtin/help.c\n> +++ b/builtin/help.c\n> @@ -37,7 +37,9 @@ static int show_all = 0;\n> static int show_guides = 0;\n> static unsigned int colopts;\n> static enum help_format help_format = HELP_FORMAT_NONE;\n> +static int swapped = 0;\n> static struct option builtin_help_options[] = {\n> +\tOPT_BOOL('s', \"swapped\", &swapped, \"mark as being called by --help\"),\n\nOPT_HIDDEN_BOOL maybe?\n\n> \tOPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n> \tOPT_BOOL('g', \"guides\", &show_guides, N_(\"print list of useful guides\")),\n> \tOPT_SET_INT('m', \"man\", &help_format, N_(\"show man page\"), HELP_FORMAT_MAN),\n"},{"id":"299479","messageId":"CAN0XMO+fikDov1FpOBUbLZUsfQpYtW3DNTEx5Fa0_Cbos4N_cw@mail.gmail.com","threadId":"43792","inReplyTo":"20160816163334.xkkuffjwzc6mw663@john.keeping.me.uk","subject":"Re: [PATCH v3] help: make option --help open man pages only for Git commands","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-16T16:39:15Z","receivedAt":"2016-08-16T16:39:21Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"2016-08-16 18:33 GMT+02:00 John Keeping :\n> On Tue, Aug 16, 2016 at 06:20:30PM +0200, Ralf Thielow wrote:\n>> static struct option builtin_help_options[] = {\n>> + OPT_BOOL('s', \"swapped\", &swapped, \"mark as being called by --help\"),\n>\n> OPT_HIDDEN_BOOL maybe?\n>\n\nYeah >_<\n\nThanks!\n"},{"id":"299486","messageId":"xmqq60r0ei9k.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"20160816162030.27754-1-ralf.thielow@gmail.com","subject":"Re: [PATCH v3] help: make option --help open man pages only for Git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-16T17:27:35Z","receivedAt":"2016-08-16T17:27:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow writes:\n\n> builtin/help.c | 30 +++++++++++++++++++++++-------\n> git.c | 15 ++++++++++++++-\n> t/t0012-help.sh | 15 +++++++++++++++\n> 3 files changed, 52 insertions(+), 8 deletions(-)\n> create mode 100755 t/t0012-help.sh\n>\n> diff --git a/builtin/help.c b/builtin/help.c\n> index 8848013..76f07c7 100644\n> --- a/builtin/help.c\n> +++ b/builtin/help.c\n> @@ -37,7 +37,9 @@ static int show_all = 0;\n> static int show_guides = 0;\n> static unsigned int colopts;\n> static enum help_format help_format = HELP_FORMAT_NONE;\n> +static int swapped = 0;\n\nThis is not the first offender (show_guides above does so, too), but\nplease do not initialize static explicitly to 0 or NULL.\n\n> static struct option builtin_help_options[] = {\n> +\tOPT_BOOL('s', \"swapped\", &swapped, \"mark as being called by --help\"),\n> \tOPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n> \tOPT_BOOL('g', \"guides\", &show_guides, N_(\"print list of useful guides\")),\n> \tOPT_SET_INT('m', \"man\", &help_format, N_(\"show man page\"), HELP_FORMAT_MAN),\n> @@ -433,10 +435,29 @@ static void list_common_guides_help(void)\n> \tputchar('\\n');\n> }\n> \n> +static const char* check_git_cmd(const char* cmd)\n\nStyle: \"static const char *check_git_cmd(const char *cmd)\". The\nasterisk that turns the base type to a pointer to the base type\nsticks to the identifier, not to the type.\n\n> +{\n> +\tchar *alias;\n> +\n> +\tif (is_git_command(cmd))\n> +\t\treturn cmd;\n> +\n> +\talias = alias_lookup(cmd);\n> +\tif (alias) {\n> +\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), cmd, alias);\n> +\t\tfree(alias);\n> +\t\texit(0);\n> +\t}\n> +\n> +\tif (swapped)\n> +\t\treturn help_unknown_cmd(cmd);\n\nI am guilty of suggesting \"swapped\"; even if we are going to mark\nthis as OPT_HIDDEN, I think we should be able to think of a better\nname. I think the meaning of this boolean is \"we know that this is\nnot a guide and is meant to be a command.\", and I hope we can come\nup with a name that concisely expresses that (e.g. \"--not-a-guide\",\n\"--must-be-a-command\").\n\n> +\treturn cmd;\n> +}\n> +\n> int cmd_help(int argc, const char **argv, const char *prefix)\n> {\n> \tint nongit;\n> -\tchar *alias;\n> \tenum help_format parsed_help_format;\n> \n> \targc = parse_options(argc, argv, prefix, builtin_help_options,\n> @@ -476,12 +497,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)\n> \tif (help_format == HELP_FORMAT_NONE)\n> \t\thelp_format = parse_help_format(DEFAULT_HELP_FORMAT);\n> \n> -\talias = alias_lookup(argv[0]);\n> -\tif (alias && !is_git_command(argv[0])) {\n> -\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), argv[0], alias);\n> -\t\tfree(alias);\n> -\t\treturn 0;\n> -\t}\n> +\targv[0] = check_git_cmd(argv[0]);\n> \n> \tswitch (help_format) {\n> \tcase HELP_FORMAT_NONE:\n> diff --git a/git.c b/git.c\n> index 0f1937f..71ea983 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -528,10 +528,23 @@ static void handle_builtin(int argc, const char **argv)\n> \tstrip_extension(argv);\n> \tcmd = argv[0];\n> \n> -\t/* Turn \"git cmd --help\" into \"git help cmd\" */\n> +\t/* Turn \"git cmd --help\" into \"git help --swapped cmd\" */\n> \tif (argc > 1 && !strcmp(argv[1], \"--help\")) {\n> +\t\tstruct argv_array args;\n> +\t\tint i;\n> +\n> \t\targv[1] = argv[0];\n> \t\targv[0] = cmd = \"help\";\n> +\n> +\t\targv_array_init(&args);\n> +\t\tfor (i = 0; i < argc; i++) {\n> +\t\t\targv_array_push(&args, argv[i]);\n> +\t\t\tif (i == 0)\n\nIt is more idiomatic to say\n\n\t\t\tif (!i)\n\naround here.\n\n> +\t\t\t\targv_array_push(&args, \"--swapped\");\n\n> +\t\t}\n> +\n> +\t\targc++;\n> +\t\targv = argv_array_detach(&args);\n> \t}\n> \n> \tbuiltin = get_builtin(cmd);\n\nThe code does this after it:\n\n\tif (builtin)\n \texit(run_builtin(...));\n\nand returns. If we didn't get builtin, we risk leaking args.argv\nhere, but we assume argv[0] = cmd = \"help\" is always a builtin,\nwhich I think is a safe assumption, so the code is OK. Static\ncheckers that are only half intelligent may yell at you for not\nreleasing the resources, though.\n\n> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n> new file mode 100755\n> index 0000000..6f700b1\n> --- /dev/null\n> +++ b/t/t0012-help.sh\n> @@ -0,0 +1,15 @@\n> +#!/bin/sh\n> +\n> +test_description='help'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success \"pass --help to common guide\" \"\n> +\tcat <<-EOF >expected &&\n> +\t\tgit: 'revisions' is not a git command. See 'git --help'.\n> +\tEOF\n> +\t(git revisions --help 2>actual || true) &&\n> +\ttest_i18ncmp expected actual\n> +\"\n> +\n> +test_done\n"},{"id":"299491","messageId":"CAN0XMOL2WZkh17v-7OtA8AMbzs4NUuj8xPVwJBD-PnK4fBaUFw@mail.gmail.com","threadId":"43792","inReplyTo":"xmqq60r0ei9k.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3] help: make option --help open man pages only for Git commands","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-16T17:57:00Z","receivedAt":"2016-08-16T17:57:06Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"2016-08-16 19:27 GMT+02:00 Junio C Hamano :\n> Ralf Thielow writes:\n>> +\n>> + if (swapped)\n>> + return help_unknown_cmd(cmd);\n>\n> I am guilty of suggesting \"swapped\"; even if we are going to mark\n> this as OPT_HIDDEN, I think we should be able to think of a better\n> name. I think the meaning of this boolean is \"we know that this is\n> not a guide and is meant to be a command.\", and I hope we can come\n> up with a name that concisely expresses that (e.g. \"--not-a-guide\",\n> \"--must-be-a-command\").\n>\n\nI think \"--cmd-only\" is a good name. With a good name I think it's worth\nto make this option visible to the user.\n"},{"id":"299498","messageId":"xmqqd1l8cz3n.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"CAN0XMOL2WZkh17v-7OtA8AMbzs4NUuj8xPVwJBD-PnK4fBaUFw@mail.gmail.com","subject":"Re: [PATCH v3] help: make option --help open man pages only for Git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-16T19:06:52Z","receivedAt":"2016-08-16T19:07:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow writes:\n\n> 2016-08-16 19:27 GMT+02:00 Junio C Hamano :\n>> Ralf Thielow writes:\n>>> +\n>>> + if (swapped)\n>>> + return help_unknown_cmd(cmd);\n>>\n>> I am guilty of suggesting \"swapped\"; even if we are going to mark\n>> this as OPT_HIDDEN, I think we should be able to think of a better\n>> name. I think the meaning of this boolean is \"we know that this is\n>> not a guide and is meant to be a command.\", and I hope we can come\n>> up with a name that concisely expresses that (e.g. \"--not-a-guide\",\n>> \"--must-be-a-command\").\n>>\n>\n> I think \"--cmd-only\" is a good name. With a good name I think it's worth\n> to make this option visible to the user.\n\nSure. I'd prefer \"cmd\" to be spelled out in anything that is\nend-user facing, though.\n"},{"id":"299627","messageId":"xmqqy43t6ek8.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"20160818185719.4909-3-ralf.thielow@gmail.com","subject":"Re: [PATCH 2/2] help: make option --help open man pages only for Git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-18T19:51:35Z","receivedAt":"2016-08-19T01:11:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow writes:\n\n> If option --help is passed to a Git command, we try to open\n> the man page of that command. However, we do it even for commands\n> we don't know. Make sure it is a Git command.\n\nWhat the patch does is correct, I think, but the explanation may\ninvite a false alarm. If you added a custom command git-who in your\n$PATH, with an appropriate documentation for git-who(1), we would\nstill show its documentation, no?\n\nThe same comment applies to 1/2, too, in that the word \"command\"\nwill be interpreted differently by different people. For example,\n\"git co --help\" and \"git help co\" would work, with or without 1/2 in\nplace when you have \"[alias] co = checkout\", so we are calling \"Git\nsubcommands that we ship, custom commands 'git-$foo' the users have\nin their $PATH, and aliases the users create\" collectively \"command\".\n\nAs long as the reader understands that definition, both the log\nmessages of 1/2 and 2/2 _and_ the updated description for \"git help\"\nwe have in 1/2 are all very clear. I do not care too much about the\ncommit log message, but we may want to think about the documentation\na bit more.\n\nHere is what 1/2 adds to \"git help\" documentation:\n\n +Note that `git --help ...` is almost identical to `git help ...` because\n +the former is internally converted into the latter with option --command-only\n +being added.\n\n To display the linkgit:git[1] man page, use `git help git`.\n\n @@ -43,6 +44,10 @@ OPTIONS\n Prints all the available commands on the standard output. This\n option overrides any given command or guide name.\n\n +-c::\n +--command-only::\n +\tDisplay help information only for commands.\n +\n\nFirst, I do not think a short form is unnecessary; the users are not\nexpected to use that form, once they started typing \"git help...\".\nIf we flip the polarity and call it --exclude-guides or something,\nwould it make it less ambiguous?\n\n> This breaks \"git --help\" while \"git help \" still works.\n\nI wouldn't call that a breakage; \"git everyday --help\" shouldn't\nhave worked in the first place. It did something useful merely by\naccident ;-).\n\n> diff --git a/git.c b/git.c\n> index 0f1937f..2cd2e06 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -528,10 +528,23 @@ static void handle_builtin(int argc, const char **argv)\n> \tstrip_extension(argv);\n> \tcmd = argv[0];\n> \n> -\t/* Turn \"git cmd --help\" into \"git help cmd\" */\n> +\t/* Turn \"git cmd --help\" into \"git help --command-only cmd\" */\n> \tif (argc > 1 && !strcmp(argv[1], \"--help\")) {\n> +\t\tstruct argv_array args;\n> +\t\tint i;\n> +\n> \t\targv[1] = argv[0];\n> \t\targv[0] = cmd = \"help\";\n> +\n> +\t\targv_array_init(&args);\n> +\t\tfor (i = 0; i < argc; i++) {\n> +\t\t\targv_array_push(&args, argv[i]);\n> +\t\t\tif (!i)\n> +\t\t\t\targv_array_push(&args, \"--command-only\");\n> +\t\t}\n> +\n> +\t\targc++;\n> +\t\targv = argv_array_detach(&args);\n> \t}\n> \n> \tbuiltin = get_builtin(cmd);\n\nThe code does this after it:\n\n if (builtin)\n exit(run_builtin(...));\n\nand returns. If we didn't get builtin, we risk leaking args.argv\nhere, but we assume argv[0] = cmd = \"help\" is always a builtin,\nwhich I think is a safe assumption, so the code is OK. Static\ncheckers that are only half intelligent may yell at you for not\nreleasing the resources, though. I wonder if it is worth doing\n\n static void handle_builtin(int argc, const char **argv)\n {\n struct argv_array args = ARGV_ARRAY_INIT;\n ...\n if (argc > 1 && !strcmp(argv[1], \"--help\")) {\n ...\n argv = args.argv;\n }\n builtin = get_builtin(cmd);\n if (builtin)\n exit(run_builtin(...));\n argv_array_clear(&args);\n } \n\nto help unconfuse them.\n\nBy the way, I do not see these patches on gmane, public-inbox or\nusual suspects. Perhaps vger is having a bad day or something?\n\n"},{"id":"299642","messageId":"20160818185719.4909-2-ralf.thielow@gmail.com","threadId":"43792","inReplyTo":"20160818185719.4909-1-ralf.thielow@gmail.com","subject":"[PATCH 1/2] help: introduce option --command-only","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-18T18:57:18Z","receivedAt":"2016-08-19T01:14:46Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"Introduce option --command-only to the help command. With this option\nbeing passed, \"git help\" will open man pages only for commands.\n\nSince we know it is a command, we can use function help_unknown_command\nto give the user advice on typos.\n\nSigned-off-by: Ralf Thielow \n---\nI am not sure about the first test case, but I think it'd have\nprevented me from making earlier mistakes of this change. That's\nwhy I added it.\nJust calling a git command that succeeds in a test isn't really\na check, so ... I dunno\n\n Documentation/git-help.txt | 11 ++++++++---\n builtin/help.c | 30 +++++++++++++++++++++++-------\n contrib/completion/git-completion.bash | 2 +-\n t/t0012-help.sh | 21 +++++++++++++++++++++\n 4 files changed, 53 insertions(+), 11 deletions(-)\n create mode 100755 t/t0012-help.sh\n\ndiff --git a/Documentation/git-help.txt b/Documentation/git-help.txt\nindex 40d328a..cf6a414 100644\n--- a/Documentation/git-help.txt\n+++ b/Documentation/git-help.txt\n@@ -8,7 +8,7 @@ git-help - Display help information about Git\n SYNOPSIS\n --------\n [verse]\n-'git help' [-a|--all] [-g|--guide]\n+'git help' [-a|--all] [-c|--command-only] [-g|--guide]\n \t [-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]\n \n DESCRIPTION\n@@ -29,8 +29,9 @@ guide is brought up. The 'man' program is used by default for this\n purpose, but this can be overridden by other options or configuration\n variables.\n \n-Note that `git --help ...` is identical to `git help ...` because the\n-former is internally converted into the latter.\n+Note that `git --help ...` is almost identical to `git help ...` because\n+the former is internally converted into the latter with option --command-only\n+being added.\n \n To display the linkgit:git[1] man page, use `git help git`.\n \n@@ -43,6 +44,10 @@ OPTIONS\n \tPrints all the available commands on the standard output. This\n \toption overrides any given command or guide name.\n \n+-c::\n+--command-only::\n+\tDisplay help information only for commands.\n+\n -g::\n --guides::\n \tPrints a list of useful guides on the standard output. This\ndiff --git a/builtin/help.c b/builtin/help.c\nindex 8848013..2249a67 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -37,8 +37,10 @@ static int show_all = 0;\n static int show_guides = 0;\n static unsigned int colopts;\n static enum help_format help_format = HELP_FORMAT_NONE;\n+static int cmd_only;\n static struct option builtin_help_options[] = {\n \tOPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n+\tOPT_BOOL('c', \"command-only\", &cmd_only, N_(\"show help only for commands\")),\n \tOPT_BOOL('g', \"guides\", &show_guides, N_(\"print list of useful guides\")),\n \tOPT_SET_INT('m', \"man\", &help_format, N_(\"show man page\"), HELP_FORMAT_MAN),\n \tOPT_SET_INT('w', \"web\", &help_format, N_(\"show manual in web browser\"),\n@@ -433,10 +435,29 @@ static void list_common_guides_help(void)\n \tputchar('\\n');\n }\n \n+static const char *check_git_cmd(const char* cmd)\n+{\n+\tchar *alias;\n+\n+\tif (is_git_command(cmd))\n+\t\treturn cmd;\n+\n+\talias = alias_lookup(cmd);\n+\tif (alias) {\n+\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), cmd, alias);\n+\t\tfree(alias);\n+\t\texit(0);\n+\t}\n+\n+\tif (cmd_only)\n+\t\treturn help_unknown_cmd(cmd);\n+\n+\treturn cmd;\n+}\n+\n int cmd_help(int argc, const char **argv, const char *prefix)\n {\n \tint nongit;\n-\tchar *alias;\n \tenum help_format parsed_help_format;\n \n \targc = parse_options(argc, argv, prefix, builtin_help_options,\n@@ -476,12 +497,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)\n \tif (help_format == HELP_FORMAT_NONE)\n \t\thelp_format = parse_help_format(DEFAULT_HELP_FORMAT);\n \n-\talias = alias_lookup(argv[0]);\n-\tif (alias && !is_git_command(argv[0])) {\n-\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), argv[0], alias);\n-\t\tfree(alias);\n-\t\treturn 0;\n-\t}\n+\targv[0] = check_git_cmd(argv[0]);\n \n \tswitch (help_format) {\n \tcase HELP_FORMAT_NONE:\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c1b2135..354afe5 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1393,7 +1393,7 @@ _git_help ()\n {\n \tcase \"$cur\" in\n \t--*)\n-\t\t__gitcomp \"--all --guides --info --man --web\"\n+\t\t__gitcomp \"--all --command-only --guides --info --man --web\"\n \t\treturn\n \t\t;;\n \tesac\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nnew file mode 100755\nindex 0000000..e20f907\n--- /dev/null\n+++ b/t/t0012-help.sh\n@@ -0,0 +1,21 @@\n+#!/bin/sh\n+\n+test_description='help'\n+\n+. ./test-lib.sh\n+\n+test_expect_success \"works for commands and guides by default\" \"\n+\tgit help status &&\n+\tgit help revisions\n+\"\n+\n+test_expect_success \"--command-only does not work for guides\" \"\n+\tgit help --command-only status &&\n+\tcat <<-EOF >expected &&\n+\t\tgit: 'revisions' is not a git command. See 'git --help'.\n+\tEOF\n+\t(git help --command-only revisions 2>actual || true) &&\n+\ttest_i18ncmp expected actual\n+\"\n+\n+test_done\n-- \n2.9.2.912.gd0c0e83\n\n"},{"id":"299651","messageId":"20160818185719.4909-1-ralf.thielow@gmail.com","threadId":"43792","inReplyTo":"20160816162030.27754-1-ralf.thielow@gmail.com","subject":"[PATCH 0/2] help: make option --help open man pages only for Git commands","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-18T18:57:17Z","receivedAt":"2016-08-19T01:32:20Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"In this version, one patch has been turned into two. The first introduces the\noption \"command-only\" to make 'help' only working for commands and additionally\ngive some nice help on typos. The second makes option --help only work for actual\nGit commands.\n\nRalf Thielow (2):\n help: introduce option --command-only\n help: make option --help open man pages only for Git commands\n\n Documentation/git-help.txt | 11 ++++++++---\n builtin/help.c | 30 +++++++++++++++++++++++-------\n contrib/completion/git-completion.bash | 2 +-\n git.c | 15 ++++++++++++++-\n t/t0012-help.sh | 29 +++++++++++++++++++++++++++++\n 5 files changed, 75 insertions(+), 12 deletions(-)\n create mode 100755 t/t0012-help.sh\n\n-- \n2.9.2.912.gd0c0e83\n\n"},{"id":"299681","messageId":"743A8D8FFE434E08B34F9AF8C21E54AE@PhilipOakley","threadId":"43792","inReplyTo":"20160818185719.4909-2-ralf.thielow@gmail.com","subject":"Re: [PATCH 1/2] help: introduce option --command-only","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-08-18T21:47:14Z","receivedAt":"2016-08-19T03:46:13Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Ralf Thielow\" \n> Introduce option --command-only to the help command. With this option\n> being passed, \"git help\" will open man pages only for commands.\n>\n> Since we know it is a command, we can use function help_unknown_command\n> to give the user advice on typos.\n>\n> Signed-off-by: Ralf Thielow \n> ---\n> I am not sure about the first test case, but I think it'd have\n> prevented me from making earlier mistakes of this change. That's\n> why I added it.\n> Just calling a git command that succeeds in a test isn't really\n> a check, so ... I dunno\n\nDo the tests work on both *nix and Windows, given that Windows uses \nthe --web option by default, so is likely to fire up a browser instead of \nthe man pages? Otherwise it sounds to be a reasonable check.\n\n>\n> Documentation/git-help.txt | 11 ++++++++---\n> builtin/help.c | 30 +++++++++++++++++++++++-------\n> contrib/completion/git-completion.bash | 2 +-\n> t/t0012-help.sh | 21 +++++++++++++++++++++\n> 4 files changed, 53 insertions(+), 11 deletions(-)\n> create mode 100755 t/t0012-help.sh\n>\n> diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt\n> index 40d328a..cf6a414 100644\n> --- a/Documentation/git-help.txt\n> +++ b/Documentation/git-help.txt\n> @@ -8,7 +8,7 @@ git-help - Display help information about Git\n> SYNOPSIS\n> --------\n> [verse]\n> -'git help' [-a|--all] [-g|--guide]\n> +'git help' [-a|--all] [-c|--command-only] [-g|--guide]\n> [-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]\n>\n> DESCRIPTION\n> @@ -29,8 +29,9 @@ guide is brought up. The 'man' program is used by \n> default for this\n> purpose, but this can be overridden by other options or configuration\n> variables.\n>\n> -Note that `git --help ...` is identical to `git help ...` because the\n> -former is internally converted into the latter.\n> +Note that `git --help ...` is almost identical to `git help ...` because\n> +the former is internally converted into the latter with \n> option --command-only\n> +being added.\n>\n> To display the linkgit:git[1] man page, use `git help git`.\n>\n> @@ -43,6 +44,10 @@ OPTIONS\n> Prints all the available commands on the standard output. This\n> option overrides any given command or guide name.\n>\n> +-c::\n> +--command-only::\n> + Display help information only for commands.\n\ns/commands/known commands/ ?\n\n> +\n> -g::\n> --guides::\n> Prints a list of useful guides on the standard output. This\n> diff --git a/builtin/help.c b/builtin/help.c\n> index 8848013..2249a67 100644\n> --- a/builtin/help.c\n> +++ b/builtin/help.c\n> @@ -37,8 +37,10 @@ static int show_all = 0;\n> static int show_guides = 0;\n> static unsigned int colopts;\n> static enum help_format help_format = HELP_FORMAT_NONE;\n> +static int cmd_only;\n> static struct option builtin_help_options[] = {\n> OPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n> + OPT_BOOL('c', \"command-only\", &cmd_only, N_(\"show help only for \n> commands\")),\n\ns/commands/known commands/ ?\n\n> OPT_BOOL('g', \"guides\", &show_guides, N_(\"print list of useful guides\")),\n> OPT_SET_INT('m', \"man\", &help_format, N_(\"show man page\"), \n> HELP_FORMAT_MAN),\n> OPT_SET_INT('w', \"web\", &help_format, N_(\"show manual in web browser\"),\n> @@ -433,10 +435,29 @@ static void list_common_guides_help(void)\n> putchar('\\n');\n> }\n>\n> +static const char *check_git_cmd(const char* cmd)\n> +{\n> + char *alias;\n> +\n> + if (is_git_command(cmd))\n> + return cmd;\n> +\n> + alias = alias_lookup(cmd);\n> + if (alias) {\n> + printf_ln(_(\"`git %s' is aliased to `%s'\"), cmd, alias);\n> + free(alias);\n> + exit(0);\n> + }\n> +\n> + if (cmd_only)\n> + return help_unknown_cmd(cmd);\n> +\n> + return cmd;\n> +}\n> +\n> int cmd_help(int argc, const char **argv, const char *prefix)\n> {\n> int nongit;\n> - char *alias;\n> enum help_format parsed_help_format;\n>\n> argc = parse_options(argc, argv, prefix, builtin_help_options,\n> @@ -476,12 +497,7 @@ int cmd_help(int argc, const char **argv, const char \n> *prefix)\n> if (help_format == HELP_FORMAT_NONE)\n> help_format = parse_help_format(DEFAULT_HELP_FORMAT);\n>\n> - alias = alias_lookup(argv[0]);\n> - if (alias && !is_git_command(argv[0])) {\n> - printf_ln(_(\"`git %s' is aliased to `%s'\"), argv[0], alias);\n> - free(alias);\n> - return 0;\n> - }\n> + argv[0] = check_git_cmd(argv[0]);\n>\n> switch (help_format) {\n> case HELP_FORMAT_NONE:\n> diff --git a/contrib/completion/git-completion.bash \n> b/contrib/completion/git-completion.bash\n> index c1b2135..354afe5 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1393,7 +1393,7 @@ _git_help ()\n> {\n> case \"$cur\" in\n> --*)\n> - __gitcomp \"--all --guides --info --man --web\"\n> + __gitcomp \"--all --command-only --guides --info --man --web\"\n> return\n> ;;\n> esac\n> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n> new file mode 100755\n> index 0000000..e20f907\n> --- /dev/null\n> +++ b/t/t0012-help.sh\n> @@ -0,0 +1,21 @@\n> +#!/bin/sh\n> +\n> +test_description='help'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success \"works for commands and guides by default\" \"\n> + git help status &&\n> + git help revisions\n> +\"\n> +\n> +test_expect_success \"--command-only does not work for guides\" \"\n> + git help --command-only status &&\n> + cat <<-EOF >expected &&\n> + git: 'revisions' is not a git command. See 'git --help'.\n> + EOF\n> + (git help --command-only revisions 2>actual || true) &&\n> + test_i18ncmp expected actual\n> +\"\n> +\n> +test_done\n> -- \n> 2.9.2.912.gd0c0e83\n>\n>\n--\nPhilip \n\n"},{"id":"299684","messageId":"20160818185719.4909-3-ralf.thielow@gmail.com","threadId":"43792","inReplyTo":"20160818185719.4909-2-ralf.thielow@gmail.com","subject":"[PATCH 2/2] help: make option --help open man pages only for Git commands","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-18T18:57:19Z","receivedAt":"2016-08-19T06:33:54Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"If option --help is passed to a Git command, we try to open\nthe man page of that command. However, we do it even for commands\nwe don't know. Make sure it is a Git command.\n\nThis breaks \"git --help\" while \"git help \" still works.\n\nSigned-off-by: Ralf Thielow \n---\n git.c | 15 ++++++++++++++-\n t/t0012-help.sh | 8 ++++++++\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/git.c b/git.c\nindex 0f1937f..2cd2e06 100644\n--- a/git.c\n+++ b/git.c\n@@ -528,10 +528,23 @@ static void handle_builtin(int argc, const char **argv)\n \tstrip_extension(argv);\n \tcmd = argv[0];\n \n-\t/* Turn \"git cmd --help\" into \"git help cmd\" */\n+\t/* Turn \"git cmd --help\" into \"git help --command-only cmd\" */\n \tif (argc > 1 && !strcmp(argv[1], \"--help\")) {\n+\t\tstruct argv_array args;\n+\t\tint i;\n+\n \t\targv[1] = argv[0];\n \t\targv[0] = cmd = \"help\";\n+\n+\t\targv_array_init(&args);\n+\t\tfor (i = 0; i < argc; i++) {\n+\t\t\targv_array_push(&args, argv[i]);\n+\t\t\tif (!i)\n+\t\t\t\targv_array_push(&args, \"--command-only\");\n+\t\t}\n+\n+\t\targc++;\n+\t\targv = argv_array_detach(&args);\n \t}\n \n \tbuiltin = get_builtin(cmd);\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nindex e20f907..81fec90 100755\n--- a/t/t0012-help.sh\n+++ b/t/t0012-help.sh\n@@ -18,4 +18,12 @@ test_expect_success \"--command-only does not work for guides\" \"\n \ttest_i18ncmp expected actual\n \"\n \n+test_expect_success \"--help does not work for guides\" \"\n+\tcat <<-EOF >expected &&\n+\t\tgit: 'revisions' is not a git command. See 'git --help'.\n+\tEOF\n+\t(git revisions --help 2>actual || true) &&\n+\ttest_i18ncmp expected actual\n+\"\n+\n test_done\n-- \n2.9.2.912.gd0c0e83\n\n"},{"id":"299686","messageId":"1462477081.1237058.1471595951145.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"43792","inReplyTo":"20160818185719.4909-2-ralf.thielow@gmail.com","subject":"Re: [PATCH 1/2] help: introduce option --command-only","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2016-08-19T08:39:11Z","receivedAt":"2016-08-19T08:18:12Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Hi Ralf,\n\nRalf Thielow writes:\n> [...]\n> +test_expect_success \"works for commands and guides by default\" \"\n> + git help status &&\n> + git help revisions\n> +\"\n> +\n> +test_expect_success \"--command-only does not work for guides\" \"\n> + git help --command-only status &&\n> + cat <<-EOF >expected &&\n> + git: 'revisions' is not a git command. See 'git --help'.\n> + EOF\n> + (git help --command-only revisions 2>actual || true) &&\n\nI think you want to use\n `test_must_fail git help --command-only revisions 2>actual`\nhere to make sure that the command does fail.\n\nThanks,\nRémi\n"},{"id":"299687","messageId":"alpine.DEB.2.20.1608190954461.4924@virtualbox","threadId":"43792","inReplyTo":"20160818185719.4909-2-ralf.thielow@gmail.com","subject":"Re: [PATCH 1/2] help: introduce option --command-only","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-19T08:32:30Z","receivedAt":"2016-08-19T08:32:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ralf,\n\nOn Thu, 18 Aug 2016, Ralf Thielow wrote:\n\n> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n> new file mode 100755\n> index 0000000..e20f907\n> --- /dev/null\n> +++ b/t/t0012-help.sh\n> @@ -0,0 +1,21 @@\n> +#!/bin/sh\n> +\n> +test_description='help'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success \"works for commands and guides by default\" \"\n> +\tgit help status &&\n> +\tgit help revisions\n> +\"\n\nApart from using double quotes (which is inconsistent with the single\nquotes used literally everwhere else in the test suite), this test is\nincorrect. If the man page is not *installed*, it will fail:\n\n$ sudo mv /usr/share/man/man1/git-status.1.gz \\\n\t/usr/share/man/man1/git-status.old.1.gz\n\n$ sh t0012-help.sh -i -v -x\nInitialized empty Git repository in .../trash directory.t0012-help/.git/\nexpecting success:\n git help status &&\n git help revisions\n\n+ git help status\nNo manual entry for git-status\nSee 'man 7 undocumented' for help when manual pages are not available.\nerror: last command exited with $?=16\nnot ok 1 - works for commands and guides by default\n#\n# git help status &&\n# git help revisions\n#\n\nIt gets even worse.\n\nOn Windows, the default format is *not* man pages but html pages. So those\nwould have to be installed, too, to guarantee that the test succeeds.\n\nIt gets *even* worse.\n\nOn Windows, there is really no central location for man/html pages for\ndocumentation, so we have to emulate that \"prefix\" (which is typically\n/usr on Linux) via a \"runtime prefix\", i.e. a prefix determined relative\nto the location of the currently running git executable. In the test\nsuite's case, it is typically the top-level directory of the git.git\ncheckout [*1*]. There are no man/html pages in that directory structure by\ndefault (I, for one, rarely build them myself), and certainly not in the\nplace expected by this test.\n\nIt gets *even worse*.\n\nSince the help.format is html on Windows, the page is opened by the\ndefault viewer for HTML pages. So even if all of the above would be fixed,\nrunning t0012-help of a supposedly unsupervised test suite would open new\ntabs in the web browser. Probably forcing it into the foreground, too.\n\nSo how about fixing that? I would suggest to do it this way:\n\n- configure help.format = html (for \"man\", the current code would always\n add $(prefix)/share/man to the MANPATH when testing, not what we want,\n and hacking this code *just* for testing is both ugly and unnecessary).\n\n- configure help.htmlpath to point to a subdirectory that is created and\n populated in the same test script.\n\n- configure help.browser to point to a script that is created in the same\n script and whose output we can verify, too.\n\nThe last point actually requires a patch that was recently introduced into\nGit for Windows [*1*] (and that did not make it upstream yet) which\nreverts that change whereby web--browse was sidestepped. That sidestepping\nwas well-intentioned but turned out to cause more harm than good.\n\nCiao,\nJohannes\n\nFootnote *1*: That statement is actually not even correct. As the git\nexecutable can live in both $(prefix)/bin/ and $(prefix)/libexec/git-core,\ni.e. at different directory levels below the prefix, we need to inspect\nthe *name* of the directory in which git.exe lives, and a git.git checkout\ntypically lives in a .../git/ directory which matches *none* of the\nexpected suffixes, so the runtime prefix defaults to \"/\", i.e. the\n*current drive's root directory*. So your current test would only succeed\nif the man pages for git-status and gitrevisions were copied into\nC:\\mingw64\\share\\man\\man1!\n\nFootnote *2*: https://github.com/git-for-windows/git/commit/243c72f5b0\n"},{"id":"299712","messageId":"xmqqinuw4uww.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"alpine.DEB.2.20.1608190954461.4924@virtualbox","subject":"Re: [PATCH 1/2] help: introduce option --command-only","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-19T15:53:35Z","receivedAt":"2016-08-19T15:53:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> - configure help.format = html (for \"man\", the current code would always\n> add $(prefix)/share/man to the MANPATH when testing, not what we want,\n> and hacking this code *just* for testing is both ugly and unnecessary).\n\nA very constructive suggestion to show a good direction. Because we\nare not in the business of verifying the \"man\" works as expected and\nhow it wants the files in MANPATH are structured, it is a very good\nidea to do our testing with the HTML format, which gives us tighter\ncontrol of what we actually use for our testing with help.browser\nconfiguration variable. I really like it.\n\n> - configure help.htmlpath to point to a subdirectory that is created and\n> populated in the same test script.\n\nYup!\n\n> - configure help.browser to point to a script that is created in the same\n> script and whose output we can verify, too.\n\nYup, yup! The \"browser\" can be something that parrots its command\nline to the standard output and does not even have to care if the\nfile pointed at is HTML at all.\n\n> The last point actually requires a patch that was recently introduced into\n> Git for Windows [*1*] (and that did not make it upstream yet) which\n> reverts that change whereby web--browse was sidestepped. That sidestepping\n> was well-intentioned but turned out to cause more harm than good.\n\nGood.\n"},{"id":"299971","messageId":"CAN0XMOJCCaOCuTejgmiAYUDMJfdMEpv5ZATLrN4ruCaGK=FjnA@mail.gmail.com","threadId":"43792","inReplyTo":"xmqqy43t6ek8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] help: make option --help open man pages only for Git commands","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-23T17:34:46Z","receivedAt":"2016-08-23T17:34:52Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"Sorry for being late in responding. It's been busy days.\n\n2016-08-18 21:51 GMT+02:00 Junio C Hamano :\n> Ralf Thielow writes:\n>\n> The same comment applies to 1/2, too, in that the word \"command\"\n> will be interpreted differently by different people. For example,\n> \"git co --help\" and \"git help co\" would work, with or without 1/2 in\n> place when you have \"[alias] co = checkout\", so we are calling \"Git\n> subcommands that we ship, custom commands 'git-$foo' the users have\n> in their $PATH, and aliases the users create\" collectively \"command\".\n>\n> As long as the reader understands that definition, both the log\n> messages of 1/2 and 2/2 _and_ the updated description for \"git help\"\n> we have in 1/2 are all very clear. I do not care too much about the\n> commit log message, but we may want to think about the documentation\n> a bit more.\n>\n> Here is what 1/2 adds to \"git help\" documentation:\n>\n> +Note that `git --help ...` is almost identical to `git help ...` because\n> +the former is internally converted into the latter with option --command-only\n> +being added.\n>\n> To display the linkgit:git[1] man page, use `git help git`.\n>\n> @@ -43,6 +44,10 @@ OPTIONS\n> Prints all the available commands on the standard output. This\n> option overrides any given command or guide name.\n>\n> +-c::\n> +--command-only::\n> + Display help information only for commands.\n> +\n>\n> First, I do not think a short form is unnecessary; the users are not\n> expected to use that form, once they started typing \"git help...\".\n> If we flip the polarity and call it --exclude-guides or something,\n> would it make it less ambiguous?\n>\n\nSure. Since \"command\" is understood as both Git command\nand guide in this context, the name --exclude-guides would describe the\nbehaviour of that option less ambiguous. I'll rename it.\n\n>> This breaks \"git --help\" while \"git help \" still works.\n>\n> I wouldn't call that a breakage; \"git everyday --help\" shouldn't\n> have worked in the first place. It did something useful merely by\n> accident ;-).\n>\n\nOK, I'll call it \"doesn't work anymore\".\n\n>> diff --git a/git.c b/git.c\n>> index 0f1937f..2cd2e06 100644\n>> --- a/git.c\n>> +++ b/git.c\n>> @@ -528,10 +528,23 @@ static void handle_builtin(int argc, const char **argv)\n>> strip_extension(argv);\n>> cmd = argv[0];\n>>\n>> - /* Turn \"git cmd --help\" into \"git help cmd\" */\n>> + /* Turn \"git cmd --help\" into \"git help --command-only cmd\" */\n>> if (argc > 1 && !strcmp(argv[1], \"--help\")) {\n>> + struct argv_array args;\n>> + int i;\n>> +\n>> argv[1] = argv[0];\n>> argv[0] = cmd = \"help\";\n>> +\n>> + argv_array_init(&args);\n>> + for (i = 0; i < argc; i++) {\n>> + argv_array_push(&args, argv[i]);\n>> + if (!i)\n>> + argv_array_push(&args, \"--command-only\");\n>> + }\n>> +\n>> + argc++;\n>> + argv = argv_array_detach(&args);\n>> }\n>>\n>> builtin = get_builtin(cmd);\n>\n> The code does this after it:\n>\n> if (builtin)\n> exit(run_builtin(...));\n>\n> and returns. If we didn't get builtin, we risk leaking args.argv\n> here, but we assume argv[0] = cmd = \"help\" is always a builtin,\n> which I think is a safe assumption, so the code is OK. Static\n> checkers that are only half intelligent may yell at you for not\n> releasing the resources, though. I wonder if it is worth doing\n>\n> static void handle_builtin(int argc, const char **argv)\n> {\n> struct argv_array args = ARGV_ARRAY_INIT;\n> ...\n> if (argc > 1 && !strcmp(argv[1], \"--help\")) {\n> ...\n> argv = args.argv;\n> }\n> builtin = get_builtin(cmd);\n> if (builtin)\n> exit(run_builtin(...));\n> argv_array_clear(&args);\n> }\n>\n> to help unconfuse them.\n>\n\nI'll do it this way.\n\nThanks!\n"},{"id":"299972","messageId":"CAN0XMO+4u+XY2F-mVoLXegz_uom+ffcUuFJqf8aRMiW83dydbg@mail.gmail.com","threadId":"43792","inReplyTo":"1462477081.1237058.1471595951145.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH 1/2] help: introduce option --command-only","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-23T17:37:00Z","receivedAt":"2016-08-23T17:37:06Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"2016-08-19 10:39 GMT+02:00 Remi Galan Alfonso\n:\n> Hi Ralf,\n>\n> Ralf Thielow writes:\n>> [...]\n>> +test_expect_success \"works for commands and guides by default\" \"\n>> + git help status &&\n>> + git help revisions\n>> +\"\n>> +\n>> +test_expect_success \"--command-only does not work for guides\" \"\n>> + git help --command-only status &&\n>> + cat <<-EOF >expected &&\n>> + git: 'revisions' is not a git command. See 'git --help'.\n>> + EOF\n>> + (git help --command-only revisions 2>actual || true) &&\n>\n> I think you want to use\n> `test_must_fail git help --command-only revisions 2>actual`\n> here to make sure that the command does fail.\n>\n\nThanks!\n\n> Thanks,\n> Rémi\n"},{"id":"299974","messageId":"CAN0XMOLTc5zzjXwnpDwhs-coP9BVD659CpYEJYp_v4789M2CpQ@mail.gmail.com","threadId":"43792","inReplyTo":"alpine.DEB.2.20.1608190954461.4924@virtualbox","subject":"Re: [PATCH 1/2] help: introduce option --command-only","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-23T17:41:25Z","receivedAt":"2016-08-23T17:56:45Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"2016-08-19 10:32 GMT+02:00 Johannes Schindelin :\n\n> So how about fixing that? I would suggest to do it this way:\n>\n> - configure help.format = html (for \"man\", the current code would always\n> add $(prefix)/share/man to the MANPATH when testing, not what we want,\n> and hacking this code *just* for testing is both ugly and unnecessary).\n>\n> - configure help.htmlpath to point to a subdirectory that is created and\n> populated in the same test script.\n>\n> - configure help.browser to point to a script that is created in the same\n> script and whose output we can verify, too.\n>\n> The last point actually requires a patch that was recently introduced into\n> Git for Windows [*1*] (and that did not make it upstream yet) which\n> reverts that change whereby web--browse was sidestepped. That sidestepping\n> was well-intentioned but turned out to cause more harm than good.\n>\n\nSo I'll pickup the patch you sent [1] to my series and prepare the test cases\nthe way you described to verify that the 'help' command works.\n\nThanks!\n\n[1]\nhttp://public-inbox.org/git/03ae6a9d47cb95a54960bfdc90c5392f890ff1e3.1471595956.git.johannes.schindelin@gmx.de/\n"},{"id":"300031","messageId":"alpine.DEB.2.20.1608240947380.4924@virtualbox","threadId":"43792","inReplyTo":"CAN0XMOLTc5zzjXwnpDwhs-coP9BVD659CpYEJYp_v4789M2CpQ@mail.gmail.com","subject":"Re: [PATCH 1/2] help: introduce option --command-only","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-24T07:47:54Z","receivedAt":"2016-08-24T07:48:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ralf,\n\nOn Tue, 23 Aug 2016, Ralf Thielow wrote:\n\n> 2016-08-19 10:32 GMT+02:00 Johannes Schindelin :\n> \n> > So how about fixing that? I would suggest to do it this way:\n> >\n> > - configure help.format = html (for \"man\", the current code would always\n> > add $(prefix)/share/man to the MANPATH when testing, not what we want,\n> > and hacking this code *just* for testing is both ugly and unnecessary).\n> >\n> > - configure help.htmlpath to point to a subdirectory that is created and\n> > populated in the same test script.\n> >\n> > - configure help.browser to point to a script that is created in the same\n> > script and whose output we can verify, too.\n> >\n> > The last point actually requires a patch that was recently introduced into\n> > Git for Windows [*1*] (and that did not make it upstream yet) which\n> > reverts that change whereby web--browse was sidestepped. That sidestepping\n> > was well-intentioned but turned out to cause more harm than good.\n> >\n> \n> So I'll pickup the patch you sent [1] to my series and prepare the test cases\n> the way you described to verify that the 'help' command works.\n\nThanks!\n\nCiao,\nJohannes\n"},{"id":"300277","messageId":"20160826175836.14073-1-ralf.thielow@gmail.com","threadId":"43792","inReplyTo":"20160818185719.4909-1-ralf.thielow@gmail.com","subject":"[PATCH v2 0/3] help: make option --help open man pages only for Git commands","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-26T17:58:33Z","receivedAt":"2016-08-26T17:58:52Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"Changes in v2 are:\n- add a patch from Dscho to make config variable 'help.browser' work on Windows again\n- rename option \"--command-only\" to \"--exclude-guides\" which is less ambiguous in 'help' context\n- improve test script\n- refactor usage of argv_array in handle_builtin\n\nJohannes Schindelin (1):\n Revert \"display HTML in default browser using Windows' shell API\"\n\nRalf Thielow (2):\n help: introduce option --exclude-guides\n help: make option --help open man pages only for Git commands\n\n Documentation/git-help.txt | 11 ++++++---\n builtin/help.c | 37 ++++++++++++++++++------------\n compat/mingw.c | 42 ----------------------------------\n compat/mingw.h | 3 ---\n contrib/completion/git-completion.bash | 2 +-\n git.c | 15 +++++++++++-\n t/t0012-help.sh | 41 +++++++++++++++++++++++++++++++++\n 7 files changed, 87 insertions(+), 64 deletions(-)\n create mode 100755 t/t0012-help.sh\n\n-- \n2.9.2.912.gd0c0e83\n\n"},{"id":"300278","messageId":"20160826175836.14073-2-ralf.thielow@gmail.com","threadId":"43792","inReplyTo":"20160826175836.14073-1-ralf.thielow@gmail.com","subject":"[PATCH v2 1/3] Revert \"display HTML in default browser using Windows' shell API\"","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-26T17:58:34Z","receivedAt":"2016-08-26T17:58:59Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"From: Johannes Schindelin \n\nSince 4804aab (help (Windows): Display HTML in default browser using\nWindows' shell API, 2008-07-13), Git for Windows used to call\n`ShellExecute()` to launch the default Windows handler for `.html`\nfiles.\n\nThe idea was to avoid going through a shell script, for performance\nreasons.\n\nHowever, this change ignores the `help.browser` config setting. Together\nwith browsing help not being a performance-critical operation, let's\njust revert that patch.\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Ralf Thielow \n---\n builtin/help.c | 7 -------\n compat/mingw.c | 42 ------------------------------------------\n compat/mingw.h | 3 ---\n 3 files changed, 52 deletions(-)\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex 8848013..e8f79d7 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -379,17 +379,10 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)\n \tfree(to_free);\n }\n \n-/*\n- * If open_html is not defined in a platform-specific way (see for\n- * example compat/mingw.h), we use the script web--browse to display\n- * HTML.\n- */\n-#ifndef open_html\n static void open_html(const char *path)\n {\n \texecl_git_cmd(\"web--browse\", \"-c\", \"help.browser\", path, (char *)NULL);\n }\n-#endif\n \n static void show_html_page(const char *git_cmd)\n {\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 2b5467d..3fbfda5 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1930,48 +1930,6 @@ int mingw_raise(int sig)\n \t}\n }\n \n-\n-static const char *make_backslash_path(const char *path)\n-{\n-\tstatic char buf[PATH_MAX + 1];\n-\tchar *c;\n-\n-\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)\n-\t\tdie(\"Too long path: %.*s\", 60, path);\n-\n-\tfor (c = buf; *c; c++) {\n-\t\tif (*c == '/')\n-\t\t\t*c = '\\\\';\n-\t}\n-\treturn buf;\n-}\n-\n-void mingw_open_html(const char *unixpath)\n-{\n-\tconst char *htmlpath = make_backslash_path(unixpath);\n-\ttypedef HINSTANCE (WINAPI *T)(HWND, const char *,\n-\t\t\tconst char *, const char *, const char *, INT);\n-\tT ShellExecute;\n-\tHMODULE shell32;\n-\tint r;\n-\n-\tshell32 = LoadLibrary(\"shell32.dll\");\n-\tif (!shell32)\n-\t\tdie(\"cannot load shell32.dll\");\n-\tShellExecute = (T)GetProcAddress(shell32, \"ShellExecuteA\");\n-\tif (!ShellExecute)\n-\t\tdie(\"cannot run browser\");\n-\n-\tprintf(\"Launching default browser to display HTML ...\\n\");\n-\tr = HCAST(int, ShellExecute(NULL, \"open\", htmlpath,\n-\t\t\t\tNULL, \"\\\\\", SW_SHOWNORMAL));\n-\tFreeLibrary(shell32);\n-\t/* see the MSDN documentation referring to the result codes here */\n-\tif (r <= 32) {\n-\t\tdie(\"failed to launch browser for %.*s\", MAX_PATH, unixpath);\n-\t}\n-}\n-\n int link(const char *oldpath, const char *newpath)\n {\n \ttypedef BOOL (WINAPI *T)(LPCWSTR, LPCWSTR, LPSECURITY_ATTRIBUTES);\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 95e128f..2cadb81 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -417,9 +417,6 @@ int mingw_offset_1st_component(const char *path);\n #include \n #endif\n \n-void mingw_open_html(const char *path);\n-#define open_html mingw_open_html\n-\n /**\n * Converts UTF-8 encoded string to UTF-16LE.\n *\n-- \n2.9.2.912.gd0c0e83\n\n"},{"id":"300279","messageId":"20160826175836.14073-4-ralf.thielow@gmail.com","threadId":"43792","inReplyTo":"20160826175836.14073-1-ralf.thielow@gmail.com","subject":"[PATCH v2 3/3] help: make option --help open man pages only for Git commands","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-26T17:58:36Z","receivedAt":"2016-08-26T17:59:02Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"If option --help is passed to a Git command, we try to open\nthe man page of that command. However, we do it for both commands\nand concepts. Make sure it is an actual command.\n\nThis makes \"git --help\" not working anymore, while\n\"git help \" still works.\n\nSigned-off-by: Ralf Thielow \n---\n Documentation/git-help.txt | 5 +++--\n git.c | 15 ++++++++++++++-\n t/t0012-help.sh | 8 ++++++++\n 3 files changed, 25 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-help.txt b/Documentation/git-help.txt\nindex eeb1950..8d21e9f 100644\n--- a/Documentation/git-help.txt\n+++ b/Documentation/git-help.txt\n@@ -29,8 +29,9 @@ guide is brought up. The 'man' program is used by default for this\n purpose, but this can be overridden by other options or configuration\n variables.\n \n-Note that `git --help ...` is identical to `git help ...` because the\n-former is internally converted into the latter.\n+Note that `git --help ...` is almost identical to `git help ...` because\n+the former is internally converted into the latter with option --exclude-guides\n+being added.\n \n To display the linkgit:git[1] man page, use `git help git`.\n \ndiff --git a/git.c b/git.c\nindex 0f1937f..1c61151 100644\n--- a/git.c\n+++ b/git.c\n@@ -522,21 +522,34 @@ static void strip_extension(const char **argv)\n \n static void handle_builtin(int argc, const char **argv)\n {\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n \tconst char *cmd;\n \tstruct cmd_struct *builtin;\n \n \tstrip_extension(argv);\n \tcmd = argv[0];\n \n-\t/* Turn \"git cmd --help\" into \"git help cmd\" */\n+\t/* Turn \"git cmd --help\" into \"git help --exclude-guides cmd\" */\n \tif (argc > 1 && !strcmp(argv[1], \"--help\")) {\n+\t\tint i;\n+\n \t\targv[1] = argv[0];\n \t\targv[0] = cmd = \"help\";\n+\n+\t\tfor (i = 0; i < argc; i++) {\n+\t\t\targv_array_push(&args, argv[i]);\n+\t\t\tif (!i)\n+\t\t\t\targv_array_push(&args, \"--exclude-guides\");\n+\t\t}\n+\n+\t\targc++;\n+\t\targv = args.argv;\n \t}\n \n \tbuiltin = get_builtin(cmd);\n \tif (builtin)\n \t\texit(run_builtin(builtin, argc, argv));\n+\targv_array_clear(&args);\n }\n \n static void execv_dashed_external(const char **argv)\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nindex fb1abd7..2b90947 100755\n--- a/t/t0012-help.sh\n+++ b/t/t0012-help.sh\n@@ -30,4 +30,12 @@ test_expect_success \"--exclude-guides does not work for guides\" \"\n \ttest_i18ncmp expected actual\n \"\n \n+test_expect_success \"--help does not work for guides\" \"\n+\tcat <<-EOF >expected &&\n+\t\tgit: 'revisions' is not a git command. See 'git --help'.\n+\tEOF\n+\ttest_must_fail git revisions --help 2>actual &&\n+\ttest_i18ncmp expected actual\n+\"\n+\n test_done\n-- \n2.9.2.912.gd0c0e83\n\n"},{"id":"300280","messageId":"20160826175836.14073-3-ralf.thielow@gmail.com","threadId":"43792","inReplyTo":"20160826175836.14073-1-ralf.thielow@gmail.com","subject":"[PATCH v2 2/3] help: introduce option --exclude-guides","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-26T17:58:35Z","receivedAt":"2016-08-26T17:59:03Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"Introduce option --exclude-guides to the help command. With this option\nbeing passed, \"git help\" will open man pages only for actual commands.\n\nSince we know it is a command, we can use function help_unknown_command\nto give the user advice on typos.\n\nHelped-by: Johannes Schindelin \nSigned-off-by: Ralf Thielow \n---\nIn the test script we do two things I'd like to point out:\n\n> + test_config help.htmlpath test://html &&\n\nAs we pass a URL, Git won't check if the given path looks like\na documentation directory. Another solution would be to create\na directory, add a file \"git.html\" to it and just use this path.\n\n> + test_config help.browser firefox\n\nGit checks if the browser is known, so the \"test-browser\" needs to\npretend it is one of them.\n\n Documentation/git-help.txt | 6 +++++-\n builtin/help.c | 30 +++++++++++++++++++++++-------\n contrib/completion/git-completion.bash | 2 +-\n t/t0012-help.sh | 33 +++++++++++++++++++++++++++++++++\n 4 files changed, 62 insertions(+), 9 deletions(-)\n create mode 100755 t/t0012-help.sh\n\ndiff --git a/Documentation/git-help.txt b/Documentation/git-help.txt\nindex 40d328a..eeb1950 100644\n--- a/Documentation/git-help.txt\n+++ b/Documentation/git-help.txt\n@@ -8,7 +8,7 @@ git-help - Display help information about Git\n SYNOPSIS\n --------\n [verse]\n-'git help' [-a|--all] [-g|--guide]\n+'git help' [-a|--all] [-e|--exclude-guides] [-g|--guide]\n \t [-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]\n \n DESCRIPTION\n@@ -43,6 +43,10 @@ OPTIONS\n \tPrints all the available commands on the standard output. This\n \toption overrides any given command or guide name.\n \n+-e::\n+--exclude-guides::\n+\tDo not show help for guides.\n+\n -g::\n --guides::\n \tPrints a list of useful guides on the standard output. This\ndiff --git a/builtin/help.c b/builtin/help.c\nindex e8f79d7..40901a9 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -37,8 +37,10 @@ static int show_all = 0;\n static int show_guides = 0;\n static unsigned int colopts;\n static enum help_format help_format = HELP_FORMAT_NONE;\n+static int exclude_guides;\n static struct option builtin_help_options[] = {\n \tOPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n+\tOPT_BOOL('e', \"exclude-guides\", &exclude_guides, N_(\"exclude guides\")),\n \tOPT_BOOL('g', \"guides\", &show_guides, N_(\"print list of useful guides\")),\n \tOPT_SET_INT('m', \"man\", &help_format, N_(\"show man page\"), HELP_FORMAT_MAN),\n \tOPT_SET_INT('w', \"web\", &help_format, N_(\"show manual in web browser\"),\n@@ -426,10 +428,29 @@ static void list_common_guides_help(void)\n \tputchar('\\n');\n }\n \n+static const char *check_git_cmd(const char* cmd)\n+{\n+\tchar *alias;\n+\n+\tif (is_git_command(cmd))\n+\t\treturn cmd;\n+\n+\talias = alias_lookup(cmd);\n+\tif (alias) {\n+\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), cmd, alias);\n+\t\tfree(alias);\n+\t\texit(0);\n+\t}\n+\n+\tif (exclude_guides)\n+\t\treturn help_unknown_cmd(cmd);\n+\n+\treturn cmd;\n+}\n+\n int cmd_help(int argc, const char **argv, const char *prefix)\n {\n \tint nongit;\n-\tchar *alias;\n \tenum help_format parsed_help_format;\n \n \targc = parse_options(argc, argv, prefix, builtin_help_options,\n@@ -469,12 +490,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)\n \tif (help_format == HELP_FORMAT_NONE)\n \t\thelp_format = parse_help_format(DEFAULT_HELP_FORMAT);\n \n-\talias = alias_lookup(argv[0]);\n-\tif (alias && !is_git_command(argv[0])) {\n-\t\tprintf_ln(_(\"`git %s' is aliased to `%s'\"), argv[0], alias);\n-\t\tfree(alias);\n-\t\treturn 0;\n-\t}\n+\targv[0] = check_git_cmd(argv[0]);\n \n \tswitch (help_format) {\n \tcase HELP_FORMAT_NONE:\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c1b2135..b148164 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1393,7 +1393,7 @@ _git_help ()\n {\n \tcase \"$cur\" in\n \t--*)\n-\t\t__gitcomp \"--all --guides --info --man --web\"\n+\t\t__gitcomp \"--all --exclude-guides --guides --info --man --web\"\n \t\treturn\n \t\t;;\n \tesac\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nnew file mode 100755\nindex 0000000..fb1abd7\n--- /dev/null\n+++ b/t/t0012-help.sh\n@@ -0,0 +1,33 @@\n+#!/bin/sh\n+\n+test_description='help'\n+\n+. ./test-lib.sh\n+\n+configure_help () {\n+\ttest_config help.format html &&\n+\ttest_config help.htmlpath test://html &&\n+\ttest_config help.browser firefox \n+}\n+\n+test_expect_success \"setup\" \"\n+\twrite_script firefox <<-\\EOF\n+\texit 0\n+\tEOF\n+\"\n+\n+test_expect_success \"works for commands and guides by default\" \"\n+\tconfigure_help &&\n+\tgit help status &&\n+\tgit help revisions\n+\"\n+\n+test_expect_success \"--exclude-guides does not work for guides\" \"\n+\tcat <<-EOF >expected &&\n+\t\tgit: 'revisions' is not a git command. See 'git --help'.\n+\tEOF\n+\ttest_must_fail git help --exclude-guides revisions 2>actual &&\n+\ttest_i18ncmp expected actual\n+\"\n+\n+test_done\n-- \n2.9.2.912.gd0c0e83\n\n"},{"id":"300286","messageId":"xmqq8tvjgxiy.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"20160826175836.14073-3-ralf.thielow@gmail.com","subject":"Re: [PATCH v2 2/3] help: introduce option --exclude-guides","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-26T19:06:45Z","receivedAt":"2016-08-26T19:07:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow writes:\n\n> Introduce option --exclude-guides to the help command. With this option\n> being passed, \"git help\" will open man pages only for actual commands.\n\nLet's hide this option from command help of \"git help\" itself, drop\nthe short-and-sweet \"-e\", not command-line complete it, and leave it\nnot-mentioned here.\n\nIt is a different story if this option would really help the end\nusers, but I do not think that is the case. If this were to face\nthe end users properly, we would need to worry about making sure\nthat \"git help -g -e\" would error out, and getting all the other\npossible corner cases right. I do not think the amount of effort\nrequired to do so (even the \"trying to enumerate what other possible\ncorner cases there may be\" part) is worth it.\n\n> In the test script we do two things I'd like to point out:\n>\n>> + test_config help.htmlpath test://html &&\n>\n> As we pass a URL, Git won't check if the given path looks like\n> a documentation directory. Another solution would be to create\n> a directory, add a file \"git.html\" to it and just use this path.\n\nI think this is OK; with s|As we pass a URL|As we pass a string with\n:// in it|, the first sentence can be a in-code comment in the test\nthat does this and will help readers of the code in the future.\n\n>> + test_config help.browser firefox\n>\n> Git checks if the browser is known, so the \"test-browser\" needs to\n> pretend it is one of them.\n\nAre you talking about the hardcoded list in valid_tool() helper\nfunction in git-web--browse.sh? If we use the established escape\nhatch implemented by valid_custom_tool() helper there by setting\nbrowser.*.cmd, would that be sufficient to work around the \"Git\nchecks if the browser is known\"?\n\n> diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt\n> index 40d328a..eeb1950 100644\n> --- a/Documentation/git-help.txt\n> +++ b/Documentation/git-help.txt\n> @@ -8,7 +8,7 @@ git-help - Display help information about Git\n> SYNOPSIS\n> --------\n> [verse]\n> -'git help' [-a|--all] [-g|--guide]\n> +'git help' [-a|--all] [-e|--exclude-guides] [-g|--guide]\n> \t [-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]\n\nSo, let's not do this.\n\n> diff --git a/builtin/help.c b/builtin/help.c\n> index e8f79d7..40901a9 100644\n> --- a/builtin/help.c\n> +++ b/builtin/help.c\n> @@ -37,8 +37,10 @@ static int show_all = 0;\n> static int show_guides = 0;\n> static unsigned int colopts;\n> static enum help_format help_format = HELP_FORMAT_NONE;\n> +static int exclude_guides;\n> static struct option builtin_help_options[] = {\n> \tOPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n> +\tOPT_BOOL('e', \"exclude-guides\", &exclude_guides, N_(\"exclude guides\")),\n\nSo I'd suggest using PARSE_OPT_HIDDEN for this one and drop 'e' shorthand.\nThe only caller of this mode does not use it.\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index c1b2135..b148164 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1393,7 +1393,7 @@ _git_help ()\n> {\n> \tcase \"$cur\" in\n> \t--*)\n> -\t\t__gitcomp \"--all --guides --info --man --web\"\n> +\t\t__gitcomp \"--all --exclude-guides --guides --info --man --web\"\n> \t\treturn\n> \t\t;;\n> \tesac\n\nSo, let's not do this.\n\n> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n> new file mode 100755\n> index 0000000..fb1abd7\n> --- /dev/null\n> +++ b/t/t0012-help.sh\n> @@ -0,0 +1,33 @@\n> +#!/bin/sh\n> +\n> +test_description='help'\n> +\n> +. ./test-lib.sh\n> +\n> +configure_help () {\n> +\ttest_config help.format html &&\n> +\ttest_config help.htmlpath test://html &&\n> +\ttest_config help.browser firefox \n> +}\n\nWould replacing the last line with:\n\n\ttest_config browser.test.cmd ./test-browser &&\n\ttest_config help.browser test\n\nand then writing to test-browser work just as well? If so, that\nwould be much cleaner and more preferrable.\n\n> +\n> +test_expect_success \"setup\" \"\n> +\twrite_script firefox <<-\\EOF\n> +\texit 0\n> +\tEOF\n> +\"\n\nUnless there is a good reason you MUST do so, avoid quoting the test\nbody with double quotes, as it invites mistakes [*1*].\n\nAlso, how about using something like:\n\n\twrite_script test-browser <<-\\EOF\n\ti=0\n\tfor arg\n do\n\t\ti=$(( $i + 1 ))\n\t\techo \"$i: $arg\"\n\tdone >test-browser.log\n EOF\n\ninstead? That way, you can ensure that \"git help status\" attempts\nto call git-status.html with the expected path, not gitstatus.html\nor status.html, or somesuch, immediately after running \"git help\nstatus\" in the next test by inspecting test-browser.log ...\n\n> +test_expect_success \"works for commands and guides by default\" \"\n> +\tconfigure_help &&\n> +\tgit help status &&\n\n... right here.\n\nThe output from the test-browser does not have to be multi-line;\njust doing\n\n\techo \"$*\"\n\nmight be sufficient.\n\n> +\tgit help revisions\n> +\"\n\nThanks.\n\n[Footnote]\n\n*1* Can you immediately tell why this test is broken?\n\ntest_expect_success \"two commits do not have the same ID\" \"\n\tgit commit --allow-empty -m first &&\n\tone=$(git rev-parse --verify HEAD) &&\n\ttest_tick &&\n\tgit commit --allow-empty -m second &&\n\ttwo=$(git rev-parse --verify HEAD) &&\n\ttest $one != $two\n\"\n\n"},{"id":"300289","messageId":"xmqqwpj3fhaz.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"xmqq8tvjgxiy.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/3] help: introduce option --exclude-guides","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-26T19:42:28Z","receivedAt":"2016-08-26T19:44:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Let's hide this option from command help of \"git help\" itself, drop\n> the short-and-sweet \"-e\", not command-line complete it, and leave it\n> not-mentioned here.\n> ...\n> Unless there is a good reason you MUST do so, avoid quoting the test\n> body with double quotes, as it invites mistakes [*1*].\n>\n> Also, how about using something like:\n> ...\n> instead? That way, you can ensure that \"git help status\" attempts\n> to call git-status.html with the expected path, not gitstatus.html\n> or status.html, or somesuch, immediately after running \"git help\n> status\" in the next test by inspecting test-browser.log ...\n\nTaking all of these together, I'll queue this as a proposed fix-up\ndirectly on top of yours.\n\n Documentation/git-help.txt | 6 +-----\n builtin/help.c | 2 +-\n contrib/completion/git-completion.bash | 2 +-\n t/t0012-help.sh | 33 ++++++++++++++++++---------------\n 4 files changed, 21 insertions(+), 22 deletions(-)\n\ndiff --git a/Documentation/git-help.txt b/Documentation/git-help.txt\nindex eeb1950..40d328a 100644\n--- a/Documentation/git-help.txt\n+++ b/Documentation/git-help.txt\n@@ -8,7 +8,7 @@ git-help - Display help information about Git\n SYNOPSIS\n --------\n [verse]\n-'git help' [-a|--all] [-e|--exclude-guides] [-g|--guide]\n+'git help' [-a|--all] [-g|--guide]\n \t [-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]\n \n DESCRIPTION\n@@ -43,10 +43,6 @@ OPTIONS\n \tPrints all the available commands on the standard output. This\n \toption overrides any given command or guide name.\n \n--e::\n---exclude-guides::\n-\tDo not show help for guides.\n-\n -g::\n --guides::\n \tPrints a list of useful guides on the standard output. This\ndiff --git a/builtin/help.c b/builtin/help.c\nindex 40901a9..49f7a07 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -40,7 +40,7 @@ static enum help_format help_format = HELP_FORMAT_NONE;\n static int exclude_guides;\n static struct option builtin_help_options[] = {\n \tOPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n-\tOPT_BOOL('e', \"exclude-guides\", &exclude_guides, N_(\"exclude guides\")),\n+\tOPT_HIDDEN_BOOL(0, \"exclude-guides\", &exclude_guides, N_(\"exclude guides\")),\n \tOPT_BOOL('g', \"guides\", &show_guides, N_(\"print list of useful guides\")),\n \tOPT_SET_INT('m', \"man\", &help_format, N_(\"show man page\"), HELP_FORMAT_MAN),\n \tOPT_SET_INT('w', \"web\", &help_format, N_(\"show manual in web browser\"),\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 63cccb9..bd25b0a 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1340,7 +1340,7 @@ _git_help ()\n {\n \tcase \"$cur\" in\n \t--*)\n-\t\t__gitcomp \"--all --exclude-guides --guides --info --man --web\"\n+\t\t__gitcomp \"--all --guides --info --man --web\"\n \t\treturn\n \t\t;;\n \tesac\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nindex f91088b..9d99812 100755\n--- a/t/t0012-help.sh\n+++ b/t/t0012-help.sh\n@@ -7,27 +7,30 @@ test_description='help'\n configure_help () {\n \ttest_config help.format html &&\n \ttest_config help.htmlpath test://html &&\n-\ttest_config help.browser firefox\n+\ttest_config browser.test.cmd ./test-browser &&\n+\ttest_config help.browser test\n }\n \n-test_expect_success \"setup\" \"\n-\twrite_script firefox <<-\\EOF\n-\texit 0\n+test_expect_success \"setup\" '\n+\twrite_script test-browser <<-\\EOF\n+\techo \"$*\" >test-browser.log\n \tEOF\n-\"\n+'\n \n-test_expect_success \"works for commands and guides by default\" \"\n+test_expect_success \"works for commands and guides by default\" '\n \tconfigure_help &&\n \tgit help status &&\n-\tgit help revisions\n-\"\n+\techo \"test://html/git-status.html\" >expect &&\n+\ttest_cmp expect test-browser.log &&\n+\tgit help revisions &&\n+\techo \"test://html/gitrevisions.html\" >expect &&\n+\ttest_cmp expect test-browser.log\n+'\n \n-test_expect_success \"--exclude-guides does not work for guides\" \"\n-\tcat <<-EOF >expected &&\n-\t\tgit: 'revisions' is not a git command. See 'git --help'.\n-\tEOF\n-\ttest_must_fail git help --exclude-guides revisions 2>actual &&\n-\ttest_i18ncmp expected actual\n-\"\n+test_expect_success \"--exclude-guides does not work for guides\" '\n+\t>test-browser.log &&\n+\ttest_must_fail git help --exclude-guides revisions &&\n+\ttest_must_be_empty test-browser.log\n+'\n \n test_done\n-- \n2.10.0-rc1-260-gbdd1a2a\n\n"},{"id":"300293","messageId":"CAN0XMOKo0VXPZF8ve2e1N5f591Kkz-Gmxt4wJKsev2zj4ubj9w@mail.gmail.com","threadId":"43792","inReplyTo":"xmqq8tvjgxiy.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/3] help: introduce option --exclude-guides","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-26T20:00:48Z","receivedAt":"2016-08-26T20:01:09Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"2016-08-26 21:06 GMT+02:00 Junio C Hamano :\n> Ralf Thielow writes:\n>\n>> Introduce option --exclude-guides to the help command. With this option\n>> being passed, \"git help\" will open man pages only for actual commands.\n>\n> Let's hide this option from command help of \"git help\" itself, drop\n> the short-and-sweet \"-e\", not command-line complete it, and leave it\n> not-mentioned here.\n>\n> It is a different story if this option would really help the end\n> users, but I do not think that is the case. If this were to face\n> the end users properly, we would need to worry about making sure\n> that \"git help -g -e\" would error out, and getting all the other\n> possible corner cases right. I do not think the amount of effort\n> required to do so (even the \"trying to enumerate what other possible\n> corner cases there may be\" part) is worth it.\n>\n\nI'm fine with that as the reason for me was just a \"why not?\", and\nyou just gave the reason to not do this. Thanks\n\n>> In the test script we do two things I'd like to point out:\n>>\n>>> + test_config help.htmlpath test://html &&\n>>\n>> As we pass a URL, Git won't check if the given path looks like\n>> a documentation directory. Another solution would be to create\n>> a directory, add a file \"git.html\" to it and just use this path.\n>\n> I think this is OK; with s|As we pass a URL|As we pass a string with\n> :// in it|, the first sentence can be a in-code comment in the test\n> that does this and will help readers of the code in the future.\n>\n\nHmm. The \"://\" is really a URL thing. That's why it's in the code, no?\nThe code may have some room for improvements in checking for\nURLs.\n\n>>> + test_config help.browser firefox\n>>\n>> Git checks if the browser is known, so the \"test-browser\" needs to\n>> pretend it is one of them.\n>\n> Are you talking about the hardcoded list in valid_tool() helper\n> function in git-web--browse.sh? If we use the established escape\n> hatch implemented by valid_custom_tool() helper there by setting\n> browser.*.cmd, would that be sufficient to work around the \"Git\n> checks if the browser is known\"?\n>\n>> diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt\n>> index 40d328a..eeb1950 100644\n>> --- a/Documentation/git-help.txt\n>> +++ b/Documentation/git-help.txt\n>> @@ -8,7 +8,7 @@ git-help - Display help information about Git\n>> SYNOPSIS\n>> --------\n>> [verse]\n>> -'git help' [-a|--all] [-g|--guide]\n>> +'git help' [-a|--all] [-e|--exclude-guides] [-g|--guide]\n>> [-i|--info|-m|--man|-w|--web] [COMMAND|GUIDE]\n>\n> So, let's not do this.\n>\n>> diff --git a/builtin/help.c b/builtin/help.c\n>> index e8f79d7..40901a9 100644\n>> --- a/builtin/help.c\n>> +++ b/builtin/help.c\n>> @@ -37,8 +37,10 @@ static int show_all = 0;\n>> static int show_guides = 0;\n>> static unsigned int colopts;\n>> static enum help_format help_format = HELP_FORMAT_NONE;\n>> +static int exclude_guides;\n>> static struct option builtin_help_options[] = {\n>> OPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n>> + OPT_BOOL('e', \"exclude-guides\", &exclude_guides, N_(\"exclude guides\")),\n>\n> So I'd suggest using PARSE_OPT_HIDDEN for this one and drop 'e' shorthand.\n> The only caller of this mode does not use it.\n>\n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index c1b2135..b148164 100644\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -1393,7 +1393,7 @@ _git_help ()\n>> {\n>> case \"$cur\" in\n>> --*)\n>> - __gitcomp \"--all --guides --info --man --web\"\n>> + __gitcomp \"--all --exclude-guides --guides --info --man --web\"\n>> return\n>> ;;\n>> esac\n>\n> So, let's not do this.\n>\n>> diff --git a/t/t0012-help.sh b/t/t0012-help.sh\n>> new file mode 100755\n>> index 0000000..fb1abd7\n>> --- /dev/null\n>> +++ b/t/t0012-help.sh\n>> @@ -0,0 +1,33 @@\n>> +#!/bin/sh\n>> +\n>> +test_description='help'\n>> +\n>> +. ./test-lib.sh\n>> +\n>> +configure_help () {\n>> + test_config help.format html &&\n>> + test_config help.htmlpath test://html &&\n>> + test_config help.browser firefox\n>> +}\n>\n> Would replacing the last line with:\n>\n> test_config browser.test.cmd ./test-browser &&\n> test_config help.browser test\n>\n> and then writing to test-browser work just as well? If so, that\n> would be much cleaner and more preferrable.\n>\n\nI wasn't aware that this is a way to configure things. Thanks.\n\n>> +\n>> +test_expect_success \"setup\" \"\n>> + write_script firefox <<-\\EOF\n>> + exit 0\n>> + EOF\n>> +\"\n>\n> Unless there is a good reason you MUST do so, avoid quoting the test\n> body with double quotes, as it invites mistakes [*1*].\n>\n\nThe test-browser was supposed to be returning just a success\nwhich is good enough for my usage.\n\n> Also, how about using something like:\n>\n> write_script test-browser <<-\\EOF\n> i=0\n> for arg\n> do\n> i=$(( $i + 1 ))\n> echo \"$i: $arg\"\n> done >test-browser.log\n> EOF\n>\n> instead? That way, you can ensure that \"git help status\" attempts\n> to call git-status.html with the expected path, not gitstatus.html\n> or status.html, or somesuch, immediately after running \"git help\n> status\" in the next test by inspecting test-browser.log ...\n>\n\nWe can use this to check whether the correct file was tried to open.\nNot a part of this \"does it work\" test, but good for new ones.\n\n>> +test_expect_success \"works for commands and guides by default\" \"\n>> + configure_help &&\n>> + git help status &&\n>\n> ... right here.\n>\n> The output from the test-browser does not have to be multi-line;\n> just doing\n>\n> echo \"$*\"\n>\n> might be sufficient.\n>\n>> + git help revisions\n>> +\"\n>\n> Thanks.\n>\n> [Footnote]\n>\n> *1* Can you immediately tell why this test is broken?\n>\n> test_expect_success \"two commits do not have the same ID\" \"\n> git commit --allow-empty -m first &&\n> one=$(git rev-parse --verify HEAD) &&\n> test_tick &&\n> git commit --allow-empty -m second &&\n> two=$(git rev-parse --verify HEAD) &&\n> test $one != $two\n> \"\n>\n\nI'm afraid I can't.\n"},{"id":"300295","messageId":"CAN0XMOJC2uqodOUDocf7o2Fi=aCvmaf_cWGYrbp1FbkjEji3cg@mail.gmail.com","threadId":"43792","inReplyTo":"xmqqwpj3fhaz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/3] help: introduce option --exclude-guides","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-26T20:03:46Z","receivedAt":"2016-08-26T20:09:36Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"2016-08-26 21:42 GMT+02:00 Junio C Hamano :\n> Junio C Hamano writes:\n>\n>\n> Taking all of these together, I'll queue this as a proposed fix-up\n> directly on top of yours.\n>\n\nThanks!\n"},{"id":"300297","messageId":"xmqqfuprffiu.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"CAN0XMOKo0VXPZF8ve2e1N5f591Kkz-Gmxt4wJKsev2zj4ubj9w@mail.gmail.com","subject":"Re: [PATCH v2 2/3] help: introduce option --exclude-guides","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-26T20:20:57Z","receivedAt":"2016-08-26T20:24:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow writes:\n\n>>> As we pass a URL, Git won't check if the given path looks like\n>>> a documentation directory. Another solution would be to create\n>>> a directory, add a file \"git.html\" to it and just use this path.\n>>\n>> I think this is OK; with s|As we pass a URL|As we pass a string with\n>> :// in it|, the first sentence can be a in-code comment in the test\n>> that does this and will help readers of the code in the future.\n>\n> Hmm. The \"://\" is really a URL thing.\n\nPerhaps you thought so, but no, \"mailto:ralf.thielow@gmail.com\" is a\nperfectly valid URL.\n\nBecause you are explaining why test://html was chosen, and the real\nreason is any path that is !strstr(path, \"://\") is subject to an\nadditional \"This must be a local path\" check and you wanted to avoid\nit, \"As we pass a URL\" is unnecessarily vague (and incorrect--we\ncannot use a mailto: URL to sidestep the check).\n\n>> *1* Can you immediately tell why this test is broken?\n>>\n>> test_expect_success \"two commits do not have the same ID\" \"\n>> git commit --allow-empty -m first &&\n>> one=$(git rev-parse --verify HEAD) &&\n>> test_tick &&\n>> git commit --allow-empty -m second &&\n>> two=$(git rev-parse --verify HEAD) &&\n>> test $one != $two\n>> \"\n>>\n>\n> I'm afraid I can't.\n\nThe reason becomes clear if you put your feet into shell's shues.\nBefore being ablt to call test_expect_success, you would need to\nfigure out what strings you give as its parameters. $1 is clear in\nthis case, a simple string \"two commits do not have the same ID\"\n(without double quotes).\n\nBut what goes in $2? Especially the part around \"one=...\"?\n\nBecause the whole thing is inside a double-quote pair, $() and $name\nare all interpolated even before test_expect_success is called.\nSo the above becomes equivalent to\n\n>> test_expect_success \"two commits do not have the same ID\" '\n>> git commit --allow-empty -m first &&\n>> one=5cb0d5ad05e027cbddcb0a3c7518ddeea0f7c286 &&\n>> test_tick &&\n>> git commit --allow-empty -m second &&\n>> two=5cb0d5ad05e027cbddcb0a3c7518ddeea0f7c286 &&\n>> test !=\n>> '\n\n(using whatever commit HEAD was pointing at before this test starts\nto run), which obviously is not what we expected to see.\n\n"},{"id":"300301","messageId":"xmqqbn0fff6d.fsf@gitster.mtv.corp.google.com","threadId":"43792","inReplyTo":"CAN0XMOJC2uqodOUDocf7o2Fi=aCvmaf_cWGYrbp1FbkjEji3cg@mail.gmail.com","subject":"Re: [PATCH v2 2/3] help: introduce option --exclude-guides","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-26T20:28:26Z","receivedAt":"2016-08-26T20:30:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow writes:\n\n> 2016-08-26 21:42 GMT+02:00 Junio C Hamano :\n>> Junio C Hamano writes:\n>>\n>>\n>> Taking all of these together, I'll queue this as a proposed fix-up\n>> directly on top of yours.\n>>\n>\n> Thanks!\n\nThank you for starting this topic. I forgot to add comment on that\ntest://html part, though, so I'd have to redo it further, but\nperhaps not today.\n"},{"id":"300302","messageId":"CAN0XMO+1o0fHNeemN0JAHnRZY4O1xh1xkSQHAPPdEGJPxiMkBQ@mail.gmail.com","threadId":"43792","inReplyTo":"xmqqfuprffiu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/3] help: introduce option --exclude-guides","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2016-08-26T20:39:50Z","receivedAt":"2016-08-26T20:39:57Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"2016-08-26 22:20 GMT+02:00 Junio C Hamano :\n>\n> Because the whole thing is inside a double-quote pair, $() and $name\n> are all interpolated even before test_expect_success is called.\n> So the above becomes equivalent to\n>\n>>> test_expect_success \"two commits do not have the same ID\" '\n>>> git commit --allow-empty -m first &&\n>>> one=5cb0d5ad05e027cbddcb0a3c7518ddeea0f7c286 &&\n>>> test_tick &&\n>>> git commit --allow-empty -m second &&\n>>> two=5cb0d5ad05e027cbddcb0a3c7518ddeea0f7c286 &&\n>>> test !=\n>>> '\n>\n\nI got it, thanks. My understanding in when a part is being interpreted\nwas obviously very wrong. Thanks again!\n"}]}