These two patches are grouped as one patch series, because they arose from the same Git for Windows issue [1], but they can be reviewed or applied independent from one another. One interesting oddity I found while preparing V2: git --version --help gets converted to git version --help which then calls git help version, but git --help --version gets converted to git help --version and git help doesn't know what to do with --version. [1] https://github.com/git-for-windows/git/issues/3308 Changes since V1: * fixed various typos in the log message of patch 1 * changed patch 1 to just check the requested page instead of both the requested page and git.html * moved the test up before the "generate builtin list" test * reworked the test slightly * added a second test * removed the redundant description of --build-options from the DESCRIPTION section * added a small paragraph to Documentation/git.txt that links to the new git version page (like we already do for git help) Matthias Aßhauer (2): help: make sure local html page exists before calling external processes documentation: add documentation for 'git version' Documentation/git-version.txt | 28 ++++++++++++++++++++++++++++ Documentation/git.txt | 4 ++++ builtin/help.c | 9 ++++++--- t/t0012-help.sh | 16 ++++++++++++++++ 4 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 Documentation/git-version.txt base-commit: 8463beaeb69fe0b7f651065813def4aa6827cd5d Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1038%2Frimrul%2Fdoc-version-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1038/rimrul/doc-version-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1038 Range-diff vs v1: 1: 8674d67a439 ! 1: c55360272d5 help: make sure local html page exists before calling external processes @@ Metadata ## Commit message ## help: make sure local html page exists before calling external processes - We already check that git.html exists, regardless of the page the user wants - to open. Additionally checking wether the requested page exists gives us a - smoother user experience when it doesn't. + We check that git.html exists, regardless of the page the user wants to open. + Checking whether the requested page exists instead gives us a smoother user + experience in two use cases: + + 1) The requested page doesn't exist When calling a git command and there is an error, most users reasonably expect git to produce an error message on the standard error stream, but in this case - we pass the filepath to git web--browse wich passes it on to a browser (or a - helper programm like xdg-open or start that should in turn open a browser) + we pass the filepath to git web--browse which passes it on to a browser (or a + helper program like xdg-open or start that should in turn open a browser) without any error and many GUI based browsers or helpers won't output such a message onto the standard error stream. - Especialy the helper programs tend to show the corresponding error message in + Especially the helper programs tend to show the corresponding error message in a message box and wait for user input before exiting. This leaves users in interactive console sessions without an error message in their console, without a console prompt and without the help page they expected. - The performance cost of the additional stat should be negliggible compared to - the two or more pocesses that we spawn after the checks. + 2) git.html is missing for some reason, but the user asked for some other page + + We currently refuse to show any local html help page when we can't find + git.html. Even if the requested help page exists. If we check for the requested + page instead, we can show the user all available pages and only error out on + those that don't exist. Signed-off-by: Matthias Aßhauer <mha1993@xxxxxxx> @@ builtin/help.c: static void get_html_page_path(struct strbuf *page_path, const c - /* Check that we have a git documentation directory. */ + /* -+ * Check that we have a git documentation directory and the page we're -+ * looking for exists. ++ * Check that the page we're looking for exists. + */ if (!strstr(html_path, "://")) { - if (stat(mkpath("%s/git.html", html_path), &st) - || !S_ISREG(st.st_mode)) - die("'%s': not a documentation directory.", html_path); +- if (stat(mkpath("%s/git.html", html_path), &st) + if (stat(mkpath("%s/%s.html", html_path, page), &st) -+ || !S_ISREG(st.st_mode)) + || !S_ISREG(st.st_mode)) +- die("'%s': not a documentation directory.", html_path); + die("'%s/%s.html': documentation file not found.", + html_path, page); } @@ builtin/help.c: static void get_html_page_path(struct strbuf *page_path, const c strbuf_init(page_path, 0); ## t/t0012-help.sh ## -@@ t/t0012-help.sh: test_expect_success 'generate builtin list' ' - git --list-cmds=builtins >builtins +@@ t/t0012-help.sh: test_expect_success 'git help -g' ' + test_i18ngrep "^ tutorial " help.output ' +test_expect_success 'git help fails for non-existing html pages' ' + configure_help && -+ mkdir html-doc && -+ touch html-doc/git.html && -+ test_must_fail git -c help.htmlpath=html-doc help status ++ mkdir html-empty && ++ test_must_fail git -c help.htmlpath=html-empty help status && ++ test_must_be_empty test-browser.log +' + - while read builtin - do - test_expect_success "$builtin can handle -h" ' ++test_expect_success 'git help succeeds without git.html' ' ++ configure_help && ++ mkdir html-with-docs && ++ touch html-with-docs/git-status.html && ++ git -c help.htmlpath=html-with-docs help status && ++ echo "html-with-docs/git-status.html" >expect && ++ test_cmp expect test-browser.log ++' ++ + test_expect_success 'generate builtin list' ' + git --list-cmds=builtins >builtins + ' 2: d3635cbfd6e ! 2: bc9a4534f5b documentation: add documentation for 'git version' @@ Commit message it is a non-experimental user-facing builtin command. As such it should have a help page. + Both `git help` and `git version` can be called as options + (`--help`/`--version`) that internally get converted to the + corresponding command. Add a small paragraph to + Documentation/git.txt describing how these two options + interact with each other and link to this help page for the + sub-options that `--version` can take. Well, currently there + is only one sub-option, but that could potentially increase + in future versions of Git. + Signed-off-by: Matthias Aßhauer <mha1993@xxxxxxx> ## Documentation/git-version.txt (new) ## @@ Documentation/git-version.txt (new) +---- +git-version - Display version information about Git + -+ +SYNOPSIS +-------- +[verse] +'git version' [--build-options] + -+ +DESCRIPTION +----------- -+ -+With no options given, the version of 'git' is printed -+on the standard output. -+ -+If the option `--build-options` is given, information about how git was built is -+printed on the standard output in addition to the version number. ++With no options given, the version of 'git' is printed on the standard output. + +Note that `git --version` is identical to `git version` because the +former is internally converted into the latter. @@ Documentation/git-version.txt (new) +OPTIONS +------- +--build-options:: -+ Prints out additional information about how git was built for diagnostic ++ Include additional information about how git was built for diagnostic + purposes. + +GIT +--- +Part of the linkgit:git[1] suite + + ## Documentation/git.txt ## +@@ Documentation/git.txt: OPTIONS + ------- + --version:: + Prints the Git suite version that the 'git' program came from. +++ ++This option is internaly converted to `git version ...` and accepts ++the same options as the linkgit:git-version[1] command. If `--help` is ++also given, it takes precedence over `--version`. + + --help:: + Prints the synopsis and a list of the most commonly used -- gitgitgadget
