Re: [PATCH] Add the DEFAULT_HELP_FORMAT option to Makefile

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 6, 2012, 19:06 UTC
Message-ID: <7vzk8gl0p0.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1339008702-704-1-git-send-email-vfr@lyx.org>

vfr@lyx.org writes:

Show 22 quoted lines

> From: Vincent van Ravesteijn <vfr@lyx.org>
>
> This patch advertises the DEFAULT_HELP_FORMAT compile option in Makefile. It will also allow to call 'make DEFAULT_HELP_FORMAT=info' to specify a different default.
>
> Proposed-by: Jeff King <peff@peff.net>
> Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
> ---
>  Makefile |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index ffbd7a4..b21afed 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -296,6 +296,10 @@ all::
>  # the diff algorithm.  It gives a nice speedup if your processor has
>  # fast unaligned word loads.  Does NOT work on big-endian systems!
>  # Enabled by default on x86_64.
> +#
> +# Define DEFAULT_HELP_FORMAT to "man", "info", "web" or "html"
> +# (defaults "man") if you want to have a different default when
> +# "git help" is called without a parameter specifying the format.

We probably should just say 'html' without the idiotic 'web' synonym.

Show 18 quoted lines

> @@ -1238,6 +1242,7 @@ ifeq ($(uname_S),Windows)
>  	NATIVE_CRLF = YesPlease
>  	NO_INET_PTON = YesPlease
>  	NO_INET_NTOP = YesPlease
> +	DEFAULT_HELP_FORMAT = html
>  
>  	CC = compat/vcbuild/scripts/clink.pl
>  	AR = compat/vcbuild/scripts/lib.pl
> @@ -1917,6 +1922,10 @@ SHELL_PATH_CQ_SQ = $(subst ','\'',$(SHELL_PATH_CQ))
>  BASIC_CFLAGS += -DSHELL_PATH='$(SHELL_PATH_CQ_SQ)'
>  endif
>  
> +ifdef DEFAULT_HELP_FORMAT
> +BASIC_CFLAGS += -DDEFAULT_HELP_FORMAT='"$(DEFAULT_HELP_FORMAT)"'
> +endif
> +
>  ALL_CFLAGS += $(BASIC_CFLAGS)
>  ALL_LDFLAGS += $(BASIC_LDFLAGS)

The choice of the variable name, decision to tweak BASIC_CFLAGS and the placement of the tweak all look good to me.

You would also need actual code to react to -DDEFAULT_HELP_FORMAT in the same patch.

I think the choices of PAGER and EDITOR share the same problem, but shouldn't this choice recorded in GIT-BUILD-OPTIONS in some way?

Thanks.