Subject: Re: [PATCH 05/12] Windows(msysgit): Per default, display help as HTML in default browser

From: Junio C Hamano <[email-masked]>
Date: Wed, 02 Jul 2008 02:11:44 -0700

Steffen Prohaska <[email-masked]> 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?

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