Subject: [PATCH v2 0/2] documentation: handle non-existing html pages and document 'git version'

From: "Matthias Aßhauer via GitGitGadget" <[email-masked]>
Date: Tue, 14 Sep 2021 13:27:16 +0000

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 <[email-masked]>
      
     @@ 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 <[email-masked]>
      
       ## 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