From: "Matthias Aßhauer via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Eric Sunshine" <sunshine@sunshineco.com>,
"Matthias Aßhauer" <mha1993@live.de>,
"Junio C Hamano" <gitster@pobox.com>,
"Matthias Aßhauer" <mha1993@live.de>
Subject: [PATCH v2 0/2] documentation: handle non-existing html pages and document 'git version'
Date: Tue, 14 Sep 2021 13:27:16 +0000 [thread overview]
Message-ID: <pull.1038.v2.git.1631626038.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1038.git.1631531218.gitgitgadget@gmail.com>
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@live.de>
@@ 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@live.de>
## 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
next prev parent reply other threads:[~2021-09-14 13:27 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-13 11:06 [PATCH 0/2] documentation: handle non-existing html pages and document 'git version' Matthias Aßhauer via GitGitGadget
2021-09-13 11:06 ` [PATCH 1/2] help: make sure local html page exists before calling external processes Matthias Aßhauer via GitGitGadget
2021-09-13 15:59 ` Eric Sunshine
2021-09-13 16:17 ` Matthias Aßhauer
2021-09-13 19:25 ` Junio C Hamano
2021-09-13 11:06 ` [PATCH 2/2] documentation: add documentation for 'git version' Matthias Aßhauer via GitGitGadget
2021-09-13 11:19 ` Ævar Arnfjörð Bjarmason
2021-09-13 11:46 ` Matthias Aßhauer
2021-09-13 19:43 ` Junio C Hamano
2021-09-14 13:27 ` Matthias Aßhauer via GitGitGadget [this message]
2021-09-14 13:27 ` [PATCH v2 1/2] help: make sure local html page exists before calling external processes Matthias Aßhauer via GitGitGadget
2021-09-14 13:27 ` [PATCH v2 2/2] documentation: add documentation for 'git version' Matthias Aßhauer via GitGitGadget
2021-09-24 13:00 ` Is "make check-docs" useful anymore? Ævar Arnfjörð Bjarmason
2021-09-24 17:59 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1038.v2.git.1631626038.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mha1993@live.de \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).