git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Matthias Aßhauer via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Matthias Aßhauer" <mha1993@live.de>,
	"Matthias Aßhauer" <mha1993@live.de>
Subject: [PATCH 1/2] help: make sure local html page exists before calling external processes
Date: Mon, 13 Sep 2021 11:06:57 +0000	[thread overview]
Message-ID: <8674d67a439a23425133fa005e519ebb6ac19c42.1631531219.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1038.git.1631531218.gitgitgadget@gmail.com>

From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de>

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.

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

Signed-off-by: Matthias Aßhauer <mha1993@live.de>
---
 builtin/help.c  | 9 ++++++++-
 t/t0012-help.sh | 7 +++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/builtin/help.c b/builtin/help.c
index b7eec06c3de..77b1b926f60 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -467,11 +467,18 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)
 	if (!html_path)
 		html_path = to_free = system_path(GIT_HTML_PATH);
 
-	/* Check that we have a git documentation directory. */
+	/*
+	 * Check that we have a git documentation directory and 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/%s.html", html_path, page), &st)
+		    || !S_ISREG(st.st_mode))
+			die("'%s/%s.html': documentation file not found.",
+				html_path, page);
 	}
 
 	strbuf_init(page_path, 0);
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 5679e29c624..a83a59d44d9 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -77,6 +77,13 @@ test_expect_success 'generate builtin list' '
 	git --list-cmds=builtins >builtins
 '
 
+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
+'
+
 while read builtin
 do
 	test_expect_success "$builtin can handle -h" '
-- 
gitgitgadget


  reply	other threads:[~2021-09-13 11:07 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 ` Matthias Aßhauer via GitGitGadget [this message]
2021-09-13 15:59   ` [PATCH 1/2] help: make sure local html page exists before calling external processes 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 ` [PATCH v2 0/2] documentation: handle non-existing html pages and document " Matthias Aßhauer via GitGitGadget
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=8674d67a439a23425133fa005e519ebb6ac19c42.1631531219.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=mha1993@live.de \
    /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).