[PATCH] Add the possibility to specify a default help format

[PATCH] Add the possibility to specify a default help format

[Vincent van Ravesteijn]

At the moment, the default help format (i.e. the format that is chosen if
'git help xxx' is called without a help format parameter) is defined by
the switch to be 'man'. However, on different platforms the preferred
format might differ. For example, on Windows there is no man viewer, so we
would prefer html.

This patch adds the possibility to choose a default help format on
compilation by defining DEFAULT_HELP_FORMAT. If it is not specified the
default is still 'man'.

Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
---
 builtin/help.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/builtin/help.c b/builtin/help.c
index 43d3c84..536d4fd 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -12,6 +12,10 @@
 #include "column.h"
 #include "help.h"
 
+#ifndef DEFAULT_HELP_FORMAT
+#define DEFAULT_HELP_FORMAT "man"
+#endif
+
 static struct man_viewer_list {
 	struct man_viewer_list *next;
 	char name[FLEX_ARRAY];
@@ -445,7 +449,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	setup_git_directory_gently(&nongit);
 	git_config(git_help_config, NULL);
 
-	if (parsed_help_format != HELP_FORMAT_NONE)
+	if (parsed_help_format == HELP_FORMAT_NONE)
+		help_format = parse_help_format(DEFAULT_HELP_FORMAT);
+	else
 		help_format = parsed_help_format;
 
 	alias = alias_lookup(argv[0]);
-- 
1.7.9.msysgit.0

Re: [PATCH] Add the possibility to specify a default help format

[Jeff King]

On Wed, Jun 06, 2012 at 12:56:37PM +0000, Vincent van Ravesteijn wrote:

> At the moment, the default help format (i.e. the format that is chosen if
> 'git help xxx' is called without a help format parameter) is defined by
> the switch to be 'man'. However, on different platforms the preferred
> format might differ. For example, on Windows there is no man viewer, so we
> would prefer html.
> 
> This patch adds the possibility to choose a default help format on
> compilation by defining DEFAULT_HELP_FORMAT. If it is not specified the
> default is still 'man'.

Makes sense to me.

> ---
>  builtin/help.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)

Should there be some minor infrastructure in the Makefile so you can do:

  make DEFAULT_HELP_FORMAT=man

rather than:

  make CFLAGS='-DDEFAULT_HELP_FORMAT=\"man\"'

(and the Makefile would be a good place to advertise this build knob,
too)?

-Peff

[PATCH] Add the DEFAULT_HELP_FORMAT option to Makefile

[vfr]

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.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -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)
 
-- 
1.7.9.msysgit.0

Re: [PATCH] Add the DEFAULT_HELP_FORMAT option to Makefile

[Junio C Hamano]

vfr@lyx.org writes:

> 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.

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

Re: [PATCH] Add the DEFAULT_HELP_FORMAT option to Makefile

[Vincent van Ravesteijn]

Op 6-6-2012 21:06, Junio C Hamano schreef:
> 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?

I don't know. The only place I know where GIT-BUILD-OPTIONS  is used is 
in the test suite. The test suite runs fine with the patches.

Vincent

Re: [PATCH] Add the DEFAULT_HELP_FORMAT option to Makefile

[Junio C Hamano]

Vincent van Ravesteijn <vfr@lyx.org> writes:

> Op 6-6-2012 21:06, Junio C Hamano schreef:
>> 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?
>
> I don't know. The only place I know where GIT-BUILD-OPTIONS  is used
> is in the test suite. The test suite runs fine with the patches.

Actually what I had in mind was GIT-CFLAGS which makes sure that we
rebuild things when compilation options change, but ALL_CFLAGS is
covered already so there is no problem.

Thanks for a sanity check.

[PATCHv2] Add the possibility to specify a default help format

[Vincent van Ravesteijn]

At the moment, the default help format (i.e. the format that is chosen if
'git help xxx' is called without a help format parameter) is defined by
the switch to be 'man'. However, on different platforms the preferred
format might differ. For example, on Windows there is no man viewer, so we
would prefer html.

This patch adds the possibility to choose a default help format on
compilation by defining DEFAULT_HELP_FORMAT. If it is not specified the
default is still 'man'.

Example:
  make DEFAULT_HELP_FORMAT=info

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
---
 Makefile       |    9 +++++++++
 builtin/help.c |    8 +++++++-
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 4592f1f..9df6213 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" or "html" 
+# (defaults to "man") if you want to have a different default when
+# "git help" is called without a parameter specifying the format.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1236,6 +1240,7 @@ ifeq ($(uname_S),Windows)
 	BLK_SHA1 = YesPlease
 	NO_POSIX_GOODIES = UnfortunatelyYes
 	NATIVE_CRLF = YesPlease
+	DEFAULT_HELP_FORMAT = html
 
 	CC = compat/vcbuild/scripts/clink.pl
 	AR = compat/vcbuild/scripts/lib.pl
@@ -1915,6 +1920,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)
 
diff --git a/builtin/help.c b/builtin/help.c
index 43d3c84..536d4fd 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -12,6 +12,10 @@
 #include "column.h"
 #include "help.h"
 
+#ifndef DEFAULT_HELP_FORMAT
+#define DEFAULT_HELP_FORMAT "man"
+#endif
+
 static struct man_viewer_list {
 	struct man_viewer_list *next;
 	char name[FLEX_ARRAY];
@@ -445,7 +449,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	setup_git_directory_gently(&nongit);
 	git_config(git_help_config, NULL);
 
-	if (parsed_help_format != HELP_FORMAT_NONE)
+	if (parsed_help_format == HELP_FORMAT_NONE)
+		help_format = parse_help_format(DEFAULT_HELP_FORMAT);
+	else
 		help_format = parsed_help_format;
 
 	alias = alias_lookup(argv[0]);
-- 
1.7.9.msysgit.0

Re: [PATCHv2] Add the possibility to specify a default help format

[Junio C Hamano]

Vincent van Ravesteijn <vfr@lyx.org> writes:

> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>

If there were an existing test that assumes "git help" will default
to "man" and verify output from the command, this patch alone will
break it, but I didn't check.

Thanks; will queue (with some rephrasing of the log message).