diff mbox series

[v4,02/15] help: move list_config_help to builtin/help

Message ID 20191213004312.169753-3-emilyshaffer@google.com (mailing list archive)
State New, archived
Headers show
Series [v4,01/15] bugreport: add tool to generate debugging info | expand

Commit Message

Emily Shaffer Dec. 13, 2019, 12:42 a.m. UTC 
Starting in 3ac68a93fd2, help.o began to depend on builtin/branch.o,
builtin/clean.o, and builtin/config.o. This meant that help.o was
unusable outside of the context of the main Git executable.

To make help.o usable by other commands again, move list_config_help()
into builtin/help.c (where it makes sense to assume other builtin libraries
are present).

When command-list.h is included but a member is not used, we start to
hear a compiler warning. Since the config list is generated in a fairly
different way than the command list, and since commands and config
options are semantically different, move the config list into its own
header and move the generator into its own script and build rule.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 .gitignore             |  1 +
 Makefile               | 16 ++++++--
 builtin/help.c         | 86 ++++++++++++++++++++++++++++++++++++++++++
 generate-cmdlist.sh    | 19 ----------
 generate-configlist.sh | 24 ++++++++++++
 help.c                 | 85 -----------------------------------------
 help.h                 |  1 -
 7 files changed, 123 insertions(+), 109 deletions(-)
 create mode 100755 generate-configlist.sh

Comments

Junio C Hamano Dec. 13, 2019, 8:51 p.m. UTC | #1 
Emily Shaffer <emilyshaffer@google.com> writes:

> Starting in 3ac68a93fd2, help.o began to depend on builtin/branch.o,
> builtin/clean.o, and builtin/config.o. This meant that help.o was
> unusable outside of the context of the main Git executable.
>
> To make help.o usable by other commands again, move list_config_help()
> into builtin/help.c (where it makes sense to assume other builtin libraries
> are present).
>
> When command-list.h is included but a member is not used, we start to
> hear a compiler warning. Since the config list is generated in a fairly
> different way than the command list, and since commands and config
> options are semantically different, move the config list into its own
> header and move the generator into its own script and build rule.
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---

OK.  Looks like a clean-up that is very worthy even without the
remainder of the topic.

Thanks.

