[PATCH 1/2] help: make sure local html page exists before calling external processes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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@xxxxxxx>
---
 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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux