Re: [PATCH 05/12] Windows(msysgit): Per default, display help as HTML in default browser
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:52
Steffen Prohaska [off-list ref] writes:
+/* Convert slashes to backslashes on Windows. */ +char *make_native_separator(char *path);
Makes one onder why it is not inside #ifdef, but presumably it is no-op on other platforms (which is fine, better than fine)?
static int show_all = 0; +#ifdef __MINGW32__ +static enum help_format help_format = HELP_FORMAT_WEB; +#else static enum help_format help_format = HELP_FORMAT_MAN; +#endif
That's Ugly isn't it? Can't you do this with Makefile macro without #ifdef please?
quoted hunk
@@ -644,12 +649,35 @@ static void get_html_page_path(struct strbuf *page_path, const char *page) static void show_html_page(const char *git_cmd) { +#ifdef __MINGW32__ + const char* exec_path = git_exec_path(); + char *htmlpath = make_native_separator( + mkpath("%s/../doc/git/html/%s.html" + , exec_path + , git_cmd) + ); + if (!file_exists(htmlpath)) { + htmlpath = make_native_separator( + mkpath("%s/../doc/git/html/git-%s.html" + , exec_path + , git_cmd) + ); + if (!file_exists(htmlpath)) { + fprintf(stderr, "Can't find HTML help for '%s'.\n" + , git_cmd); + exit(1); + } + } + printf("Launching default browser to display HTML help ...\n"); + ShellExecute(NULL, "open", htmlpath, NULL, "\\", 0); +#else const char *page = cmd_to_page(git_cmd); struct strbuf page_path; /* it leaks but we exec bellow */ get_html_page_path(&page_path, page); execl_git_cmd("web--browse", "-c", "help.browser", page_path.buf, NULL); +#endif }
Hmm. The above almost makes me barf and suggest making them two totally separate functions (i.e. introduce a new "show_html_page_on_windows()" function and do not bother us Unix folks ;-). But I suspect your code is not beyond salvaging. Why is the htmlpath computed by hand in this function, instead of having the port specific implementation hidden inside get_html_page_path() function? About the execution part, isn't it the matter of using "open" (whatever that is) as one of the supported backend for web--browse to unify these two independent case arms?