>  .gitignore             |  1 +
>  Makefile               | 16 ++++++--
>  builtin/help.c         | 86 ++++++++++++++++++++++++++++++++++++++++++
>  generate-cmdlist.sh    | 19 ----------
>  generate-configlist.sh | 24 ++++++++++++
>  help.c                 | 85 -----------------------------------------
>  help.h                 |  1 -
>  7 files changed, 123 insertions(+), 109 deletions(-)
>  create mode 100755 generate-configlist.sh
>
> diff --git a/.gitignore b/.gitignore
> index 055a84c4a8..5dde2cc4c8 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -189,6 +189,7 @@
>  /gitweb/gitweb.cgi
>  /gitweb/static/gitweb.js
>  /gitweb/static/gitweb.min.*
> +/config-list.h
>  /command-list.h
>  *.tar.gz
>  *.dsc
> diff --git a/Makefile b/Makefile
> index 9dff91436e..c49f55a521 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -815,6 +815,7 @@ LIB_FILE = libgit.a
>  XDIFF_LIB = xdiff/lib.a
>  VCSSVN_LIB = vcs-svn/lib.a
>  
> +GENERATED_H += config-list.h
>  GENERATED_H += command-list.h
>  
>  LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
> @@ -2127,7 +2128,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
>  
>  help.sp help.s help.o: command-list.h
>  
> -builtin/help.sp builtin/help.s builtin/help.o: command-list.h GIT-PREFIX
> +builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
>  builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
>  	'-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
>  	'-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \
> @@ -2147,6 +2148,12 @@ $(BUILT_INS): git$X
>  	ln -s $< $@ 2>/dev/null || \
>  	cp $< $@
>  
> +config-list.h: generate-configlist.sh
> +
> +config-list.h:
> +	$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \
> +		>$@+ && mv $@+ $@
> +
>  command-list.h: generate-cmdlist.sh command-list.txt
>  
>  command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Documentation/config/*.txt
> @@ -2784,7 +2791,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
>  .PHONY: sparse $(SP_OBJ)
>  sparse: $(SP_OBJ)
>  
> -GEN_HDRS := command-list.h unicode-width.h
> +GEN_HDRS := config-list.h command-list.h unicode-width.h
>  EXCEPT_HDRS := $(GEN_HDRS) compat/% xdiff/%
>  ifndef GCRYPT_SHA256
>  	EXCEPT_HDRS += sha256/gcrypt.h
> @@ -2807,7 +2814,7 @@ hdr-check: $(HCO)
>  style:
>  	git clang-format --style file --diff --extensions c,h
>  
> -check: command-list.h
> +check: config-list.h command-list.h
>  	@if sparse; \
>  	then \
>  		echo >&2 "Use 'make sparse' instead"; \
> @@ -3110,7 +3117,8 @@ clean: profile-clean coverage-clean cocciclean
>  	$(RM) $(HCC)
>  	$(RM) -r bin-wrappers $(dep_dirs)
>  	$(RM) -r po/build/
> -	$(RM) *.pyc *.pyo */*.pyc */*.pyo command-list.h $(ETAGS_TARGET) tags cscope*
> +	$(RM) *.pyc *.pyo */*.pyc */*.pyo config-list.h command-list.h
> +	$(RM) $(ETAGS_TARGET) tags cscope*
>  	$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
>  	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
>  	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
> diff --git a/builtin/help.c b/builtin/help.c
> index e5590d7787..1c5f2b9255 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -8,6 +8,7 @@
>  #include "parse-options.h"
>  #include "run-command.h"
>  #include "column.h"
> +#include "config-list.h"
>  #include "help.h"
>  #include "alias.h"
>  
> @@ -62,6 +63,91 @@ static const char * const builtin_help_usage[] = {
>  	NULL
>  };
>  
> +struct slot_expansion {
> +	const char *prefix;
> +	const char *placeholder;
> +	void (*fn)(struct string_list *list, const char *prefix);
> +	int found;
> +};
> +
> +static void list_config_help(int for_human)
> +{
> +	struct slot_expansion slot_expansions[] = {
> +		{ "advice", "*", list_config_advices },
> +		{ "color.branch", "<slot>", list_config_color_branch_slots },
> +		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
> +		{ "color.diff", "<slot>", list_config_color_diff_slots },
> +		{ "color.grep", "<slot>", list_config_color_grep_slots },
> +		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
> +		{ "color.remote", "<slot>", list_config_color_sideband_slots },
> +		{ "color.status", "<slot>", list_config_color_status_slots },
> +		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
> +		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
> +		{ NULL, NULL, NULL }
> +	};
> +	const char **p;
> +	struct slot_expansion *e;
> +	struct string_list keys = STRING_LIST_INIT_DUP;
> +	int i;
> +
> +	for (p = config_name_list; *p; p++) {
> +		const char *var = *p;
> +		struct strbuf sb = STRBUF_INIT;
> +
> +		for (e = slot_expansions; e->prefix; e++) {
> +
> +			strbuf_reset(&sb);
> +			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
> +			if (!strcasecmp(var, sb.buf)) {
> +				e->fn(&keys, e->prefix);
> +				e->found++;
> +				break;
> +			}
> +		}
> +		strbuf_release(&sb);
> +		if (!e->prefix)
> +			string_list_append(&keys, var);
> +	}
> +
> +	for (e = slot_expansions; e->prefix; e++)
> +		if (!e->found)
> +			BUG("slot_expansion %s.%s is not used",
> +			    e->prefix, e->placeholder);
> +
> +	string_list_sort(&keys);
> +	for (i = 0; i < keys.nr; i++) {
> +		const char *var = keys.items[i].string;
> +		const char *wildcard, *tag, *cut;
> +
> +		if (for_human) {
> +			puts(var);
> +			continue;
> +		}
> +
> +		wildcard = strchr(var, '*');
> +		tag = strchr(var, '<');
> +
> +		if (!wildcard && !tag) {
> +			puts(var);
> +			continue;
> +		}
> +
> +		if (wildcard && !tag)
> +			cut = wildcard;
> +		else if (!wildcard && tag)
> +			cut = tag;
> +		else
> +			cut = wildcard < tag ? wildcard : tag;
> +
> +		/*
> +		 * We may produce duplicates, but that's up to
> +		 * git-completion.bash to handle
> +		 */
> +		printf("%.*s\n", (int)(cut - var), var);
> +	}
> +	string_list_clear(&keys, 0);
> +}
> +
>  static enum help_format parse_help_format(const char *format)
>  {
>  	if (!strcmp(format, "man"))
> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> index 71158f7d8b..45fecf8bdf 100755
> --- a/generate-cmdlist.sh
> +++ b/generate-cmdlist.sh
> @@ -76,23 +76,6 @@ print_command_list () {
>  	echo "};"
>  }
>  
> -print_config_list () {
> -	cat <<EOF
> -static const char *config_name_list[] = {
> -EOF
> -	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
> -	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
> -	sort |
> -	while read line
> -	do
> -		echo "	\"$line\","
> -	done
> -	cat <<EOF
> -	NULL,
> -};
> -EOF
> -}
> -
>  exclude_programs=
>  while test "--exclude-program" = "$1"
>  do
> @@ -113,5 +96,3 @@ echo
>  define_category_names "$1"
>  echo
>  print_command_list "$1"
> -echo
> -print_config_list
> diff --git a/generate-configlist.sh b/generate-configlist.sh
> new file mode 100755
> index 0000000000..eca6a00c30
> --- /dev/null
> +++ b/generate-configlist.sh
> @@ -0,0 +1,24 @@
> +#!/bin/sh
> +
> +echo "/* Automatically generated by generate-configlist.sh */"
> +echo
> +
> +print_config_list () {
> +	cat <<EOF
> +static const char *config_name_list[] = {
> +EOF
> +	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
> +	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
> +	sort |
> +	while read line
> +	do
> +		echo "	\"$line\","
> +	done
> +	cat <<EOF
> +	NULL,
> +};
> +EOF
> +}
> +
> +echo
> +print_config_list
> diff --git a/help.c b/help.c
> index cf67624a94..a21487db77 100644
> --- a/help.c
> +++ b/help.c
> @@ -407,91 +407,6 @@ void list_common_guides_help(void)
>  	putchar('\n');
>  }
>  
> -struct slot_expansion {
> -	const char *prefix;
> -	const char *placeholder;
> -	void (*fn)(struct string_list *list, const char *prefix);
> -	int found;
> -};
> -
> -void list_config_help(int for_human)
> -{
> -	struct slot_expansion slot_expansions[] = {
> -		{ "advice", "*", list_config_advices },
> -		{ "color.branch", "<slot>", list_config_color_branch_slots },
> -		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
> -		{ "color.diff", "<slot>", list_config_color_diff_slots },
> -		{ "color.grep", "<slot>", list_config_color_grep_slots },
> -		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
> -		{ "color.remote", "<slot>", list_config_color_sideband_slots },
> -		{ "color.status", "<slot>", list_config_color_status_slots },
> -		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
> -		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
> -		{ NULL, NULL, NULL }
> -	};
> -	const char **p;
> -	struct slot_expansion *e;
> -	struct string_list keys = STRING_LIST_INIT_DUP;
> -	int i;
> -
> -	for (p = config_name_list; *p; p++) {
> -		const char *var = *p;
> -		struct strbuf sb = STRBUF_INIT;
> -
> -		for (e = slot_expansions; e->prefix; e++) {
> -
> -			strbuf_reset(&sb);
> -			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
> -			if (!strcasecmp(var, sb.buf)) {
> -				e->fn(&keys, e->prefix);
> -				e->found++;
> -				break;
> -			}
> -		}
> -		strbuf_release(&sb);
> -		if (!e->prefix)
> -			string_list_append(&keys, var);
> -	}
> -
> -	for (e = slot_expansions; e->prefix; e++)
> -		if (!e->found)
> -			BUG("slot_expansion %s.%s is not used",
> -			    e->prefix, e->placeholder);
> -
> -	string_list_sort(&keys);
> -	for (i = 0; i < keys.nr; i++) {
> -		const char *var = keys.items[i].string;
> -		const char *wildcard, *tag, *cut;
> -
> -		if (for_human) {
> -			puts(var);
> -			continue;
> -		}
> -
> -		wildcard = strchr(var, '*');
> -		tag = strchr(var, '<');
> -
> -		if (!wildcard && !tag) {
> -			puts(var);
> -			continue;
> -		}
> -
> -		if (wildcard && !tag)
> -			cut = wildcard;
> -		else if (!wildcard && tag)
> -			cut = tag;
> -		else
> -			cut = wildcard < tag ? wildcard : tag;
> -
> -		/*
> -		 * We may produce duplicates, but that's up to
> -		 * git-completion.bash to handle
> -		 */
> -		printf("%.*s\n", (int)(cut - var), var);
> -	}
> -	string_list_clear(&keys, 0);
> -}
> -
>  static int get_alias(const char *var, const char *value, void *data)
>  {
>  	struct string_list *list = data;
> diff --git a/help.h b/help.h
> index 7a455beeb7..9071894e8c 100644
> --- a/help.h
> +++ b/help.h
> @@ -22,7 +22,6 @@ static inline void mput_char(char c, unsigned int num)
>  void list_common_cmds_help(void);
>  void list_all_cmds_help(void);
>  void list_common_guides_help(void);
> -void list_config_help(int for_human);
>  
>  void list_all_main_cmds(struct string_list *list);
>  void list_all_other_cmds(struct string_list *list);
Emily Shaffer Dec. 16, 2019, 9:36 p.m. UTC | #2 
On Fri, Dec 13, 2019 at 12:51:58PM -0800, Junio C Hamano wrote:
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > Starting in 3ac68a93fd2, help.o began to depend on builtin/branch.o,
> > builtin/clean.o, and builtin/config.o. This meant that help.o was
> > unusable outside of the context of the main Git executable.
> >
> > To make help.o usable by other commands again, move list_config_help()
> > into builtin/help.c (where it makes sense to assume other builtin libraries
> > are present).
> >
> > When command-list.h is included but a member is not used, we start to
> > hear a compiler warning. Since the config list is generated in a fairly
> > different way than the command list, and since commands and config
> > options are semantically different, move the config list into its own
> > header and move the generator into its own script and build rule.
> >
> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> > ---
> 
> OK.  Looks like a clean-up that is very worthy even without the
> remainder of the topic.
> 
> Thanks.

Ok. Are you going to take it on its own? If so, I'll be relying on two
branches in pu/ - what's the best way to let you know I need them both
if/when I send another version of this chain?

> 
> >  .gitignore             |  1 +
> >  Makefile               | 16 ++++++--
> >  builtin/help.c         | 86 ++++++++++++++++++++++++++++++++++++++++++
> >  generate-cmdlist.sh    | 19 ----------
> >  generate-configlist.sh | 24 ++++++++++++
> >  help.c                 | 85 -----------------------------------------
> >  help.h                 |  1 -
> >  7 files changed, 123 insertions(+), 109 deletions(-)
> >  create mode 100755 generate-configlist.sh
> >
> > diff --git a/.gitignore b/.gitignore
> > index 055a84c4a8..5dde2cc4c8 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -189,6 +189,7 @@
> >  /gitweb/gitweb.cgi
> >  /gitweb/static/gitweb.js
> >  /gitweb/static/gitweb.min.*
> > +/config-list.h
> >  /command-list.h
> >  *.tar.gz
> >  *.dsc
> > diff --git a/Makefile b/Makefile
> > index 9dff91436e..c49f55a521 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -815,6 +815,7 @@ LIB_FILE = libgit.a
> >  XDIFF_LIB = xdiff/lib.a
> >  VCSSVN_LIB = vcs-svn/lib.a
> >  
> > +GENERATED_H += config-list.h
> >  GENERATED_H += command-list.h
> >  
> >  LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
> > @@ -2127,7 +2128,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
> >  
> >  help.sp help.s help.o: command-list.h
> >  
> > -builtin/help.sp builtin/help.s builtin/help.o: command-list.h GIT-PREFIX
> > +builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
> >  builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
> >  	'-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
> >  	'-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \
> > @@ -2147,6 +2148,12 @@ $(BUILT_INS): git$X
> >  	ln -s $< $@ 2>/dev/null || \
> >  	cp $< $@
> >  
> > +config-list.h: generate-configlist.sh
> > +
> > +config-list.h:
> > +	$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \
> > +		>$@+ && mv $@+ $@
> > +
> >  command-list.h: generate-cmdlist.sh command-list.txt
> >  
> >  command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Documentation/config/*.txt
> > @@ -2784,7 +2791,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
> >  .PHONY: sparse $(SP_OBJ)
> >  sparse: $(SP_OBJ)
> >  
> > -GEN_HDRS := command-list.h unicode-width.h
> > +GEN_HDRS := config-list.h command-list.h unicode-width.h
> >  EXCEPT_HDRS := $(GEN_HDRS) compat/% xdiff/%
> >  ifndef GCRYPT_SHA256
> >  	EXCEPT_HDRS += sha256/gcrypt.h
> > @@ -2807,7 +2814,7 @@ hdr-check: $(HCO)
> >  style:
> >  	git clang-format --style file --diff --extensions c,h
> >  
> > -check: command-list.h
> > +check: config-list.h command-list.h
> >  	@if sparse; \
> >  	then \
> >  		echo >&2 "Use 'make sparse' instead"; \
> > @@ -3110,7 +3117,8 @@ clean: profile-clean coverage-clean cocciclean
> >  	$(RM) $(HCC)
> >  	$(RM) -r bin-wrappers $(dep_dirs)
> >  	$(RM) -r po/build/
> > -	$(RM) *.pyc *.pyo */*.pyc */*.pyo command-list.h $(ETAGS_TARGET) tags cscope*
> > +	$(RM) *.pyc *.pyo */*.pyc */*.pyo config-list.h command-list.h
> > +	$(RM) $(ETAGS_TARGET) tags cscope*
> >  	$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
> >  	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
> >  	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
> > diff --git a/builtin/help.c b/builtin/help.c
> > index e5590d7787..1c5f2b9255 100644
> > --- a/builtin/help.c
> > +++ b/builtin/help.c
> > @@ -8,6 +8,7 @@
> >  #include "parse-options.h"
> >  #include "run-command.h"
> >  #include "column.h"
> > +#include "config-list.h"
> >  #include "help.h"
> >  #include "alias.h"
> >  
> > @@ -62,6 +63,91 @@ static const char * const builtin_help_usage[] = {
> >  	NULL
> >  };
> >  
> > +struct slot_expansion {
> > +	const char *prefix;
> > +	const char *placeholder;
> > +	void (*fn)(struct string_list *list, const char *prefix);
> > +	int found;
> > +};
> > +
> > +static void list_config_help(int for_human)
> > +{
> > +	struct slot_expansion slot_expansions[] = {
> > +		{ "advice", "*", list_config_advices },
> > +		{ "color.branch", "<slot>", list_config_color_branch_slots },
> > +		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
> > +		{ "color.diff", "<slot>", list_config_color_diff_slots },
> > +		{ "color.grep", "<slot>", list_config_color_grep_slots },
> > +		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
> > +		{ "color.remote", "<slot>", list_config_color_sideband_slots },
> > +		{ "color.status", "<slot>", list_config_color_status_slots },
> > +		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
> > +		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
> > +		{ NULL, NULL, NULL }
> > +	};
> > +	const char **p;
> > +	struct slot_expansion *e;
> > +	struct string_list keys = STRING_LIST_INIT_DUP;
> > +	int i;
> > +
> > +	for (p = config_name_list; *p; p++) {
> > +		const char *var = *p;
> > +		struct strbuf sb = STRBUF_INIT;
> > +
> > +		for (e = slot_expansions; e->prefix; e++) {
> > +
> > +			strbuf_reset(&sb);
> > +			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
> > +			if (!strcasecmp(var, sb.buf)) {
> > +				e->fn(&keys, e->prefix);
> > +				e->found++;
> > +				break;
> > +			}
> > +		}
> > +		strbuf_release(&sb);
> > +		if (!e->prefix)
> > +			string_list_append(&keys, var);
> > +	}
> > +
> > +	for (e = slot_expansions; e->prefix; e++)
> > +		if (!e->found)
> > +			BUG("slot_expansion %s.%s is not used",
> > +			    e->prefix, e->placeholder);
> > +
> > +	string_list_sort(&keys);
> > +	for (i = 0; i < keys.nr; i++) {
> > +		const char *var = keys.items[i].string;
> > +		const char *wildcard, *tag, *cut;
> > +
> > +		if (for_human) {
> > +			puts(var);
> > +			continue;
> > +		}
> > +
> > +		wildcard = strchr(var, '*');
> > +		tag = strchr(var, '<');
> > +
> > +		if (!wildcard && !tag) {
> > +			puts(var);
> > +			continue;
> > +		}
> > +
> > +		if (wildcard && !tag)
> > +			cut = wildcard;
> > +		else if (!wildcard && tag)
> > +			cut = tag;
> > +		else
> > +			cut = wildcard < tag ? wildcard : tag;
> > +
> > +		/*
> > +		 * We may produce duplicates, but that's up to
> > +		 * git-completion.bash to handle
> > +		 */
> > +		printf("%.*s\n", (int)(cut - var), var);
> > +	}
> > +	string_list_clear(&keys, 0);
> > +}
> > +
> >  static enum help_format parse_help_format(const char *format)
> >  {
> >  	if (!strcmp(format, "man"))
> > diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> > index 71158f7d8b..45fecf8bdf 100755
> > --- a/generate-cmdlist.sh
> > +++ b/generate-cmdlist.sh
> > @@ -76,23 +76,6 @@ print_command_list () {
> >  	echo "};"
> >  }
> >  
> > -print_config_list () {
> > -	cat <<EOF
> > -static const char *config_name_list[] = {
> > -EOF
> > -	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
> > -	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
> > -	sort |
> > -	while read line
> > -	do
> > -		echo "	\"$line\","
> > -	done
> > -	cat <<EOF
> > -	NULL,
> > -};
> > -EOF
> > -}
> > -
> >  exclude_programs=
> >  while test "--exclude-program" = "$1"
> >  do
> > @@ -113,5 +96,3 @@ echo
> >  define_category_names "$1"
> >  echo
> >  print_command_list "$1"
> > -echo
> > -print_config_list
> > diff --git a/generate-configlist.sh b/generate-configlist.sh
> > new file mode 100755
> > index 0000000000..eca6a00c30
> > --- /dev/null
> > +++ b/generate-configlist.sh
> > @@ -0,0 +1,24 @@
> > +#!/bin/sh
> > +
> > +echo "/* Automatically generated by generate-configlist.sh */"
> > +echo
> > +
> > +print_config_list () {
> > +	cat <<EOF
> > +static const char *config_name_list[] = {
> > +EOF
> > +	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
> > +	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
> > +	sort |
> > +	while read line
> > +	do
> > +		echo "	\"$line\","
> > +	done
> > +	cat <<EOF
> > +	NULL,
> > +};
> > +EOF
> > +}
> > +
> > +echo
> > +print_config_list
> > diff --git a/help.c b/help.c
> > index cf67624a94..a21487db77 100644
> > --- a/help.c
> > +++ b/help.c
> > @@ -407,91 +407,6 @@ void list_common_guides_help(void)
> >  	putchar('\n');
> >  }
> >  
> > -struct slot_expansion {
> > -	const char *prefix;
> > -	const char *placeholder;
> > -	void (*fn)(struct string_list *list, const char *prefix);
> > -	int found;
> > -};
> > -
> > -void list_config_help(int for_human)
> > -{
> > -	struct slot_expansion slot_expansions[] = {
> > -		{ "advice", "*", list_config_advices },
> > -		{ "color.branch", "<slot>", list_config_color_branch_slots },
> > -		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
> > -		{ "color.diff", "<slot>", list_config_color_diff_slots },
> > -		{ "color.grep", "<slot>", list_config_color_grep_slots },
> > -		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
> > -		{ "color.remote", "<slot>", list_config_color_sideband_slots },
> > -		{ "color.status", "<slot>", list_config_color_status_slots },
> > -		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
> > -		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
> > -		{ NULL, NULL, NULL }
> > -	};
> > -	const char **p;
> > -	struct slot_expansion *e;
> > -	struct string_list keys = STRING_LIST_INIT_DUP;
> > -	int i;
> > -
> > -	for (p = config_name_list; *p; p++) {
> > -		const char *var = *p;
> > -		struct strbuf sb = STRBUF_INIT;
> > -
> > -		for (e = slot_expansions; e->prefix; e++) {
> > -
> > -			strbuf_reset(&sb);
> > -			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
> > -			if (!strcasecmp(var, sb.buf)) {
> > -				e->fn(&keys, e->prefix);
> > -				e->found++;
> > -				break;
> > -			}
> > -		}
> > -		strbuf_release(&sb);
> > -		if (!e->prefix)
> > -			string_list_append(&keys, var);
> > -	}
> > -
> > -	for (e = slot_expansions; e->prefix; e++)
> > -		if (!e->found)
> > -			BUG("slot_expansion %s.%s is not used",
> > -			    e->prefix, e->placeholder);
> > -
> > -	string_list_sort(&keys);
> > -	for (i = 0; i < keys.nr; i++) {
> > -		const char *var = keys.items[i].string;
> > -		const char *wildcard, *tag, *cut;
> > -
> > -		if (for_human) {
> > -			puts(var);
> > -			continue;
> > -		}
> > -
> > -		wildcard = strchr(var, '*');
> > -		tag = strchr(var, '<');
> > -
> > -		if (!wildcard && !tag) {
> > -			puts(var);
> > -			continue;
> > -		}
> > -
> > -		if (wildcard && !tag)
> > -			cut = wildcard;
> > -		else if (!wildcard && tag)
> > -			cut = tag;
> > -		else
> > -			cut = wildcard < tag ? wildcard : tag;
> > -
> > -		/*
> > -		 * We may produce duplicates, but that's up to
> > -		 * git-completion.bash to handle
> > -		 */
> > -		printf("%.*s\n", (int)(cut - var), var);
> > -	}
> > -	string_list_clear(&keys, 0);
> > -}
> > -
> >  static int get_alias(const char *var, const char *value, void *data)
> >  {
> >  	struct string_list *list = data;
> > diff --git a/help.h b/help.h
> > index 7a455beeb7..9071894e8c 100644
> > --- a/help.h
> > +++ b/help.h
> > @@ -22,7 +22,6 @@ static inline void mput_char(char c, unsigned int num)
> >  void list_common_cmds_help(void);
> >  void list_all_cmds_help(void);
> >  void list_common_guides_help(void);
> > -void list_config_help(int for_human);
> >  
> >  void list_all_main_cmds(struct string_list *list);
> >  void list_all_other_cmds(struct string_list *list);
Junio C Hamano Dec. 16, 2019, 10:19 p.m. UTC | #3 
Emily Shaffer <emilyshaffer@google.com> writes:

>> OK.  Looks like a clean-up that is very worthy even without the
>> remainder of the topic.
>> 
>> Thanks.
>
> Ok. Are you going to take it on its own?

Not really.  Unless you abandon the rest of the topic, in which case
it may be worth resurrecting this one on its own, that is.  But that
is not what you plan to do anyway, so I expect to see it as part of
an updated series ;-)
Emily Shaffer Dec. 16, 2019, 10:34 p.m. UTC | #4 
On Mon, Dec 16, 2019 at 02:19:48PM -0800, Junio C Hamano wrote:
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> >> OK.  Looks like a clean-up that is very worthy even without the
> >> remainder of the topic.
> >> 
> >> Thanks.
> >
> > Ok. Are you going to take it on its own?
> 
> Not really.  Unless you abandon the rest of the topic, in which case
> it may be worth resurrecting this one on its own, that is.  But that
> is not what you plan to do anyway, so I expect to see it as part of
> an updated series ;-)

Ok! That's fine and makes my life easier. Just making sure. :)

 - Emily
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index 055a84c4a8..5dde2cc4c8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -189,6 +189,7 @@ 
 /gitweb/gitweb.cgi
 /gitweb/static/gitweb.js
 /gitweb/static/gitweb.min.*
+/config-list.h
 /command-list.h
 *.tar.gz
 *.dsc
diff --git a/Makefile b/Makefile
index 9dff91436e..c49f55a521 100644
--- a/Makefile
+++ b/Makefile
@@ -815,6 +815,7 @@  LIB_FILE = libgit.a
 XDIFF_LIB = xdiff/lib.a
 VCSSVN_LIB = vcs-svn/lib.a
 
+GENERATED_H += config-list.h
 GENERATED_H += command-list.h
 
 LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
@@ -2127,7 +2128,7 @@  git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
 
 help.sp help.s help.o: command-list.h
 
-builtin/help.sp builtin/help.s builtin/help.o: command-list.h GIT-PREFIX
+builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
 builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 	'-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
 	'-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \
@@ -2147,6 +2148,12 @@  $(BUILT_INS): git$X
 	ln -s $< $@ 2>/dev/null || \
 	cp $< $@
 
+config-list.h: generate-configlist.sh
+
+config-list.h:
+	$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \
+		>$@+ && mv $@+ $@
+
 command-list.h: generate-cmdlist.sh command-list.txt
 
 command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Documentation/config/*.txt
@@ -2784,7 +2791,7 @@  $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
 .PHONY: sparse $(SP_OBJ)
 sparse: $(SP_OBJ)
 
-GEN_HDRS := command-list.h unicode-width.h
+GEN_HDRS := config-list.h command-list.h unicode-width.h
 EXCEPT_HDRS := $(GEN_HDRS) compat/% xdiff/%
 ifndef GCRYPT_SHA256
 	EXCEPT_HDRS += sha256/gcrypt.h
@@ -2807,7 +2814,7 @@  hdr-check: $(HCO)
 style:
 	git clang-format --style file --diff --extensions c,h
 
-check: command-list.h
+check: config-list.h command-list.h
 	@if sparse; \
 	then \
 		echo >&2 "Use 'make sparse' instead"; \
@@ -3110,7 +3117,8 @@  clean: profile-clean coverage-clean cocciclean
 	$(RM) $(HCC)
 	$(RM) -r bin-wrappers $(dep_dirs)
 	$(RM) -r po/build/
-	$(RM) *.pyc *.pyo */*.pyc */*.pyo command-list.h $(ETAGS_TARGET) tags cscope*
+	$(RM) *.pyc *.pyo */*.pyc */*.pyo config-list.h command-list.h
+	$(RM) $(ETAGS_TARGET) tags cscope*
 	$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
 	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
diff --git a/builtin/help.c b/builtin/help.c
index e5590d7787..1c5f2b9255 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -8,6 +8,7 @@ 
 #include "parse-options.h"
 #include "run-command.h"
 #include "column.h"
+#include "config-list.h"
 #include "help.h"
 #include "alias.h"
 
@@ -62,6 +63,91 @@  static const char * const builtin_help_usage[] = {
 	NULL
 };
 
+struct slot_expansion {
+	const char *prefix;
+	const char *placeholder;
+	void (*fn)(struct string_list *list, const char *prefix);
+	int found;
+};
+
+static void list_config_help(int for_human)
+{
+	struct slot_expansion slot_expansions[] = {
+		{ "advice", "*", list_config_advices },
+		{ "color.branch", "<slot>", list_config_color_branch_slots },
+		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
+		{ "color.diff", "<slot>", list_config_color_diff_slots },
+		{ "color.grep", "<slot>", list_config_color_grep_slots },
+		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
+		{ "color.remote", "<slot>", list_config_color_sideband_slots },
+		{ "color.status", "<slot>", list_config_color_status_slots },
+		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
+		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
+		{ NULL, NULL, NULL }
+	};
+	const char **p;
+	struct slot_expansion *e;
+	struct string_list keys = STRING_LIST_INIT_DUP;
+	int i;
+
+	for (p = config_name_list; *p; p++) {
+		const char *var = *p;
+		struct strbuf sb = STRBUF_INIT;
+
+		for (e = slot_expansions; e->prefix; e++) {
+
+			strbuf_reset(&sb);
+			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
+			if (!strcasecmp(var, sb.buf)) {
+				e->fn(&keys, e->prefix);
+				e->found++;
+				break;
+			}
+		}
+		strbuf_release(&sb);
+		if (!e->prefix)
+			string_list_append(&keys, var);
+	}
+
+	for (e = slot_expansions; e->prefix; e++)
+		if (!e->found)
+			BUG("slot_expansion %s.%s is not used",
+			    e->prefix, e->placeholder);
+
+	string_list_sort(&keys);
+	for (i = 0; i < keys.nr; i++) {
+		const char *var = keys.items[i].string;
+		const char *wildcard, *tag, *cut;
+
+		if (for_human) {
+			puts(var);
+			continue;
+		}
+
+		wildcard = strchr(var, '*');
+		tag = strchr(var, '<');
+
+		if (!wildcard && !tag) {
+			puts(var);
+			continue;
+		}
+
+		if (wildcard && !tag)
+			cut = wildcard;
+		else if (!wildcard && tag)
+			cut = tag;
+		else
+			cut = wildcard < tag ? wildcard : tag;
+
+		/*
+		 * We may produce duplicates, but that's up to
+		 * git-completion.bash to handle
+		 */
+		printf("%.*s\n", (int)(cut - var), var);
+	}
+	string_list_clear(&keys, 0);
+}
+
 static enum help_format parse_help_format(const char *format)
 {
 	if (!strcmp(format, "man"))
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 71158f7d8b..45fecf8bdf 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -76,23 +76,6 @@  print_command_list () {
 	echo "};"
 }
 
-print_config_list () {
-	cat <<EOF
-static const char *config_name_list[] = {
-EOF
-	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
-	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
-	sort |
-	while read line
-	do
-		echo "	\"$line\","
-	done
-	cat <<EOF
-	NULL,
-};
-EOF
-}
-
 exclude_programs=
 while test "--exclude-program" = "$1"
 do
@@ -113,5 +96,3 @@  echo
 define_category_names "$1"
 echo
 print_command_list "$1"
-echo
-print_config_list
diff --git a/generate-configlist.sh b/generate-configlist.sh
new file mode 100755
index 0000000000..eca6a00c30
--- /dev/null
+++ b/generate-configlist.sh
@@ -0,0 +1,24 @@ 
+#!/bin/sh
+
+echo "/* Automatically generated by generate-configlist.sh */"
+echo
+
+print_config_list () {
+	cat <<EOF
+static const char *config_name_list[] = {
+EOF
+	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
+	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
+	sort |
+	while read line
+	do
+		echo "	\"$line\","
+	done
+	cat <<EOF
+	NULL,
+};
+EOF
+}
+
+echo
+print_config_list
diff --git a/help.c b/help.c
index cf67624a94..a21487db77 100644
--- a/help.c
+++ b/help.c
@@ -407,91 +407,6 @@  void list_common_guides_help(void)
 	putchar('\n');
 }
 
