threads / patch / 30727

patchAdd the possibility to specify a default help format

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

## tl;dr

8 messages between Jun 6, 2012 and Jun 6, 2012. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Vincent van Ravesteijn· Jun 6, 2012, 12:56 UTC · lore

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(-)
Show changes to builtin/help.c +7 −1
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
Jeff King· Jun 6, 2012, 13:53 UTC · re: Vincent van Ravesteijn · lore

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

On Wed, Jun 06, 2012 at 12:56:37PM +0000, Vincent van Ravesteijn wrote:
Show 9 quoted lines
> 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
vfr@lyx.org· Jun 6, 2012, 18:51 UTC · re: Jeff King · lore

[PATCH] Add the DEFAULT_HELP_FORMAT option to Makefile

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(-)
Show changes to Makefile +9 −0
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
Junio C Hamano· Jun 6, 2012, 19:06 UTC · re: vfr@lyx.org · lore

Re: [PATCH] Add the DEFAULT_HELP_FORMAT option to Makefile

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.
Vincent van Ravesteijn· Jun 6, 2012, 20:25 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add the DEFAULT_HELP_FORMAT option to Makefile

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
Junio C Hamano· Jun 6, 2012, 21:07 UTC · re: Vincent van Ravesteijn · lore

Re: [PATCH] Add the DEFAULT_HELP_FORMAT option to Makefile

Vincent van Ravesteijn <vfr@lyx.org> writes:
Show 6 quoted lines
> 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.
Vincent van Ravesteijn· Jun 6, 2012, 20:28 UTC · re: Vincent van Ravesteijn · lore

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

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(-)
Show changes to 2 files +16 −1

Makefile, builtin/help.c

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
Junio C Hamano· Jun 6, 2012, 21:21 UTC · re: Vincent van Ravesteijn · lore

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

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

← back to recent threads