Skip to content

documentation: handle non-existing html pages and document 'git version'

Matthias Aßhauer via GitGitGadget git 2 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest2 patches 2 repliesIn review
  2. v12 patches 6 repliesSuperseded

Changes in v2 from the cover letter

* 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)

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2help: make sure local html page exists before calling external processesbuiltin/help.c t/t0012-help.sh+22 -30 replies
2/2documentation: add documentation for 'git version'Documentation/git-version.txt Documentation/git.txt+32 -02 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.

Read the v2 thread

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

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.

@@ 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.
+

## 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

Signature · 1 line
gitgitgadget