-struct slot_expansion {
-	const char *prefix;
-	const char *placeholder;
-	void (*fn)(struct string_list *list, const char *prefix);
-	int found;
-};
-
-void list_config_help(int for_human)
-{
-	struct slot_expansion slot_expansions[] = {
-		{ "advice", "*", list_config_advices },
-		{ "color.branch", "<slot>", list_config_color_branch_slots },
-		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
-		{ "color.diff", "<slot>", list_config_color_diff_slots },
-		{ "color.grep", "<slot>", list_config_color_grep_slots },
-		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
-		{ "color.remote", "<slot>", list_config_color_sideband_slots },
-		{ "color.status", "<slot>", list_config_color_status_slots },
-		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
-		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
-		{ NULL, NULL, NULL }
-	};
-	const char **p;
-	struct slot_expansion *e;
-	struct string_list keys = STRING_LIST_INIT_DUP;
-	int i;
-
-	for (p = config_name_list; *p; p++) {
-		const char *var = *p;
-		struct strbuf sb = STRBUF_INIT;
-
-		for (e = slot_expansions; e->prefix; e++) {
-
-			strbuf_reset(&sb);
-			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
-			if (!strcasecmp(var, sb.buf)) {
-				e->fn(&keys, e->prefix);
-				e->found++;
-				break;
-			}
-		}
-		strbuf_release(&sb);
-		if (!e->prefix)
-			string_list_append(&keys, var);
-	}
-
-	for (e = slot_expansions; e->prefix; e++)
-		if (!e->found)
-			BUG("slot_expansion %s.%s is not used",
-			    e->prefix, e->placeholder);
-
-	string_list_sort(&keys);
-	for (i = 0; i < keys.nr; i++) {
-		const char *var = keys.items[i].string;
-		const char *wildcard, *tag, *cut;
-
-		if (for_human) {
-			puts(var);
-			continue;
-		}
-
-		wildcard = strchr(var, '*');
-		tag = strchr(var, '<');
-
-		if (!wildcard && !tag) {
-			puts(var);
-			continue;
-		}
-
-		if (wildcard && !tag)
-			cut = wildcard;
-		else if (!wildcard && tag)
-			cut = tag;
-		else
-			cut = wildcard < tag ? wildcard : tag;
-
-		/*
-		 * We may produce duplicates, but that's up to
-		 * git-completion.bash to handle
-		 */
-		printf("%.*s\n", (int)(cut - var), var);
-	}
-	string_list_clear(&keys, 0);
-}
-
 static int get_alias(const char *var, const char *value, void *data)
 {
 	struct string_list *list = data;
diff --git a/help.h b/help.h
index 7a455beeb7..9071894e8c 100644
--- a/help.h
+++ b/help.h
@@ -22,7 +22,6 @@  static inline void mput_char(char c, unsigned int num)
 void list_common_cmds_help(void);
 void list_all_cmds_help(void);
 void list_common_guides_help(void);
-void list_config_help(int for_human);
 
 void list_all_main_cmds(struct string_list *list);
 void list_all_other_cmds(struct string_list *list);