@matheusbsbs / matheus-bagliano / commits / a3952f8e7c

help: make sure local html page exists before calling external processes

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 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. 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. 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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Matthias Aßhauer committed Sep 14, 2021 at 13:27 UTC a3952f8e7c0e74d9266bc9cf4ac50dd179129f72

2 files changed +22 -3

builtin/help.c

+6 -3

@@ -467,11 +467,14 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)

467 if (!html_path)

468 html_path = to_free = system_path(GIT_HTML_PATH);

469

470 - /* Check that we have a git documentation directory. */

470 + /*

471 + * Check that the page we're looking for exists.

472 + */

473 if (!strstr(html_path, "://")) {

472 - if (stat(mkpath("%s/git.html", html_path), &st)

474 + if (stat(mkpath("%s/%s.html", html_path, page), &st)

475 || !S_ISREG(st.st_mode))

474 - die("'%s': not a documentation directory.", html_path);

476 + die("'%s/%s.html': documentation file not found.",

477 + html_path, page);

478 }

479

480 strbuf_init(page_path, 0);

t/t0012-help.sh

+16

@@ -73,6 +73,22 @@ test_expect_success 'git help -g' '

73 test_i18ngrep "^ tutorial " help.output

74 '

76 +test_expect_success 'git help fails for non-existing html pages' '

77 + configure_help &&

78 + mkdir html-empty &&

79 + test_must_fail git -c help.htmlpath=html-empty help status &&

80 + test_must_be_empty test-browser.log

81 +'

82 +

83 +test_expect_success 'git help succeeds without git.html' '

84 + configure_help &&

85 + mkdir html-with-docs &&

86 + touch html-with-docs/git-status.html &&

87 + git -c help.htmlpath=html-with-docs help status &&

88 + echo "html-with-docs/git-status.html" >expect &&

89 + test_cmp expect test-browser.log

90 +'

91 +

92 test_expect_success 'generate builtin list' '

93 git --list-cmds=builtins >builtins

94 '