Re: [PATCH 1/2] help.c: Add support for htmldir relative to git_exec_path()

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 4, 2008, 09:26 UTC
Message-ID: <7vk5g2uhi9.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1215162566-16062-1-git-send-email-prohaska@zib.de>

Steffen Prohaska <prohaska@zib.de> writes:

Show 7 quoted lines

> If htmldir (in the Makefile) is a relative path, this path will be
> interpreted relative to git_exec_path.  This can be used to create an
> installation that can be moved to a different directory without
> re-compiling.  The Windows installer (msysgit) is an example for such
> a setup.
> ...
> +	const char* html_path = GIT_HTML_PATH;

Style.  Asterisk sticks to the variable, not type.

Show 5 quoted lines

> +	if (!is_absolute_path(html_path)) {
> +		struct strbuf d = STRBUF_INIT;
> +		strbuf_addf(&d, "%s/%s", git_exec_path(), html_path);
> +		html_path = strbuf_detach(&d, NULL);
> +	}

I've seen similar "if $this (which is usually an absolute) is relative, it is taken as relative to git_exec_path" solution employed elsewhere in the MinGW series, and I think it makes sense, even though initially I thought it was somewhat hacky.

Could you check if there are copy-and-pasted duplicated code you can factor out before continuing this direction? I suspect templates and etc/gitconfig are specified in similar fashion, and it would probably be easier to maintain if you define once:

	char *system_path(const char *specified)
        {
        	if (is_absolute_path(specified))
	                return specified;
		... strbuf dance ...
                return strbuf_detach(...);
        }

and use it like this:

	const char *html_path = system_path(GIT_HTML_PATH);