git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/9] completion: avoid hard coding config var list
@ 2018-05-10 14:19 Nguyễn Thái Ngọc Duy
  2018-05-10 14:19 ` [PATCH 1/9] Add and use generic name->id mapping code for color slot parsing Nguyễn Thái Ngọc Duy
                   ` (11 more replies)
  0 siblings, 12 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-10 14:19 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This is part of the attempt to automate more in git-completion.bash.
This series is about making 'git' binary provide the list of
recognized configuration variables and feeding it to git-completion.bash.

The approach is less than ideal. Unlike parse-options, we don't have a
reliable source for configuration variables. Instead, we go grep
config.txt for variable names.

This should work most of the time even if it might miss some variables
if we start to write funny things in config.txt. But compared to the
current approach "every now and then, manually go through config.txt
(or something) and update _git_config() function", this is still
definitely better.

The result is about 200 more completable options (from current 324).
Granted most of this is fsck message ids, but I'm sure it has picked
up a few good config variables and will continue so in the future.

While at there, this config list is also available to the user via
`git help --config` if you can't remember the exact config name you
want and don't want to go through that big git-config man page.

PS. Yes too many complete options (524 now) is not that helpful. I
have a plan to deal with this but probably after I'm done with the
--no-xxx completion topic. Stay tuned.

(on top of nd/command-list)

Nguyễn Thái Ngọc Duy (9):
  Add and use generic name->id mapping code for color slot parsing
  grep: keep all colors in an array
  fsck: factor out msg_id_info[] lazy initialization code
  help: add --config to list all available config
  advice: keep config name in camelCase in advice_config[]
  am: move advice.amWorkDir parsing back to advice.c
  completion: drop the hard coded list of config vars
  completion: support case-insensitive config vars just a bit
  log-tree: allow to customize 'grafted' color

 Documentation/config.txt               |   3 +-
 Documentation/git-help.txt             |   5 +
 advice.c                               |  51 ++--
 advice.h                               |   1 +
 builtin/am.c                           |   5 +-
 builtin/branch.c                       |  28 +-
 builtin/clean.c                        |  28 +-
 builtin/commit.c                       |  40 +--
 builtin/help.c                         |  14 +
 config.c                               |  13 +
 config.h                               |   4 +
 contrib/completion/git-completion.bash | 340 +------------------------
 diff.c                                 |  60 ++---
 fsck.c                                 |  49 ++--
 generate-cmdlist.sh                    |  20 ++
 grep.c                                 | 113 ++++----
 grep.h                                 |  21 +-
 help.c                                 |  74 ++++++
 help.h                                 |  18 ++
 log-tree.c                             |  36 +--
 t/t4254-am-corrupt.sh                  |   2 +-
 21 files changed, 387 insertions(+), 538 deletions(-)

-- 
2.17.0.705.g3525833791


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/9] Add and use generic name->id mapping code for color slot parsing
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
@ 2018-05-10 14:19 ` Nguyễn Thái Ngọc Duy
  2018-05-10 17:16   ` Stefan Beller
  2018-05-10 14:19 ` [PATCH 2/9] grep: keep all colors in an array Nguyễn Thái Ngọc Duy
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-10 14:19 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Instead of hard coding the name-to-id mapping in C code, keep it in an
array and use a common function to do the parsing. This reduces code
and also allows us to list all possible color slots later.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/branch.c | 28 +++++++++----------------
 builtin/clean.c  | 28 +++++++++----------------
 builtin/commit.c | 33 ++++++++++++++----------------
 config.c         | 13 ++++++++++++
 config.h         |  4 ++++
 diff.c           | 53 +++++++++++++++++++-----------------------------
 log-tree.c       | 35 ++++++++------------------------
 7 files changed, 82 insertions(+), 112 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 5bd2a0dd48..b41f332589 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -55,26 +55,18 @@ enum color_branch {
 	BRANCH_COLOR_UPSTREAM = 5
 };
 
+static const char *color_branch_slots[] = {
+	[BRANCH_COLOR_RESET]	= "reset",
+	[BRANCH_COLOR_PLAIN]	= "plain",
+	[BRANCH_COLOR_REMOTE]	= "remote",
+	[BRANCH_COLOR_LOCAL]	= "local",
+	[BRANCH_COLOR_CURRENT]	= "current",
+	[BRANCH_COLOR_UPSTREAM] = "upstream",
+};
+
 static struct string_list output = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
-static int parse_branch_color_slot(const char *slot)
-{
-	if (!strcasecmp(slot, "plain"))
-		return BRANCH_COLOR_PLAIN;
-	if (!strcasecmp(slot, "reset"))
-		return BRANCH_COLOR_RESET;
-	if (!strcasecmp(slot, "remote"))
-		return BRANCH_COLOR_REMOTE;
-	if (!strcasecmp(slot, "local"))
-		return BRANCH_COLOR_LOCAL;
-	if (!strcasecmp(slot, "current"))
-		return BRANCH_COLOR_CURRENT;
-	if (!strcasecmp(slot, "upstream"))
-		return BRANCH_COLOR_UPSTREAM;
-	return -1;
-}
-
 static int git_branch_config(const char *var, const char *value, void *cb)
 {
 	const char *slot_name;
@@ -86,7 +78,7 @@ static int git_branch_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (skip_prefix(var, "color.branch.", &slot_name)) {
-		int slot = parse_branch_color_slot(slot_name);
+		int slot = LOOKUP_CONFIG(color_branch_slots, slot_name);
 		if (slot < 0)
 			return 0;
 		if (!value)
diff --git a/builtin/clean.c b/builtin/clean.c
index fad533a0a7..0ccd95e693 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -42,6 +42,15 @@ enum color_clean {
 	CLEAN_COLOR_ERROR = 5
 };
 
+static const char *color_interactive_slots[] = {
+	[CLEAN_COLOR_ERROR]  = "error",
+	[CLEAN_COLOR_HEADER] = "header",
+	[CLEAN_COLOR_HELP]   = "help",
+	[CLEAN_COLOR_PLAIN]  = "plain",
+	[CLEAN_COLOR_PROMPT] = "prompt",
+	[CLEAN_COLOR_RESET]  = "reset",
+};
+
 static int clean_use_color = -1;
 static char clean_colors[][COLOR_MAXLEN] = {
 	[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
@@ -82,23 +91,6 @@ struct menu_stuff {
 	void *stuff;
 };
 
-static int parse_clean_color_slot(const char *var)
-{
-	if (!strcasecmp(var, "reset"))
-		return CLEAN_COLOR_RESET;
-	if (!strcasecmp(var, "plain"))
-		return CLEAN_COLOR_PLAIN;
-	if (!strcasecmp(var, "prompt"))
-		return CLEAN_COLOR_PROMPT;
-	if (!strcasecmp(var, "header"))
-		return CLEAN_COLOR_HEADER;
-	if (!strcasecmp(var, "help"))
-		return CLEAN_COLOR_HELP;
-	if (!strcasecmp(var, "error"))
-		return CLEAN_COLOR_ERROR;
-	return -1;
-}
-
 static int git_clean_config(const char *var, const char *value, void *cb)
 {
 	const char *slot_name;
@@ -113,7 +105,7 @@ static int git_clean_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (skip_prefix(var, "color.interactive.", &slot_name)) {
-		int slot = parse_clean_color_slot(slot_name);
+		int slot = LOOKUP_CONFIG(color_interactive_slots, slot_name);
 		if (slot < 0)
 			return 0;
 		if (!value)
diff --git a/builtin/commit.c b/builtin/commit.c
index 37fcb55ab0..bee5825bd2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -66,6 +66,18 @@ N_("If you wish to skip this commit, use:\n"
 "Then \"git cherry-pick --continue\" will resume cherry-picking\n"
 "the remaining commits.\n");
 
+static const char *color_status_slots[] = {
+	[WT_STATUS_HEADER]	  = "header",
+	[WT_STATUS_UPDATED]	  = "updated",
+	[WT_STATUS_CHANGED]	  = "changed",
+	[WT_STATUS_UNTRACKED]	  = "untracked",
+	[WT_STATUS_NOBRANCH]	  = "noBranch",
+	[WT_STATUS_UNMERGED]	  = "unmerged",
+	[WT_STATUS_LOCAL_BRANCH]  = "localBranch",
+	[WT_STATUS_REMOTE_BRANCH] = "remoteBranch",
+	[WT_STATUS_ONBRANCH]	  = "branch",
+};
+
 static const char *use_message_buffer;
 static struct lock_file index_lock; /* real index */
 static struct lock_file false_lock; /* used only for partial commits */
@@ -1176,25 +1188,10 @@ static int dry_run_commit(int argc, const char **argv, const char *prefix,
 
 static int parse_status_slot(const char *slot)
 {
-	if (!strcasecmp(slot, "header"))
-		return WT_STATUS_HEADER;
-	if (!strcasecmp(slot, "branch"))
-		return WT_STATUS_ONBRANCH;
-	if (!strcasecmp(slot, "updated") || !strcasecmp(slot, "added"))
+	if (!strcasecmp(slot, "added"))
 		return WT_STATUS_UPDATED;
-	if (!strcasecmp(slot, "changed"))
-		return WT_STATUS_CHANGED;
-	if (!strcasecmp(slot, "untracked"))
-		return WT_STATUS_UNTRACKED;
-	if (!strcasecmp(slot, "nobranch"))
-		return WT_STATUS_NOBRANCH;
-	if (!strcasecmp(slot, "unmerged"))
-		return WT_STATUS_UNMERGED;
-	if (!strcasecmp(slot, "localBranch"))
-		return WT_STATUS_LOCAL_BRANCH;
-	if (!strcasecmp(slot, "remoteBranch"))
-		return WT_STATUS_REMOTE_BRANCH;
-	return -1;
+
+	return LOOKUP_CONFIG(color_status_slots, slot);
 }
 
 static int git_status_config(const char *k, const char *v, void *cb)
diff --git a/config.c b/config.c
index c698988f5e..69ea707f36 100644
--- a/config.c
+++ b/config.c
@@ -3041,3 +3041,16 @@ enum config_scope current_config_scope(void)
 	else
 		return current_parsing_scope;
 }
+
+int lookup_config(const char **mapping, int nr_mapping, const char *var)
+{
+	int i;
+
+	for (i = 0; i < nr_mapping; i++) {
+		const char *name = mapping[i];
+
+		if (name && !strcasecmp(var, name))
+			return i;
+	}
+	return -1;
+}
diff --git a/config.h b/config.h
index ef70a9cac1..cf35ab74db 100644
--- a/config.h
+++ b/config.h
@@ -231,4 +231,8 @@ struct key_value_info {
 extern NORETURN void git_die_config(const char *key, const char *err, ...) __attribute__((format(printf, 2, 3)));
 extern NORETURN void git_die_config_linenr(const char *key, const char *filename, int linenr);
 
+#define LOOKUP_CONFIG(mapping, var) \
+	lookup_config(mapping, ARRAY_SIZE(mapping), var)
+int lookup_config(const char **mapping, int nr_mapping, const char *var);
+
 #endif /* CONFIG_H */
diff --git a/diff.c b/diff.c
index 1289df4b1f..6f781a2c57 100644
--- a/diff.c
+++ b/diff.c
@@ -69,6 +69,25 @@ static char diff_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_FAINT_ITALIC,	/* NEW_MOVED_ALTERNATIVE_DIM */
 };
 
+static const char *color_diff_slots[] = {
+	[DIFF_CONTEXT]		      = "context",
+	[DIFF_METAINFO]		      = "meta",
+	[DIFF_FRAGINFO]		      = "frag",
+	[DIFF_FILE_OLD]		      = "old",
+	[DIFF_FILE_NEW]		      = "new",
+	[DIFF_COMMIT]		      = "commit",
+	[DIFF_WHITESPACE]	      = "whitespace",
+	[DIFF_FUNCINFO]		      = "func",
+	[DIFF_FILE_OLD_MOVED]	      = "oldMoved",
+	[DIFF_FILE_OLD_MOVED_ALT]     = "oldMovedAlternative",
+	[DIFF_FILE_OLD_MOVED_DIM]     = "oldMovedDimmed",
+	[DIFF_FILE_OLD_MOVED_ALT_DIM] = "oldMovedAlternativeDimmed",
+	[DIFF_FILE_NEW_MOVED]	      = "newMoved",
+	[DIFF_FILE_NEW_MOVED_ALT]     = "newMovedAlternative",
+	[DIFF_FILE_NEW_MOVED_DIM]     = "newMovedDimmed",
+	[DIFF_FILE_NEW_MOVED_ALT_DIM] = "newMovedAlternativeDimmed",
+};
+
 static NORETURN void die_want_option(const char *option_name)
 {
 	die(_("option '%s' requires a value"), option_name);
@@ -76,39 +95,9 @@ static NORETURN void die_want_option(const char *option_name)
 
 static int parse_diff_color_slot(const char *var)
 {
-	if (!strcasecmp(var, "context") || !strcasecmp(var, "plain"))
+	if (!strcasecmp(var, "plain"))
 		return DIFF_CONTEXT;
-	if (!strcasecmp(var, "meta"))
-		return DIFF_METAINFO;
-	if (!strcasecmp(var, "frag"))
-		return DIFF_FRAGINFO;
-	if (!strcasecmp(var, "old"))
-		return DIFF_FILE_OLD;
-	if (!strcasecmp(var, "new"))
-		return DIFF_FILE_NEW;
-	if (!strcasecmp(var, "commit"))
-		return DIFF_COMMIT;
-	if (!strcasecmp(var, "whitespace"))
-		return DIFF_WHITESPACE;
-	if (!strcasecmp(var, "func"))
-		return DIFF_FUNCINFO;
-	if (!strcasecmp(var, "oldmoved"))
-		return DIFF_FILE_OLD_MOVED;
-	if (!strcasecmp(var, "oldmovedalternative"))
-		return DIFF_FILE_OLD_MOVED_ALT;
-	if (!strcasecmp(var, "oldmoveddimmed"))
-		return DIFF_FILE_OLD_MOVED_DIM;
-	if (!strcasecmp(var, "oldmovedalternativedimmed"))
-		return DIFF_FILE_OLD_MOVED_ALT_DIM;
-	if (!strcasecmp(var, "newmoved"))
-		return DIFF_FILE_NEW_MOVED;
-	if (!strcasecmp(var, "newmovedalternative"))
-		return DIFF_FILE_NEW_MOVED_ALT;
-	if (!strcasecmp(var, "newmoveddimmed"))
-		return DIFF_FILE_NEW_MOVED_DIM;
-	if (!strcasecmp(var, "newmovedalternativedimmed"))
-		return DIFF_FILE_NEW_MOVED_ALT_DIM;
-	return -1;
+	return LOOKUP_CONFIG(color_diff_slots, var);
 }
 
 static int parse_dirstat_params(struct diff_options *options, const char *params_string,
diff --git a/log-tree.c b/log-tree.c
index d1c0bedf24..093efb477e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -27,6 +27,14 @@ static char decoration_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_BOLD_BLUE,	/* GRAFTED */
 };
 
+static const char *color_decorate_slots[] = {
+	[DECORATION_REF_LOCAL]	= "branch",
+	[DECORATION_REF_REMOTE] = "remoteBranch",
+	[DECORATION_REF_TAG]	= "tag",
+	[DECORATION_REF_STASH]	= "stash",
+	[DECORATION_REF_HEAD]	= "HEAD",
+};
+
 static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)
 {
 	if (want_color(decorate_use_color))
@@ -34,34 +42,9 @@ static const char *decorate_get_color(int decorate_use_color, enum decoration_ty
 	return "";
 }
 
-static int parse_decorate_color_slot(const char *slot)
-{
-	/*
-	 * We're comparing with 'ignore-case' on
-	 * (because config.c sets them all tolower),
-	 * but let's match the letters in the literal
-	 * string values here with how they are
-	 * documented in Documentation/config.txt, for
-	 * consistency.
-	 *
-	 * We love being consistent, don't we?
-	 */
-	if (!strcasecmp(slot, "branch"))
-		return DECORATION_REF_LOCAL;
-	if (!strcasecmp(slot, "remoteBranch"))
-		return DECORATION_REF_REMOTE;
-	if (!strcasecmp(slot, "tag"))
-		return DECORATION_REF_TAG;
-	if (!strcasecmp(slot, "stash"))
-		return DECORATION_REF_STASH;
-	if (!strcasecmp(slot, "HEAD"))
-		return DECORATION_REF_HEAD;
-	return -1;
-}
-
 int parse_decorate_color_config(const char *var, const char *slot_name, const char *value)
 {
-	int slot = parse_decorate_color_slot(slot_name);
+	int slot = LOOKUP_CONFIG(color_decorate_slots, slot_name);
 	if (slot < 0)
 		return 0;
 	if (!value)
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 2/9] grep: keep all colors in an array
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
  2018-05-10 14:19 ` [PATCH 1/9] Add and use generic name->id mapping code for color slot parsing Nguyễn Thái Ngọc Duy
@ 2018-05-10 14:19 ` Nguyễn Thái Ngọc Duy
  2018-05-10 14:19 ` [PATCH 3/9] fsck: factor out msg_id_info[] lazy initialization code Nguyễn Thái Ngọc Duy
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-10 14:19 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This is more inline with how we handle color slots in other code. It
also allows us to get the list of configurable color slots later.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 grep.c | 106 ++++++++++++++++++++++++++-------------------------------
 grep.h |  21 +++++++-----
 2 files changed, 62 insertions(+), 65 deletions(-)

diff --git a/grep.c b/grep.c
index 65b90c10a3..2f7ebe60f6 100644
--- a/grep.c
+++ b/grep.c
@@ -13,6 +13,17 @@ static int grep_source_is_binary(struct grep_source *gs);
 
 static struct grep_opt grep_defaults;
 
+static const char *color_grep_slots[] = {
+	[GREP_COLOR_CONTEXT] = "context",
+	[GREP_COLOR_FILENAME] = "filename",
+	[GREP_COLOR_FUNCTION] = "function",
+	[GREP_COLOR_LINENO] = "lineNumber",
+	[GREP_COLOR_MATCH_CONTEXT] = "matchContext",
+	[GREP_COLOR_MATCH_SELECTED] = "matchSelected",
+	[GREP_COLOR_SELECTED] = "selected",
+	[GREP_COLOR_SEP] = "separator",
+};
+
 static void std_output(struct grep_opt *opt, const void *buf, size_t size)
 {
 	fwrite(buf, size, 1, stdout);
@@ -42,14 +53,14 @@ void init_grep_defaults(void)
 	opt->pathname = 1;
 	opt->max_depth = -1;
 	opt->pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED;
-	color_set(opt->color_context, "");
-	color_set(opt->color_filename, "");
-	color_set(opt->color_function, "");
-	color_set(opt->color_lineno, "");
-	color_set(opt->color_match_context, GIT_COLOR_BOLD_RED);
-	color_set(opt->color_match_selected, GIT_COLOR_BOLD_RED);
-	color_set(opt->color_selected, "");
-	color_set(opt->color_sep, GIT_COLOR_CYAN);
+	color_set(opt->colors[GREP_COLOR_CONTEXT], "");
+	color_set(opt->colors[GREP_COLOR_FILENAME], "");
+	color_set(opt->colors[GREP_COLOR_FUNCTION], "");
+	color_set(opt->colors[GREP_COLOR_LINENO], "");
+	color_set(opt->colors[GREP_COLOR_MATCH_CONTEXT], GIT_COLOR_BOLD_RED);
+	color_set(opt->colors[GREP_COLOR_MATCH_SELECTED], GIT_COLOR_BOLD_RED);
+	color_set(opt->colors[GREP_COLOR_SELECTED], "");
+	color_set(opt->colors[GREP_COLOR_SEP], GIT_COLOR_CYAN);
 	opt->color = -1;
 	opt->output = std_output;
 }
@@ -76,7 +87,7 @@ static int parse_pattern_type_arg(const char *opt, const char *arg)
 int grep_config(const char *var, const char *value, void *cb)
 {
 	struct grep_opt *opt = &grep_defaults;
-	char *color = NULL;
+	const char *slot;
 
 	if (userdiff_config(var, value) < 0)
 		return -1;
@@ -103,32 +114,18 @@ int grep_config(const char *var, const char *value, void *cb)
 
 	if (!strcmp(var, "color.grep"))
 		opt->color = git_config_colorbool(var, value);
-	else if (!strcmp(var, "color.grep.context"))
-		color = opt->color_context;
-	else if (!strcmp(var, "color.grep.filename"))
-		color = opt->color_filename;
-	else if (!strcmp(var, "color.grep.function"))
-		color = opt->color_function;
-	else if (!strcmp(var, "color.grep.linenumber"))
-		color = opt->color_lineno;
-	else if (!strcmp(var, "color.grep.matchcontext"))
-		color = opt->color_match_context;
-	else if (!strcmp(var, "color.grep.matchselected"))
-		color = opt->color_match_selected;
-	else if (!strcmp(var, "color.grep.selected"))
-		color = opt->color_selected;
-	else if (!strcmp(var, "color.grep.separator"))
-		color = opt->color_sep;
-	else if (!strcmp(var, "color.grep.match")) {
-		int rc = 0;
-		if (!value)
-			return config_error_nonbool(var);
-		rc |= color_parse(value, opt->color_match_context);
-		rc |= color_parse(value, opt->color_match_selected);
-		return rc;
-	}
-
-	if (color) {
+	if (!strcmp(var, "color.grep.match")) {
+		if (grep_config("color.grep.matchcontext", value, cb) < 0)
+			return -1;
+		if (grep_config("color.grep.matchselected", value, cb) < 0)
+			return -1;
+	} else if (skip_prefix(var, "color.grep.", &slot)) {
+		int i = LOOKUP_CONFIG(color_grep_slots, slot);
+		char *color;
+
+		if (i < 0)
+			return -1;
+		color = opt->colors[i];
 		if (!value)
 			return config_error_nonbool(var);
 		return color_parse(value, color);
@@ -144,6 +141,7 @@ int grep_config(const char *var, const char *value, void *cb)
 void grep_init(struct grep_opt *opt, const char *prefix)
 {
 	struct grep_opt *def = &grep_defaults;
+	int i;
 
 	memset(opt, 0, sizeof(*opt));
 	opt->prefix = prefix;
@@ -160,14 +158,8 @@ void grep_init(struct grep_opt *opt, const char *prefix)
 	opt->relative = def->relative;
 	opt->output = def->output;
 
-	color_set(opt->color_context, def->color_context);
-	color_set(opt->color_filename, def->color_filename);
-	color_set(opt->color_function, def->color_function);
-	color_set(opt->color_lineno, def->color_lineno);
-	color_set(opt->color_match_context, def->color_match_context);
-	color_set(opt->color_match_selected, def->color_match_selected);
-	color_set(opt->color_selected, def->color_selected);
-	color_set(opt->color_sep, def->color_sep);
+	for (i = 0; i < NR_GREP_COLORS; i++)
+		color_set(opt->colors[i], def->colors[i]);
 }
 
 static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, struct grep_opt *opt)
@@ -1100,12 +1092,12 @@ static void output_sep(struct grep_opt *opt, char sign)
 	if (opt->null_following_name)
 		opt->output(opt, "\0", 1);
 	else
-		output_color(opt, &sign, 1, opt->color_sep);
+		output_color(opt, &sign, 1, opt->colors[GREP_COLOR_SEP]);
 }
 
 static void show_name(struct grep_opt *opt, const char *name)
 {
-	output_color(opt, name, strlen(name), opt->color_filename);
+	output_color(opt, name, strlen(name), opt->colors[GREP_COLOR_FILENAME]);
 	opt->output(opt, opt->null_following_name ? "\0" : "\n", 1);
 }
 
@@ -1372,28 +1364,28 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	} else if (opt->pre_context || opt->post_context || opt->funcbody) {
 		if (opt->last_shown == 0) {
 			if (opt->show_hunk_mark) {
-				output_color(opt, "--", 2, opt->color_sep);
+				output_color(opt, "--", 2, opt->colors[GREP_COLOR_SEP]);
 				opt->output(opt, "\n", 1);
 			}
 		} else if (lno > opt->last_shown + 1) {
-			output_color(opt, "--", 2, opt->color_sep);
+			output_color(opt, "--", 2, opt->colors[GREP_COLOR_SEP]);
 			opt->output(opt, "\n", 1);
 		}
 	}
 	if (opt->heading && opt->last_shown == 0) {
-		output_color(opt, name, strlen(name), opt->color_filename);
+		output_color(opt, name, strlen(name), opt->colors[GREP_COLOR_FILENAME]);
 		opt->output(opt, "\n", 1);
 	}
 	opt->last_shown = lno;
 
 	if (!opt->heading && opt->pathname) {
-		output_color(opt, name, strlen(name), opt->color_filename);
+		output_color(opt, name, strlen(name), opt->colors[GREP_COLOR_FILENAME]);
 		output_sep(opt, sign);
 	}
 	if (opt->linenum) {
 		char buf[32];
 		xsnprintf(buf, sizeof(buf), "%d", lno);
-		output_color(opt, buf, strlen(buf), opt->color_lineno);
+		output_color(opt, buf, strlen(buf), opt->colors[GREP_COLOR_LINENO]);
 		output_sep(opt, sign);
 	}
 	if (opt->color) {
@@ -1403,15 +1395,15 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		int eflags = 0;
 
 		if (sign == ':')
-			match_color = opt->color_match_selected;
+			match_color = opt->colors[GREP_COLOR_MATCH_SELECTED];
 		else
-			match_color = opt->color_match_context;
+			match_color = opt->colors[GREP_COLOR_MATCH_CONTEXT];
 		if (sign == ':')
-			line_color = opt->color_selected;
+			line_color = opt->colors[GREP_COLOR_SELECTED];
 		else if (sign == '-')
-			line_color = opt->color_context;
+			line_color = opt->colors[GREP_COLOR_CONTEXT];
 		else if (sign == '=')
-			line_color = opt->color_function;
+			line_color = opt->colors[GREP_COLOR_FUNCTION];
 		*eol = '\0';
 		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
 			if (match.rm_so == match.rm_eo)
@@ -1818,7 +1810,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 			if (binary_match_only) {
 				opt->output(opt, "Binary file ", 12);
 				output_color(opt, gs->name, strlen(gs->name),
-					     opt->color_filename);
+					     opt->colors[GREP_COLOR_FILENAME]);
 				opt->output(opt, " matches\n", 9);
 				return 1;
 			}
@@ -1892,7 +1884,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 		char buf[32];
 		if (opt->pathname) {
 			output_color(opt, gs->name, strlen(gs->name),
-				     opt->color_filename);
+				     opt->colors[GREP_COLOR_FILENAME]);
 			output_sep(opt, ':');
 		}
 		xsnprintf(buf, sizeof(buf), "%u\n", count);
diff --git a/grep.h b/grep.h
index 399381c908..ed25be271b 100644
--- a/grep.h
+++ b/grep.h
@@ -62,6 +62,18 @@ enum grep_header_field {
 	GREP_HEADER_FIELD_MAX
 };
 
+enum grep_color {
+	GREP_COLOR_CONTEXT,
+	GREP_COLOR_FILENAME,
+	GREP_COLOR_FUNCTION,
+	GREP_COLOR_LINENO,
+	GREP_COLOR_MATCH_CONTEXT,
+	GREP_COLOR_MATCH_SELECTED,
+	GREP_COLOR_SELECTED,
+	GREP_COLOR_SEP,
+	NR_GREP_COLORS
+};
+
 struct grep_pat {
 	struct grep_pat *next;
 	const char *origin;
@@ -155,14 +167,7 @@ struct grep_opt {
 	int funcbody;
 	int extended_regexp_option;
 	int pattern_type_option;
-	char color_context[COLOR_MAXLEN];
-	char color_filename[COLOR_MAXLEN];
-	char color_function[COLOR_MAXLEN];
-	char color_lineno[COLOR_MAXLEN];
-	char color_match_context[COLOR_MAXLEN];
-	char color_match_selected[COLOR_MAXLEN];
-	char color_selected[COLOR_MAXLEN];
-	char color_sep[COLOR_MAXLEN];
+	char colors[NR_GREP_COLORS][COLOR_MAXLEN];
 	unsigned pre_context;
 	unsigned post_context;
 	unsigned last_shown;
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 3/9] fsck: factor out msg_id_info[] lazy initialization code
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
  2018-05-10 14:19 ` [PATCH 1/9] Add and use generic name->id mapping code for color slot parsing Nguyễn Thái Ngọc Duy
  2018-05-10 14:19 ` [PATCH 2/9] grep: keep all colors in an array Nguyễn Thái Ngọc Duy
@ 2018-05-10 14:19 ` Nguyễn Thái Ngọc Duy
  2018-05-11 22:23   ` Eric Sunshine
  2018-05-10 14:19 ` [PATCH 4/9] help: add --config to list all available config Nguyễn Thái Ngọc Duy
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-10 14:19 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This array will be used by some other function than parse_msg_id() in
the following commit. Factor out this prep code so it could be called
from that one.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 fsck.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/fsck.c b/fsck.c
index 9218c2a643..f2534abd44 100644
--- a/fsck.c
+++ b/fsck.c
@@ -84,26 +84,32 @@ static struct {
 };
 #undef MSG_ID
 
-static int parse_msg_id(const char *text)
+static void prepare_msg_ids(void)
 {
 	int i;
 
-	if (!msg_id_info[0].downcased) {
-		/* convert id_string to lower case, without underscores. */
-		for (i = 0; i < FSCK_MSG_MAX; i++) {
-			const char *p = msg_id_info[i].id_string;
-			int len = strlen(p);
-			char *q = xmalloc(len);
-
-			msg_id_info[i].downcased = q;
-			while (*p)
-				if (*p == '_')
-					p++;
-				else
-					*(q)++ = tolower(*(p)++);
-			*q = '\0';
-		}
+	/* convert id_string to lower case, without underscores. */
+	for (i = 0; i < FSCK_MSG_MAX; i++) {
+		const char *p = msg_id_info[i].id_string;
+		int len = strlen(p);
+		char *q = xmalloc(len);
+
+		msg_id_info[i].downcased = q;
+		while (*p)
+			if (*p == '_')
+				p++;
+			else
+				*(q)++ = tolower(*(p)++);
+		*q = '\0';
 	}
+}
+
+static int parse_msg_id(const char *text)
+{
+	int i;
+
+	if (!msg_id_info[0].downcased)
+		prepare_msg_ids();
 
 	for (i = 0; i < FSCK_MSG_MAX; i++)
 		if (!strcmp(text, msg_id_info[i].downcased))
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 4/9] help: add --config to list all available config
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
                   ` (2 preceding siblings ...)
  2018-05-10 14:19 ` [PATCH 3/9] fsck: factor out msg_id_info[] lazy initialization code Nguyễn Thái Ngọc Duy
@ 2018-05-10 14:19 ` Nguyễn Thái Ngọc Duy
  2018-05-11 22:39   ` Eric Sunshine
  2018-05-10 14:19 ` [PATCH 5/9] advice: keep config name in camelCase in advice_config[] Nguyễn Thái Ngọc Duy
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-10 14:19 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Sometimes it helps to list all available config vars so the user can
search for something they want. The config man page can also be used
but it's harder to search if you want to focus on the variable name,
for example.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-help.txt |  5 ++++
 advice.c                   |  9 +++++++
 builtin/branch.c           |  6 +++++
 builtin/clean.c            |  6 +++++
 builtin/commit.c           |  7 ++++++
 builtin/help.c             |  9 +++++++
 diff.c                     |  7 ++++++
 fsck.c                     | 11 +++++++++
 generate-cmdlist.sh        | 20 ++++++++++++++++
 grep.c                     |  7 ++++++
 help.c                     | 48 ++++++++++++++++++++++++++++++++++++++
 help.h                     | 18 ++++++++++++++
 log-tree.c                 |  6 +++++
 13 files changed, 159 insertions(+)

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index a40fc38d8b..83d25d825a 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -45,6 +45,11 @@ OPTIONS
 	When used with `--verbose` print description for all recognized
 	commands.
 
+-c::
+--config::
+	List all available configuration variables. This is a short
+	summary of the list in linkgit:git-config[1].
+
 -g::
 --guides::
 	Prints a list of useful guides on the standard output. This
diff --git a/advice.c b/advice.c
index 406efc183b..b211ebc588 100644
--- a/advice.c
+++ b/advice.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "config.h"
+#include "help.h"
 
 int advice_push_update_rejected = 1;
 int advice_push_non_ff_current = 1;
@@ -84,6 +85,14 @@ int git_default_advice_config(const char *var, const char *value)
 	return 0;
 }
 
+void list_advices(const char *prefix)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(advice_config); i++)
+		printf("%s.%s\n", prefix, advice_config[i].name);
+}
+
 int error_resolve_conflict(const char *me)
 {
 	if (!strcmp(me, "cherry-pick"))
diff --git a/builtin/branch.c b/builtin/branch.c
index b41f332589..23bbdcb301 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -22,6 +22,7 @@
 #include "wt-status.h"
 #include "ref-filter.h"
 #include "worktree.h"
+#include "help.h"
 
 static const char * const builtin_branch_usage[] = {
 	N_("git branch [<options>] [-r | -a] [--merged | --no-merged]"),
@@ -67,6 +68,11 @@ static const char *color_branch_slots[] = {
 static struct string_list output = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
+void list_color_branch_slots(const char *prefix)
+{
+	PRINT_ARRAY(prefix, color_branch_slots);
+}
+
 static int git_branch_config(const char *var, const char *value, void *cb)
 {
 	const char *slot_name;
diff --git a/builtin/clean.c b/builtin/clean.c
index 0ccd95e693..72cea1537e 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -16,6 +16,7 @@
 #include "column.h"
 #include "color.h"
 #include "pathspec.h"
+#include "help.h"
 
 static int force = -1; /* unset */
 static int interactive;
@@ -91,6 +92,11 @@ struct menu_stuff {
 	void *stuff;
 };
 
+void list_color_interactive_slots(const char *prefix)
+{
+	PRINT_ARRAY(prefix, color_interactive_slots);
+}
+
 static int git_clean_config(const char *var, const char *value, void *cb)
 {
 	const char *slot_name;
diff --git a/builtin/commit.c b/builtin/commit.c
index bee5825bd2..41c3c4f5cd 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -32,6 +32,7 @@
 #include "column.h"
 #include "sequencer.h"
 #include "mailmap.h"
+#include "help.h"
 
 static const char * const builtin_commit_usage[] = {
 	N_("git commit [<options>] [--] <pathspec>..."),
@@ -1194,6 +1195,12 @@ static int parse_status_slot(const char *slot)
 	return LOOKUP_CONFIG(color_status_slots, slot);
 }
 
+void list_color_status_slots(const char *prefix)
+{
+	printf("%s.%s\n", prefix, "added");
+	PRINT_ARRAY(prefix, color_status_slots);
+}
+
 static int git_status_config(const char *k, const char *v, void *cb)
 {
 	struct wt_status *s = cb;
diff --git a/builtin/help.c b/builtin/help.c
index 5727fb5e51..a1153cf473 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -36,6 +36,7 @@ static const char *html_path;
 
 static int show_all = 0;
 static int show_guides = 0;
+static int show_config;
 static int verbose;
 static unsigned int colopts;
 static enum help_format help_format = HELP_FORMAT_NONE;
@@ -44,6 +45,7 @@ static struct option builtin_help_options[] = {
 	OPT_BOOL('a', "all", &show_all, N_("print all available commands")),
 	OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")),
 	OPT_BOOL('g', "guides", &show_guides, N_("print list of useful guides")),
+	OPT_BOOL('c', "config", &show_config, N_("print list recognized config variables")),
 	OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN),
 	OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
 			HELP_FORMAT_WEB),
@@ -443,6 +445,13 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 		list_commands(colopts, &main_cmds, &other_cmds);
 	}
 
+	if (show_config) {
+		setup_pager();
+		list_config_help();
+		printf("\n%s\n", _("'git help config' for more information"));
+		return 0;
+	}
+
 	if (show_guides)
 		list_common_guides_help();
 
diff --git a/diff.c b/diff.c
index 6f781a2c57..5cae59ec1a 100644
--- a/diff.c
+++ b/diff.c
@@ -22,6 +22,7 @@
 #include "argv-array.h"
 #include "graph.h"
 #include "packfile.h"
+#include "help.h"
 
 #ifdef NO_FAST_WORKING_DIRECTORY
 #define FAST_WORKING_DIRECTORY 0
@@ -100,6 +101,12 @@ static int parse_diff_color_slot(const char *var)
 	return LOOKUP_CONFIG(color_diff_slots, var);
 }
 
+void list_color_diff_slots(const char *prefix)
+{
+	printf("%s.%s\n", prefix, "plain");
+	PRINT_ARRAY(prefix, color_diff_slots);
+}
+
 static int parse_dirstat_params(struct diff_options *options, const char *params_string,
 				struct strbuf *errmsg)
 {
diff --git a/fsck.c b/fsck.c
index f2534abd44..b9f5e7b3f1 100644
--- a/fsck.c
+++ b/fsck.c
@@ -10,6 +10,7 @@
 #include "utf8.h"
 #include "sha1-array.h"
 #include "decorate.h"
+#include "help.h"
 
 #define FSCK_FATAL -1
 #define FSCK_INFO -2
@@ -118,6 +119,16 @@ static int parse_msg_id(const char *text)
 	return -1;
 }
 
+void list_fsck_msg_ids(const char *prefix)
+{
+	int i;
+
+	prepare_msg_ids();
+	/* TODO: we can do better by producing camelCase names */
+	for (i = 0; i < FSCK_MSG_MAX; i++)
+		printf("%s.%s\n", prefix, msg_id_info[i].downcased);
+}
+
 static int fsck_msg_type(enum fsck_msg_id msg_id,
 	struct fsck_options *options)
 {
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 8d6d8b45ce..25d4516f1f 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -76,6 +76,24 @@ print_command_list () {
 	echo "};"
 }
 
+print_config_list() {
+	cat <<EOF
+static const char *config_name_list[] = {
+EOF
+	grep '^[a-zA-Z].*\..*::$' Documentation/config.txt |
+	grep -v deprecated |
+	sed 's/::$//; s/,  */\n/g' |
+	sort |
+	while read line
+	do
+		echo "	\"$line\","
+	done
+	cat <<EOF
+	NULL,
+};
+EOF
+}
+
 echo "/* Automatically generated by generate-cmdlist.sh */
 struct cmdname_help {
 	const char *name;
@@ -88,3 +106,5 @@ echo
 define_category_names "$1"
 echo
 print_command_list "$1"
+echo
+print_config_list
diff --git a/grep.c b/grep.c
index 2f7ebe60f6..da6dc9af4f 100644
--- a/grep.c
+++ b/grep.c
@@ -7,6 +7,7 @@
 #include "diffcore.h"
 #include "commit.h"
 #include "quote.h"
+#include "help.h"
 
 static int grep_source_load(struct grep_source *gs);
 static int grep_source_is_binary(struct grep_source *gs);
@@ -133,6 +134,12 @@ int grep_config(const char *var, const char *value, void *cb)
 	return 0;
 }
 
+void list_color_grep_slots(const char *prefix)
+{
+	printf("%s.%s\n", prefix, "match");
+	PRINT_ARRAY(prefix, color_grep_slots);
+}
+
 /*
  * Initialize one instance of grep_opt and copy the
  * default values from the template we read the configuration
diff --git a/help.c b/help.c
index abf87205b2..b10c8b0230 100644
--- a/help.c
+++ b/help.c
@@ -409,6 +409,54 @@ void list_common_guides_help(void)
 	putchar('\n');
 }
 
+struct slot_expansion {
+	const char *prefix;
+	const char *placeholder;
+	void (*fn)(const char *prefix);
+	int found;
+};
+
+void list_config_help(void)
+{
+	struct slot_expansion slot_expansions[] = {
+		{ "advice", "*", list_advices },
+		{ "color.branch", "<slot>", list_color_branch_slots },
+		{ "color.decorate", "<slot>", list_color_decorate_slots },
+		{ "color.diff", "<slot>", list_color_diff_slots },
+		{ "color.grep", "<slot>", list_color_grep_slots },
+		{ "color.interactive", "<slot>", list_color_interactive_slots },
+		{ "color.status", "<slot>", list_color_status_slots },
+		{ "fsck", "<msg-id>", list_fsck_msg_ids },
+		{ "receive.fsck", "<msg-id>", list_fsck_msg_ids },
+		{ NULL, NULL, NULL }
+	};
+	const char **p;
+	struct slot_expansion *e;
+
+	for (p = config_name_list; *p; p++) {
+		const char *var = *p;
+
+		for (e = slot_expansions; e->prefix; e++) {
+			struct strbuf sb = STRBUF_INIT;
+
+			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
+			if (strcasecmp(var, sb.buf))
+				continue;
+			e->fn(e->prefix);
+			strbuf_release(&sb);
+			e->found++;
+			break;
+		}
+		if (!e->prefix)
+			puts(var);
+	}
+
+	for (e = slot_expansions; e->prefix; e++)
+		if (!e->found)
+			BUG("slot_expansion %s.%s is not used",
+			    e->prefix, e->placeholder);
+}
+
 void list_all_cmds_help(void)
 {
 	print_cmd_by_category(main_categories);
diff --git a/help.h b/help.h
index 3b38292a1b..d7d1409718 100644
--- a/help.h
+++ b/help.h
@@ -21,6 +21,7 @@ static inline void mput_char(char c, unsigned int num)
 extern void list_common_cmds_help(void);
 extern void list_all_cmds_help(void);
 extern void list_common_guides_help(void);
+extern void list_config_help(void);
 
 extern void list_all_main_cmds(struct string_list *list);
 extern void list_all_other_cmds(struct string_list *list);
@@ -42,4 +43,21 @@ extern void list_commands(unsigned int colopts, struct cmdnames *main_cmds, stru
  * ref to the command, to give suggested "correct" refs.
  */
 extern void help_unknown_ref(const char *ref, const char *cmd, const char *error);
+
+/* These are actually scattered over many C files */
+#define PRINT_ARRAY(prefix, array)  {					\
+		int i;							\
+		for (i = 0; i < ARRAY_SIZE(array); i++)			\
+			if (array[i])					\
+				printf("%s.%s\n", prefix, array[i]);	\
+	}
+void list_advices(const char *prefix);
+void list_color_branch_slots(const char *prefix);
+void list_color_decorate_slots(const char *prefix);
+void list_color_diff_slots(const char *prefix);
+void list_color_grep_slots(const char *prefix);
+void list_color_interactive_slots(const char *prefix);
+void list_color_status_slots(const char *prefix);
+void list_fsck_msg_ids(const char *prefix);
+
 #endif /* HELP_H */
diff --git a/log-tree.c b/log-tree.c
index 093efb477e..19cfebd231 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -12,6 +12,7 @@
 #include "gpg-interface.h"
 #include "sequencer.h"
 #include "line-log.h"
+#include "help.h"
 
 static struct decoration name_decoration = { "object names" };
 static int decoration_loaded;
@@ -42,6 +43,11 @@ static const char *decorate_get_color(int decorate_use_color, enum decoration_ty
 	return "";
 }
 
+void list_color_decorate_slots(const char *prefix)
+{
+	PRINT_ARRAY(prefix, color_decorate_slots);
+}
+
 int parse_decorate_color_config(const char *var, const char *slot_name, const char *value)
 {
 	int slot = LOOKUP_CONFIG(color_decorate_slots, slot_name);
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 5/9] advice: keep config name in camelCase in advice_config[]
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
                   ` (3 preceding siblings ...)
  2018-05-10 14:19 ` [PATCH 4/9] help: add --config to list all available config Nguyễn Thái Ngọc Duy
@ 2018-05-10 14:19 ` Nguyễn Thái Ngọc Duy
  2018-05-10 14:19 ` [PATCH 6/9] am: move advice.amWorkDir parsing back to advice.c Nguyễn Thái Ngọc Duy
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-10 14:19 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

For parsing, we don't really need this because the main config parser
will lowercase everything so we can do exact matching. But this array
now is also used for printing in `git help --config`. Keep camelCase
so we have a nice printout.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 advice.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/advice.c b/advice.c
index b211ebc588..d8ea93637a 100644
--- a/advice.c
+++ b/advice.c
@@ -25,27 +25,27 @@ static struct {
 	const char *name;
 	int *preference;
 } advice_config[] = {
-	{ "pushupdaterejected", &advice_push_update_rejected },
-	{ "pushnonffcurrent", &advice_push_non_ff_current },
-	{ "pushnonffmatching", &advice_push_non_ff_matching },
-	{ "pushalreadyexists", &advice_push_already_exists },
-	{ "pushfetchfirst", &advice_push_fetch_first },
-	{ "pushneedsforce", &advice_push_needs_force },
-	{ "statushints", &advice_status_hints },
-	{ "statusuoption", &advice_status_u_option },
-	{ "commitbeforemerge", &advice_commit_before_merge },
-	{ "resolveconflict", &advice_resolve_conflict },
-	{ "implicitidentity", &advice_implicit_identity },
-	{ "detachedhead", &advice_detached_head },
-	{ "setupstreamfailure", &advice_set_upstream_failure },
-	{ "objectnamewarning", &advice_object_name_warning },
-	{ "rmhints", &advice_rm_hints },
-	{ "addembeddedrepo", &advice_add_embedded_repo },
-	{ "ignoredhook", &advice_ignored_hook },
-	{ "waitingforeditor", &advice_waiting_for_editor },
+	{ "pushUpdateRejected", &advice_push_update_rejected },
+	{ "pushNonFFCurrent", &advice_push_non_ff_current },
+	{ "pushNonFFMatching", &advice_push_non_ff_matching },
+	{ "pushAlreadyExists", &advice_push_already_exists },
+	{ "pushFetchFirst", &advice_push_fetch_first },
+	{ "pushNeedsForce", &advice_push_needs_force },
+	{ "statusHints", &advice_status_hints },
+	{ "statusUoption", &advice_status_u_option },
+	{ "commitBeforeMerge", &advice_commit_before_merge },
+	{ "resolveConflict", &advice_resolve_conflict },
+	{ "implicitIdentity", &advice_implicit_identity },
+	{ "detachedHead", &advice_detached_head },
+	{ "setupStreamFailure", &advice_set_upstream_failure },
+	{ "objectNameWarning", &advice_object_name_warning },
+	{ "rmHints", &advice_rm_hints },
+	{ "addEmbeddedRepo", &advice_add_embedded_repo },
+	{ "ignoredHook", &advice_ignored_hook },
+	{ "waitingForEditor", &advice_waiting_for_editor },
 
 	/* make this an alias for backward compatibility */
-	{ "pushnonfastforward", &advice_push_update_rejected }
+	{ "pushNonFastForward", &advice_push_update_rejected }
 };
 
 void advise(const char *advice, ...)
@@ -76,7 +76,7 @@ int git_default_advice_config(const char *var, const char *value)
 		return 0;
 
 	for (i = 0; i < ARRAY_SIZE(advice_config); i++) {
-		if (strcmp(k, advice_config[i].name))
+		if (strcasecmp(k, advice_config[i].name))
 			continue;
 		*advice_config[i].preference = git_config_bool(var, value);
 		return 0;
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 6/9] am: move advice.amWorkDir parsing back to advice.c
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
                   ` (4 preceding siblings ...)
  2018-05-10 14:19 ` [PATCH 5/9] advice: keep config name in camelCase in advice_config[] Nguyễn Thái Ngọc Duy
@ 2018-05-10 14:19 ` Nguyễn Thái Ngọc Duy
  2018-05-11 22:42   ` Eric Sunshine
  2018-05-10 14:19 ` [PATCH 7/9] completion: drop the hard coded list of config vars Nguyễn Thái Ngọc Duy
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-10 14:19 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

The only benefit from this move (apart from cleaner code) is that
advice.amWorkDir should now show up in `git help --config`. There
should be no regression since advice config is always read by the
git_default_config().

While at there, use advise() like other code. We now get "hint: "
prefix and the output is stderr instead of stdout (which is also the
reason for the test update because stderr is checked in a following
test and the extra advice can fail it).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 advice.c              | 2 ++
 advice.h              | 1 +
 builtin/am.c          | 5 +----
 t/t4254-am-corrupt.sh | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/advice.c b/advice.c
index d8ea93637a..d300491a6f 100644
--- a/advice.c
+++ b/advice.c
@@ -16,6 +16,7 @@ int advice_implicit_identity = 1;
 int advice_detached_head = 1;
 int advice_set_upstream_failure = 1;
 int advice_object_name_warning = 1;
+int advice_amworkdir = 1;
 int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
@@ -39,6 +40,7 @@ static struct {
 	{ "detachedHead", &advice_detached_head },
 	{ "setupStreamFailure", &advice_set_upstream_failure },
 	{ "objectNameWarning", &advice_object_name_warning },
+	{ "amWorkDir", &advice_amworkdir },
 	{ "rmHints", &advice_rm_hints },
 	{ "addEmbeddedRepo", &advice_add_embedded_repo },
 	{ "ignoredHook", &advice_ignored_hook },
diff --git a/advice.h b/advice.h
index 70568fa792..7555385aa5 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_implicit_identity;
 extern int advice_detached_head;
 extern int advice_set_upstream_failure;
 extern int advice_object_name_warning;
+extern int advice_amworkdir;
 extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
diff --git a/builtin/am.c b/builtin/am.c
index 9c82603f70..03e5870c62 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1827,15 +1827,12 @@ static void am_run(struct am_state *state, int resume)
 		}
 
 		if (apply_status) {
-			int advice_amworkdir = 1;
 
 			printf_ln(_("Patch failed at %s %.*s"), msgnum(state),
 				linelen(state->msg), state->msg);
 
-			git_config_get_bool("advice.amworkdir", &advice_amworkdir);
-
 			if (advice_amworkdir)
-				printf_ln(_("Use 'git am --show-current-patch' to see the failed patch"));
+				advise(_("Use 'git am --show-current-patch' to see the failed patch"));
 
 			die_user_resolve(state);
 		}
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
index 168739c721..fd3bdbfe2c 100755
--- a/t/t4254-am-corrupt.sh
+++ b/t/t4254-am-corrupt.sh
@@ -25,7 +25,7 @@ test_expect_success setup '
 #   fatal: unable to write file '(null)' mode 100644: Bad address
 # Also, it had the unwanted side-effect of deleting f.
 test_expect_success 'try to apply corrupted patch' '
-	test_must_fail git am bad-patch.diff 2>actual
+	test_must_fail git -c advice.amWorkDir=false am bad-patch.diff 2>actual
 '
 
 test_expect_success 'compare diagnostic; ensure file is still here' '
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 7/9] completion: drop the hard coded list of config vars
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
                   ` (5 preceding siblings ...)
  2018-05-10 14:19 ` [PATCH 6/9] am: move advice.amWorkDir parsing back to advice.c Nguyễn Thái Ngọc Duy
@ 2018-05-10 14:19 ` Nguyễn Thái Ngọc Duy
  2018-05-11 22:47   ` Eric Sunshine
  2018-05-20 16:01   ` SZEDER Gábor
  2018-05-10 14:19 ` [PATCH 8/9] completion: support case-insensitive config vars just a bit Nguyễn Thái Ngọc Duy
                   ` (4 subsequent siblings)
  11 siblings, 2 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-10 14:19 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

The new help option --config-for-completion is a machine friendlier
version of --config where all the placeholders and wildcards are
dropped, leaving only the good, completable prefixes for
git-completion.bash to consume.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/help.c                         |  13 +-
 contrib/completion/git-completion.bash | 334 +------------------------
 help.c                                 |  30 ++-
 help.h                                 |   2 +-
 4 files changed, 47 insertions(+), 332 deletions(-)

diff --git a/builtin/help.c b/builtin/help.c
index a1153cf473..fbd2b0238a 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -45,7 +45,8 @@ static struct option builtin_help_options[] = {
 	OPT_BOOL('a', "all", &show_all, N_("print all available commands")),
 	OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")),
 	OPT_BOOL('g', "guides", &show_guides, N_("print list of useful guides")),
-	OPT_BOOL('c', "config", &show_config, N_("print list recognized config variables")),
+	OPT_SET_INT('c', "config", &show_config, N_("print list recognized config variables"), 1),
+	OPT_SET_INT_F(0, "config-for-completion", &show_config, "", 2, PARSE_OPT_HIDDEN),
 	OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN),
 	OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
 			HELP_FORMAT_WEB),
@@ -446,9 +447,13 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	}
 
 	if (show_config) {
-		setup_pager();
-		list_config_help();
-		printf("\n%s\n", _("'git help config' for more information"));
+		int for_human = show_config == 1;
+
+		if (for_human)
+			setup_pager();
+		list_config_help(for_human);
+		if (for_human)
+			printf("\n%s\n", _("'git help config' for more information"));
 		return 0;
 	}
 
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f7ca9a4d4f..8d3257c4de 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2074,6 +2074,13 @@ __git_config_get_set_variables ()
 	__git config $config_file --name-only --list
 }
 
+__git_config_vars=
+__git_compute_config_vars ()
+{
+	test -n "$__git_config_vars" ||
+	__git_config_vars="$(git help --config-for-completion | sort | uniq)"
+}
+
 _git_config ()
 {
 	case "$prev" in
@@ -2232,331 +2239,8 @@ _git_config ()
 		return
 		;;
 	esac
-	__gitcomp "
-		add.ignoreErrors
-		advice.amWorkDir
-		advice.commitBeforeMerge
-		advice.detachedHead
-		advice.implicitIdentity
-		advice.pushAlreadyExists
-		advice.pushFetchFirst
-		advice.pushNeedsForce
-		advice.pushNonFFCurrent
-		advice.pushNonFFMatching
-		advice.pushUpdateRejected
-		advice.resolveConflict
-		advice.rmHints
-		advice.statusHints
-		advice.statusUoption
-		advice.ignoredHook
-		alias.
-		am.keepcr
-		am.threeWay
-		apply.ignorewhitespace
-		apply.whitespace
-		branch.autosetupmerge
-		branch.autosetuprebase
-		browser.
-		clean.requireForce
-		color.branch
-		color.branch.current
-		color.branch.local
-		color.branch.plain
-		color.branch.remote
-		color.decorate.HEAD
-		color.decorate.branch
-		color.decorate.remoteBranch
-		color.decorate.stash
-		color.decorate.tag
-		color.diff
-		color.diff.commit
-		color.diff.frag
-		color.diff.func
-		color.diff.meta
-		color.diff.new
-		color.diff.old
-		color.diff.plain
-		color.diff.whitespace
-		color.grep
-		color.grep.context
-		color.grep.filename
-		color.grep.function
-		color.grep.linenumber
-		color.grep.match
-		color.grep.selected
-		color.grep.separator
-		color.interactive
-		color.interactive.error
-		color.interactive.header
-		color.interactive.help
-		color.interactive.prompt
-		color.pager
-		color.showbranch
-		color.status
-		color.status.added
-		color.status.changed
-		color.status.header
-		color.status.localBranch
-		color.status.nobranch
-		color.status.remoteBranch
-		color.status.unmerged
-		color.status.untracked
-		color.status.updated
-		color.ui
-		commit.cleanup
-		commit.gpgSign
-		commit.status
-		commit.template
-		commit.verbose
-		core.abbrev
-		core.askpass
-		core.attributesfile
-		core.autocrlf
-		core.bare
-		core.bigFileThreshold
-		core.checkStat
-		core.commentChar
-		core.compression
-		core.createObject
-		core.deltaBaseCacheLimit
-		core.editor
-		core.eol
-		core.excludesfile
-		core.fileMode
-		core.fsyncobjectfiles
-		core.gitProxy
-		core.hideDotFiles
-		core.hooksPath
-		core.ignoreStat
-		core.ignorecase
-		core.logAllRefUpdates
-		core.loosecompression
-		core.notesRef
-		core.packedGitLimit
-		core.packedGitWindowSize
-		core.packedRefsTimeout
-		core.pager
-		core.precomposeUnicode
-		core.preferSymlinkRefs
-		core.preloadindex
-		core.protectHFS
-		core.protectNTFS
-		core.quotepath
-		core.repositoryFormatVersion
-		core.safecrlf
-		core.sharedRepository
-		core.sparseCheckout
-		core.splitIndex
-		core.sshCommand
-		core.symlinks
-		core.trustctime
-		core.untrackedCache
-		core.warnAmbiguousRefs
-		core.whitespace
-		core.worktree
-		credential.helper
-		credential.useHttpPath
-		credential.username
-		credentialCache.ignoreSIGHUP
-		diff.autorefreshindex
-		diff.external
-		diff.ignoreSubmodules
-		diff.mnemonicprefix
-		diff.noprefix
-		diff.renameLimit
-		diff.renames
-		diff.statGraphWidth
-		diff.submodule
-		diff.suppressBlankEmpty
-		diff.tool
-		diff.wordRegex
-		diff.algorithm
-		difftool.
-		difftool.prompt
-		fetch.recurseSubmodules
-		fetch.unpackLimit
-		format.attach
-		format.cc
-		format.coverLetter
-		format.from
-		format.headers
-		format.numbered
-		format.pretty
-		format.signature
-		format.signoff
-		format.subjectprefix
-		format.suffix
-		format.thread
-		format.to
-		gc.
-		gc.aggressiveDepth
-		gc.aggressiveWindow
-		gc.auto
-		gc.autoDetach
-		gc.autopacklimit
-		gc.logExpiry
-		gc.packrefs
-		gc.pruneexpire
-		gc.reflogexpire
-		gc.reflogexpireunreachable
-		gc.rerereresolved
-		gc.rerereunresolved
-		gc.worktreePruneExpire
-		gitcvs.allbinary
-		gitcvs.commitmsgannotation
-		gitcvs.dbTableNamePrefix
-		gitcvs.dbdriver
-		gitcvs.dbname
-		gitcvs.dbpass
-		gitcvs.dbuser
-		gitcvs.enabled
-		gitcvs.logfile
-		gitcvs.usecrlfattr
-		guitool.
-		gui.blamehistoryctx
-		gui.commitmsgwidth
-		gui.copyblamethreshold
-		gui.diffcontext
-		gui.encoding
-		gui.fastcopyblame
-		gui.matchtrackingbranch
-		gui.newbranchtemplate
-		gui.pruneduringfetch
-		gui.spellingdictionary
-		gui.trustmtime
-		help.autocorrect
-		help.browser
-		help.format
-		http.lowSpeedLimit
-		http.lowSpeedTime
-		http.maxRequests
-		http.minSessions
-		http.noEPSV
-		http.postBuffer
-		http.proxy
-		http.sslCipherList
-		http.sslVersion
-		http.sslCAInfo
-		http.sslCAPath
-		http.sslCert
-		http.sslCertPasswordProtected
-		http.sslKey
-		http.sslVerify
-		http.useragent
-		i18n.commitEncoding
-		i18n.logOutputEncoding
-		imap.authMethod
-		imap.folder
-		imap.host
-		imap.pass
-		imap.port
-		imap.preformattedHTML
-		imap.sslverify
-		imap.tunnel
-		imap.user
-		init.templatedir
-		instaweb.browser
-		instaweb.httpd
-		instaweb.local
-		instaweb.modulepath
-		instaweb.port
-		interactive.singlekey
-		log.date
-		log.decorate
-		log.showroot
-		mailmap.file
-		man.
-		man.viewer
-		merge.
-		merge.conflictstyle
-		merge.log
-		merge.renameLimit
-		merge.renormalize
-		merge.stat
-		merge.tool
-		merge.verbosity
-		mergetool.
-		mergetool.keepBackup
-		mergetool.keepTemporaries
-		mergetool.prompt
-		notes.displayRef
-		notes.rewrite.
-		notes.rewrite.amend
-		notes.rewrite.rebase
-		notes.rewriteMode
-		notes.rewriteRef
-		pack.compression
-		pack.deltaCacheLimit
-		pack.deltaCacheSize
-		pack.depth
-		pack.indexVersion
-		pack.packSizeLimit
-		pack.threads
-		pack.window
-		pack.windowMemory
-		pager.
-		pretty.
-		pull.octopus
-		pull.twohead
-		push.default
-		push.followTags
-		rebase.autosquash
-		rebase.stat
-		receive.autogc
-		receive.denyCurrentBranch
-		receive.denyDeleteCurrent
-		receive.denyDeletes
-		receive.denyNonFastForwards
-		receive.fsckObjects
-		receive.unpackLimit
-		receive.updateserverinfo
-		remote.pushdefault
-		remotes.
-		repack.usedeltabaseoffset
-		rerere.autoupdate
-		rerere.enabled
-		sendemail.
-		sendemail.aliasesfile
-		sendemail.aliasfiletype
-		sendemail.bcc
-		sendemail.cc
-		sendemail.cccmd
-		sendemail.chainreplyto
-		sendemail.confirm
-		sendemail.envelopesender
-		sendemail.from
-		sendemail.identity
-		sendemail.multiedit
-		sendemail.signedoffbycc
-		sendemail.smtpdomain
-		sendemail.smtpencryption
-		sendemail.smtppass
-		sendemail.smtpserver
-		sendemail.smtpserveroption
-		sendemail.smtpserverport
-		sendemail.smtpuser
-		sendemail.suppresscc
-		sendemail.suppressfrom
-		sendemail.thread
-		sendemail.to
-		sendemail.tocmd
-		sendemail.validate
-		sendemail.smtpbatchsize
-		sendemail.smtprelogindelay
-		showbranch.default
-		status.relativePaths
-		status.showUntrackedFiles
-		status.submodulesummary
-		submodule.
-		tar.umask
-		transfer.unpackLimit
-		url.
-		user.email
-		user.name
-		user.signingkey
-		web.browser
-		branch. remote.
-	"
+	__git_compute_config_vars
+	__gitcomp "$__git_config_vars"
 }
 
 _git_remote ()
diff --git a/help.c b/help.c
index b10c8b0230..a4157a3512 100644
--- a/help.c
+++ b/help.c
@@ -416,7 +416,7 @@ struct slot_expansion {
 	int found;
 };
 
-void list_config_help(void)
+void list_config_help(int for_human)
 {
 	struct slot_expansion slot_expansions[] = {
 		{ "advice", "*", list_advices },
@@ -435,6 +435,7 @@ void list_config_help(void)
 
 	for (p = config_name_list; *p; p++) {
 		const char *var = *p;
+		const char *wildcard, *tag, *cut;
 
 		for (e = slot_expansions; e->prefix; e++) {
 			struct strbuf sb = STRBUF_INIT;
@@ -447,8 +448,33 @@ void list_config_help(void)
 			e->found++;
 			break;
 		}
-		if (!e->prefix)
+		if (e->prefix)
+			continue;
+		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);
 	}
 
 	for (e = slot_expansions; e->prefix; e++)
diff --git a/help.h b/help.h
index d7d1409718..04df559c64 100644
--- a/help.h
+++ b/help.h
@@ -21,7 +21,7 @@ static inline void mput_char(char c, unsigned int num)
 extern void list_common_cmds_help(void);
 extern void list_all_cmds_help(void);
 extern void list_common_guides_help(void);
-extern void list_config_help(void);
+extern void list_config_help(int for_human);
 
 extern void list_all_main_cmds(struct string_list *list);
 extern void list_all_other_cmds(struct string_list *list);
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 8/9] completion: support case-insensitive config vars just a bit
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
                   ` (6 preceding siblings ...)
  2018-05-10 14:19 ` [PATCH 7/9] completion: drop the hard coded list of config vars Nguyễn Thái Ngọc Duy
@ 2018-05-10 14:19 ` Nguyễn Thái Ngọc Duy
  2018-05-20 16:33   ` SZEDER Gábor
  2018-05-10 14:19 ` [PATCH 9/9] log-tree: allow to customize 'grafted' color Nguyễn Thái Ngọc Duy
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-10 14:19 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

config variable names are technically case-insensitive while this case
construct is by default case-sensitive. Moving it to case-insensitive
could be a lot more work. Instead let's just accept the form that is
more likely to show up in practice.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 contrib/completion/git-completion.bash | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8d3257c4de..e7829b5b24 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2084,7 +2084,7 @@ __git_compute_config_vars ()
 _git_config ()
 {
 	case "$prev" in
-	branch.*.remote|branch.*.pushremote)
+	branch.*.remote|branch.*.push[Rr]emote)
 		__gitcomp_nl "$(__git_remotes)"
 		return
 		;;
@@ -2096,7 +2096,7 @@ _git_config ()
 		__gitcomp "false true preserve interactive"
 		return
 		;;
-	remote.pushdefault)
+	remote.push[Dd]efault)
 		__gitcomp_nl "$(__git_remotes)"
 		return
 		;;
@@ -2162,7 +2162,7 @@ _git_config ()
 		__gitcomp "$__git_send_email_suppresscc_options"
 		return
 		;;
-	sendemail.transferencoding)
+	sendemail.transfer[Ee]ncoding)
 		__gitcomp "7bit 8bit quoted-printable base64"
 		return
 		;;
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 9/9] log-tree: allow to customize 'grafted' color
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
                   ` (7 preceding siblings ...)
  2018-05-10 14:19 ` [PATCH 8/9] completion: support case-insensitive config vars just a bit Nguyễn Thái Ngọc Duy
@ 2018-05-10 14:19 ` Nguyễn Thái Ngọc Duy
  2018-05-10 14:22 ` [PATCH 0/9] completion: avoid hard coding config var list Duy Nguyen
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-10 14:19 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Commit 76f5df305b (log: decorate grafted commits with "grafted" -
2011-08-18) lets us decorate grafted commits but I forgot about the
color.decorate.* config.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt | 3 ++-
 log-tree.c               | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 91f7eaed7b..c63d66906d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1138,7 +1138,8 @@ color.diff.<slot>::
 color.decorate.<slot>::
 	Use customized color for 'git log --decorate' output.  `<slot>` is one
 	of `branch`, `remoteBranch`, `tag`, `stash` or `HEAD` for local
-	branches, remote-tracking branches, tags, stash and HEAD, respectively.
+	branches, remote-tracking branches, tags, stash and HEAD, respectively
+	and `grafted` for grafted commits.
 
 color.grep::
 	When set to `always`, always highlight matches.  When `false` (or
diff --git a/log-tree.c b/log-tree.c
index 19cfebd231..414dbce0dd 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -34,6 +34,7 @@ static const char *color_decorate_slots[] = {
 	[DECORATION_REF_TAG]	= "tag",
 	[DECORATION_REF_STASH]	= "stash",
 	[DECORATION_REF_HEAD]	= "HEAD",
+	[DECORATION_GRAFTED]	= "grafted",
 };
 
 static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/9] completion: avoid hard coding config var list
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
                   ` (8 preceding siblings ...)
  2018-05-10 14:19 ` [PATCH 9/9] log-tree: allow to customize 'grafted' color Nguyễn Thái Ngọc Duy
@ 2018-05-10 14:22 ` Duy Nguyen
  2018-05-11  6:00 ` Junio C Hamano
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
  11 siblings, 0 replies; 36+ messages in thread
From: Duy Nguyen @ 2018-05-10 14:22 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Nguyễn Thái Ngọc Duy

On Thu, May 10, 2018 at 4:19 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> (on top of nd/command-list)

Oh.. and it does make use of C99 designated initializer feature. I
could go with out it, but since git-clean has used it for some time
without anybody complaining, I figure I should take advantage of it.
-- 
Duy

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/9] Add and use generic name->id mapping code for color slot parsing
  2018-05-10 14:19 ` [PATCH 1/9] Add and use generic name->id mapping code for color slot parsing Nguyễn Thái Ngọc Duy
@ 2018-05-10 17:16   ` Stefan Beller
  2018-05-11  7:22     ` Duy Nguyen
  0 siblings, 1 reply; 36+ messages in thread
From: Stefan Beller @ 2018-05-10 17:16 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git

On Thu, May 10, 2018 at 7:19 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:

>  7 files changed, 82 insertions(+), 112 deletions(-)

Nice!


>
> +static const char *color_branch_slots[] = {
> +       [BRANCH_COLOR_RESET]    = "reset",

In 512f41cfac5 (clean.c: use designated initializer, 2017-07-14)
we thought we'll do it once and see if anyone complains
(and it shipped v2.15.0, 2017-10-29), and so far
nobody complained half a year later. So designated initializers
are all good now? Do we want to mention this decision in the
commit message?

If so, the patch looks good!
Thanks,
Stefan

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/9] completion: avoid hard coding config var list
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
                   ` (9 preceding siblings ...)
  2018-05-10 14:22 ` [PATCH 0/9] completion: avoid hard coding config var list Duy Nguyen
@ 2018-05-11  6:00 ` Junio C Hamano
  2018-05-11  6:24   ` Duy Nguyen
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
  11 siblings, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2018-05-11  6:00 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> While at there, this config list is also available to the user via
> `git help --config` if you can't remember the exact config name you
> want and don't want to go through that big git-config man page.

Makes a reader anticipate a new user of levenshtein code, perhaps
;-)



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/9] completion: avoid hard coding config var list
  2018-05-11  6:00 ` Junio C Hamano
@ 2018-05-11  6:24   ` Duy Nguyen
  0 siblings, 0 replies; 36+ messages in thread
From: Duy Nguyen @ 2018-05-11  6:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

On Fri, May 11, 2018 at 8:00 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> While at there, this config list is also available to the user via
>> `git help --config` if you can't remember the exact config name you
>> want and don't want to go through that big git-config man page.
>
> Makes a reader anticipate a new user of levenshtein code, perhaps
> ;-)

Why would you go and write that for? Now I want to do it :-) Yeah I
think config name typo has been a common complaint (including from
me).
-- 
Duy

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/9] Add and use generic name->id mapping code for color slot parsing
  2018-05-10 17:16   ` Stefan Beller
@ 2018-05-11  7:22     ` Duy Nguyen
  0 siblings, 0 replies; 36+ messages in thread
From: Duy Nguyen @ 2018-05-11  7:22 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git

On Thu, May 10, 2018 at 7:16 PM, Stefan Beller <sbeller@google.com> wrote:
> On Thu, May 10, 2018 at 7:19 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>
>>  7 files changed, 82 insertions(+), 112 deletions(-)
>
> Nice!
>
>
>>
>> +static const char *color_branch_slots[] = {
>> +       [BRANCH_COLOR_RESET]    = "reset",
>
> In 512f41cfac5 (clean.c: use designated initializer, 2017-07-14)
> we thought we'll do it once and see if anyone complains
> (and it shipped v2.15.0, 2017-10-29), and so far
> nobody complained half a year later. So designated initializers
> are all good now?

I don't know :) but it's worth pushing. I don't think this series
would land on the next release, which gives people a couple more
months to complain about C99. It does save me time and if it causes
problem, removing designated initializers is trivial

> Do we want to mention this decision in the commit message?
>
> If so, the patch looks good!
> Thanks,
> Stefan
-- 
Duy

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 3/9] fsck: factor out msg_id_info[] lazy initialization code
  2018-05-10 14:19 ` [PATCH 3/9] fsck: factor out msg_id_info[] lazy initialization code Nguyễn Thái Ngọc Duy
@ 2018-05-11 22:23   ` Eric Sunshine
  0 siblings, 0 replies; 36+ messages in thread
From: Eric Sunshine @ 2018-05-11 22:23 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: Git List

On Thu, May 10, 2018 at 10:19 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
wrote:
> This array will be used by some other function than parse_msg_id() in
> the following commit. Factor out this prep code so it could be called
> from that one.

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/fsck.c b/fsck.c
> @@ -84,26 +84,32 @@ static struct {
> -static int parse_msg_id(const char *text)
> +static void prepare_msg_ids(void)
>   {
> -       if (!msg_id_info[0].downcased) {
> -               /* convert id_string to lower case, without underscores.
*/
> -               for (i = 0; i < FSCK_MSG_MAX; i++) {
> -                       [...]
> -               }
> +       /* convert id_string to lower case, without underscores. */
> +       for (i = 0; i < FSCK_MSG_MAX; i++) {
> +               [...]
>          }
> +}
> +
> +static int parse_msg_id(const char *text)
> +{
> +       if (!msg_id_info[0].downcased)
> +               prepare_msg_ids();

If you move the "if (!msg_id_info...)" conditional into the new
parpare_msg_ids() function, then it becomes self-contained; it takes care
of avoiding double-initialization so callers don't have to worry or know
about it. (Doing so would also make the diff less noisy.)

Not at all worth a re-roll.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 4/9] help: add --config to list all available config
  2018-05-10 14:19 ` [PATCH 4/9] help: add --config to list all available config Nguyễn Thái Ngọc Duy
@ 2018-05-11 22:39   ` Eric Sunshine
  0 siblings, 0 replies; 36+ messages in thread
From: Eric Sunshine @ 2018-05-11 22:39 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: Git List

On Thu, May 10, 2018 at 10:20 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
wrote:
> Sometimes it helps to list all available config vars so the user can
> search for something they want. The config man page can also be used
> but it's harder to search if you want to focus on the variable name,
> for example.

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/builtin/help.c b/builtin/help.c
> @@ -44,6 +45,7 @@ static struct option builtin_help_options[] = {
>          OPT_BOOL('g', "guides", &show_guides, N_("print list of useful
guides")),
> +       OPT_BOOL('c', "config", &show_config, N_("print list recognized
config variables")),

s/list/& of/

Though, simpler might be better: "print all configuration variable names"

> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> @@ -76,6 +76,24 @@ print_command_list () {
> +print_config_list() {
> +       cat <<EOF
> +static const char *config_name_list[] = {
> +EOF
> +       grep '^[a-zA-Z].*\..*::$' Documentation/config.txt |
> +       grep -v deprecated |
> +       sed 's/::$//; s/,  */\n/g' |

Nit: "grep -v" and "sed" could be combined:

     sed '/deprecated/d; s/::$//; s/,  */\n/g' |

> +       sort |
> +       while read line
> +       do
> +               echo "  \"$line\","
> +       done
> +       cat <<EOF
> +       NULL,
> +};
> +EOF
> diff --git a/help.c b/help.c
> @@ -409,6 +409,54 @@ void list_common_guides_help(void)
> +void list_config_help(void)
> +{
> +       [...]
> +       for (p = config_name_list; *p; p++) {
> +               const char *var = *p;
> +
> +               for (e = slot_expansions; e->prefix; e++) {
> +                       struct strbuf sb = STRBUF_INIT;
> +
> +                       strbuf_addf(&sb, "%s.%s", e->prefix,
e->placeholder);
> +                       if (strcasecmp(var, sb.buf))
> +                               continue;

Isn't this "continue" leaking the strbuf? It seems that it would be easier
to declare the strbuf once outside the loop, strbuf_reset() it at the top
of the loop, and finally strbuf_release() it after the loop exits.

> +                       e->fn(e->prefix);
> +                       strbuf_release(&sb);
> +                       e->found++;
> +                       break;
> +               }
> +               if (!e->prefix)
> +                       puts(var);
> +       }

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 6/9] am: move advice.amWorkDir parsing back to advice.c
  2018-05-10 14:19 ` [PATCH 6/9] am: move advice.amWorkDir parsing back to advice.c Nguyễn Thái Ngọc Duy
@ 2018-05-11 22:42   ` Eric Sunshine
  0 siblings, 0 replies; 36+ messages in thread
From: Eric Sunshine @ 2018-05-11 22:42 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: Git List

On Thu, May 10, 2018 at 10:19 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
wrote:
> The only benefit from this move (apart from cleaner code) is that
> advice.amWorkDir should now show up in `git help --config`. There
> should be no regression since advice config is always read by the
> git_default_config().

> While at there, use advise() like other code. We now get "hint: "
> prefix and the output is stderr instead of stdout (which is also the
> reason for the test update because stderr is checked in a following
> test and the extra advice can fail it).

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/builtin/am.c b/builtin/am.c
> @@ -1827,15 +1827,12 @@ static void am_run(struct am_state *state, int
resume)
>                  if (apply_status) {
> -                       int advice_amworkdir = 1;


Nit: Maybe also delete the blank line following the removed declaration...

>                          printf_ln(_("Patch failed at %s %.*s"),
msgnum(state),
>                                  linelen(state->msg), state->msg);

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 7/9] completion: drop the hard coded list of config vars
  2018-05-10 14:19 ` [PATCH 7/9] completion: drop the hard coded list of config vars Nguyễn Thái Ngọc Duy
@ 2018-05-11 22:47   ` Eric Sunshine
  2018-05-20 16:01   ` SZEDER Gábor
  1 sibling, 0 replies; 36+ messages in thread
From: Eric Sunshine @ 2018-05-11 22:47 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: Git List

On Thu, May 10, 2018 at 10:19 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
wrote:
> The new help option --config-for-completion is a machine friendlier
> version of --config where all the placeholders and wildcards are
> dropped, leaving only the good, completable prefixes for
> git-completion.bash to consume.

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/builtin/help.c b/builtin/help.c
> @@ -446,9 +447,13 @@ int cmd_help(int argc, const char **argv, const char
*prefix)
> +               int for_human = show_config == 1;
> +
> +               if (for_human)
> +                       setup_pager();
> +               list_config_help(for_human);
> +               if (for_human)
> +                       printf("\n%s\n", _("'git help config' for more
information"));

Simpler to read, perhaps:

     if (!for_human)
         list_config_help(0);
     else {
         setup_pager();
         list_config_help(1);
         printf(...);
     }

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 7/9] completion: drop the hard coded list of config vars
  2018-05-10 14:19 ` [PATCH 7/9] completion: drop the hard coded list of config vars Nguyễn Thái Ngọc Duy
  2018-05-11 22:47   ` Eric Sunshine
@ 2018-05-20 16:01   ` SZEDER Gábor
  1 sibling, 0 replies; 36+ messages in thread
From: SZEDER Gábor @ 2018-05-20 16:01 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: SZEDER Gábor, git

> The new help option --config-for-completion is a machine friendlier
> version of --config where all the placeholders and wildcards are
> dropped, leaving only the good, completable prefixes for
> git-completion.bash to consume.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/help.c                         |  13 +-
>  contrib/completion/git-completion.bash | 334 +------------------------
>  help.c                                 |  30 ++-
>  help.h                                 |   2 +-
>  4 files changed, 47 insertions(+), 332 deletions(-)

Oh, this diffstat is fantastic! :)

I'm glad to see that enormous hardcoded list go.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 8/9] completion: support case-insensitive config vars just a bit
  2018-05-10 14:19 ` [PATCH 8/9] completion: support case-insensitive config vars just a bit Nguyễn Thái Ngọc Duy
@ 2018-05-20 16:33   ` SZEDER Gábor
  0 siblings, 0 replies; 36+ messages in thread
From: SZEDER Gábor @ 2018-05-20 16:33 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: SZEDER Gábor, git

> config variable names are technically case-insensitive while this case
> construct is by default case-sensitive. Moving it to case-insensitive
> could be a lot more work.

Bash v4 supports the case modification expansion in the form of
${prev,,}.  Alas OSX (or older LTS/Enterprise Linux releases) ships
Bash v3.2, which doesn't yet support this syntax, and there is ZSH,
which doesn't understand it either (though I suspect it has its own
weird syntax for case modification).  Supporting them could look
something like this:

  local varname
  
  if [ "${BASH_VERSINFO[0]:-0}" -ge 4 ]; then
          varname="${prev,,}"
  else
          varname="$(echo "$prev" |tr A-Z a-z)"
  fi
  
  case "$varname" in
  <....>

Not pretty, but I think the additional complexity and overhead is
acceptable.  Yeah, on some platforms/shells we would run an extra
command substitution and an external command, but I suppose on those
platforms the overhead of the extra processes is not that critical.
And where this additional overhead matters the most is Windows, but
luckily Git for Windows ships with Bash v4.


> Instead let's just accept the form that is
> more likely to show up in practice.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 8d3257c4de..e7829b5b24 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2084,7 +2084,7 @@ __git_compute_config_vars ()
>  _git_config ()
>  {
>  	case "$prev" in
> -	branch.*.remote|branch.*.pushremote)
> +	branch.*.remote|branch.*.push[Rr]emote)
>  		__gitcomp_nl "$(__git_remotes)"
>  		return
>  		;;
> @@ -2096,7 +2096,7 @@ _git_config ()
>  		__gitcomp "false true preserve interactive"
>  		return
>  		;;
> -	remote.pushdefault)
> +	remote.push[Dd]efault)
>  		__gitcomp_nl "$(__git_remotes)"
>  		return
>  		;;
> @@ -2162,7 +2162,7 @@ _git_config ()
>  		__gitcomp "$__git_send_email_suppresscc_options"
>  		return
>  		;;
> -	sendemail.transferencoding)
> +	sendemail.transfer[Ee]ncoding)
>  		__gitcomp "7bit 8bit quoted-printable base64"
>  		return
>  		;;

'git help config' shows 'color.showBranch' and
'sendemail.aliasesfiletype' in camelCase as well.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v2 00/11] completion: avoid hard coding config var list
  2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
                   ` (10 preceding siblings ...)
  2018-05-11  6:00 ` Junio C Hamano
@ 2018-05-26 13:55 ` Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 01/11] Add and use generic name->id mapping code for color slot parsing Nguyễn Thái Ngọc Duy
                     ` (10 more replies)
  11 siblings, 11 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

v2 changes:

- based on 'next'
- C99 is now mentioned in the commit message
- fixed Eric's comments
- the old 8/9 partial case-insensitive support patch is replaced
  with Szeder's version.
  
  Szeder I claimed authorship because I wrote the commit message which
  may not be what you like. If you want to claim it instead, I'll be
  glad to resend.

- There is no levenshtein support yet. But the code is updated to keep
  this config list in memory instead of printf'ing directly to make it
  easier in the future to do that.

Nguyễn Thái Ngọc Duy (11):
  Add and use generic name->id mapping code for color slot parsing
  grep: keep all colors in an array
  fsck: factor out msg_id_info[] lazy initialization code
  help: add --config to list all available config
  fsck: produce camelCase config key names
  advice: keep config name in camelCase in advice_config[]
  am: move advice.amWorkDir parsing back to advice.c
  completion: drop the hard coded list of config vars
  completion: keep other config var completion in camelCase
  completion: support case-insensitive config vars
  log-tree: allow to customize 'grafted' color

 Documentation/config.txt               |   3 +-
 Documentation/git-help.txt             |   5 +
 advice.c                               |  53 ++--
 advice.h                               |   1 +
 builtin/am.c                           |   6 +-
 builtin/branch.c                       |  29 +-
 builtin/clean.c                        |  29 +-
 builtin/commit.c                       |  36 +--
 builtin/help.c                         |  16 ++
 config.c                               |  13 +
 config.h                               |   4 +
 contrib/completion/git-completion.bash | 357 ++-----------------------
 diff.c                                 |  56 ++--
 fsck.c                                 |  68 +++--
 generate-cmdlist.sh                    |  19 ++
 grep.c                                 | 109 ++++----
 grep.h                                 |  21 +-
 help.c                                 |  84 ++++++
 help.h                                 |  45 +++-
 log-tree.c                             |  37 +--
 t/t4254-am-corrupt.sh                  |   2 +-
 21 files changed, 440 insertions(+), 553 deletions(-)

-- 
2.17.0.705.g3525833791


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v2 01/11] Add and use generic name->id mapping code for color slot parsing
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
@ 2018-05-26 13:55   ` Nguyễn Thái Ngọc Duy
  2018-05-29 17:41     ` Stefan Beller
  2018-05-26 13:55   ` [PATCH v2 02/11] grep: keep all colors in an array Nguyễn Thái Ngọc Duy
                     ` (9 subsequent siblings)
  10 siblings, 1 reply; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

Instead of hard coding the name-to-id mapping in C code, keep it in an
array and use a common function to do the parsing. This reduces code
and also allows us to list all possible color slots later.

This starts using C99 designated initializers more for convenience
(the first designated initializers have been introduced in builtin/clean.c
for some time without complaints)

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/branch.c | 28 +++++++++----------------
 builtin/clean.c  | 28 +++++++++----------------
 builtin/commit.c | 33 ++++++++++++++----------------
 config.c         | 13 ++++++++++++
 config.h         |  4 ++++
 diff.c           | 53 +++++++++++++++++++-----------------------------
 log-tree.c       | 35 ++++++++------------------------
 7 files changed, 82 insertions(+), 112 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index f85d3de531..09232576b6 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -55,26 +55,18 @@ enum color_branch {
 	BRANCH_COLOR_UPSTREAM = 5
 };
 
+static const char *color_branch_slots[] = {
+	[BRANCH_COLOR_RESET]	= "reset",
+	[BRANCH_COLOR_PLAIN]	= "plain",
+	[BRANCH_COLOR_REMOTE]	= "remote",
+	[BRANCH_COLOR_LOCAL]	= "local",
+	[BRANCH_COLOR_CURRENT]	= "current",
+	[BRANCH_COLOR_UPSTREAM] = "upstream",
+};
+
 static struct string_list output = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
-static int parse_branch_color_slot(const char *slot)
-{
-	if (!strcasecmp(slot, "plain"))
-		return BRANCH_COLOR_PLAIN;
-	if (!strcasecmp(slot, "reset"))
-		return BRANCH_COLOR_RESET;
-	if (!strcasecmp(slot, "remote"))
-		return BRANCH_COLOR_REMOTE;
-	if (!strcasecmp(slot, "local"))
-		return BRANCH_COLOR_LOCAL;
-	if (!strcasecmp(slot, "current"))
-		return BRANCH_COLOR_CURRENT;
-	if (!strcasecmp(slot, "upstream"))
-		return BRANCH_COLOR_UPSTREAM;
-	return -1;
-}
-
 static int git_branch_config(const char *var, const char *value, void *cb)
 {
 	const char *slot_name;
@@ -86,7 +78,7 @@ static int git_branch_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (skip_prefix(var, "color.branch.", &slot_name)) {
-		int slot = parse_branch_color_slot(slot_name);
+		int slot = LOOKUP_CONFIG(color_branch_slots, slot_name);
 		if (slot < 0)
 			return 0;
 		if (!value)
diff --git a/builtin/clean.c b/builtin/clean.c
index fad533a0a7..0ccd95e693 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -42,6 +42,15 @@ enum color_clean {
 	CLEAN_COLOR_ERROR = 5
 };
 
+static const char *color_interactive_slots[] = {
+	[CLEAN_COLOR_ERROR]  = "error",
+	[CLEAN_COLOR_HEADER] = "header",
+	[CLEAN_COLOR_HELP]   = "help",
+	[CLEAN_COLOR_PLAIN]  = "plain",
+	[CLEAN_COLOR_PROMPT] = "prompt",
+	[CLEAN_COLOR_RESET]  = "reset",
+};
+
 static int clean_use_color = -1;
 static char clean_colors[][COLOR_MAXLEN] = {
 	[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
@@ -82,23 +91,6 @@ struct menu_stuff {
 	void *stuff;
 };
 
-static int parse_clean_color_slot(const char *var)
-{
-	if (!strcasecmp(var, "reset"))
-		return CLEAN_COLOR_RESET;
-	if (!strcasecmp(var, "plain"))
-		return CLEAN_COLOR_PLAIN;
-	if (!strcasecmp(var, "prompt"))
-		return CLEAN_COLOR_PROMPT;
-	if (!strcasecmp(var, "header"))
-		return CLEAN_COLOR_HEADER;
-	if (!strcasecmp(var, "help"))
-		return CLEAN_COLOR_HELP;
-	if (!strcasecmp(var, "error"))
-		return CLEAN_COLOR_ERROR;
-	return -1;
-}
-
 static int git_clean_config(const char *var, const char *value, void *cb)
 {
 	const char *slot_name;
@@ -113,7 +105,7 @@ static int git_clean_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (skip_prefix(var, "color.interactive.", &slot_name)) {
-		int slot = parse_clean_color_slot(slot_name);
+		int slot = LOOKUP_CONFIG(color_interactive_slots, slot_name);
 		if (slot < 0)
 			return 0;
 		if (!value)
diff --git a/builtin/commit.c b/builtin/commit.c
index a842fea666..2b43a6c48a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -66,6 +66,18 @@ N_("If you wish to skip this commit, use:\n"
 "Then \"git cherry-pick --continue\" will resume cherry-picking\n"
 "the remaining commits.\n");
 
+static const char *color_status_slots[] = {
+	[WT_STATUS_HEADER]	  = "header",
+	[WT_STATUS_UPDATED]	  = "updated",
+	[WT_STATUS_CHANGED]	  = "changed",
+	[WT_STATUS_UNTRACKED]	  = "untracked",
+	[WT_STATUS_NOBRANCH]	  = "noBranch",
+	[WT_STATUS_UNMERGED]	  = "unmerged",
+	[WT_STATUS_LOCAL_BRANCH]  = "localBranch",
+	[WT_STATUS_REMOTE_BRANCH] = "remoteBranch",
+	[WT_STATUS_ONBRANCH]	  = "branch",
+};
+
 static const char *use_message_buffer;
 static struct lock_file index_lock; /* real index */
 static struct lock_file false_lock; /* used only for partial commits */
@@ -1185,25 +1197,10 @@ static int dry_run_commit(int argc, const char **argv, const char *prefix,
 
 static int parse_status_slot(const char *slot)
 {
-	if (!strcasecmp(slot, "header"))
-		return WT_STATUS_HEADER;
-	if (!strcasecmp(slot, "branch"))
-		return WT_STATUS_ONBRANCH;
-	if (!strcasecmp(slot, "updated") || !strcasecmp(slot, "added"))
+	if (!strcasecmp(slot, "added"))
 		return WT_STATUS_UPDATED;
-	if (!strcasecmp(slot, "changed"))
-		return WT_STATUS_CHANGED;
-	if (!strcasecmp(slot, "untracked"))
-		return WT_STATUS_UNTRACKED;
-	if (!strcasecmp(slot, "nobranch"))
-		return WT_STATUS_NOBRANCH;
-	if (!strcasecmp(slot, "unmerged"))
-		return WT_STATUS_UNMERGED;
-	if (!strcasecmp(slot, "localBranch"))
-		return WT_STATUS_LOCAL_BRANCH;
-	if (!strcasecmp(slot, "remoteBranch"))
-		return WT_STATUS_REMOTE_BRANCH;
-	return -1;
+
+	return LOOKUP_CONFIG(color_status_slots, slot);
 }
 
 static int git_status_config(const char *k, const char *v, void *cb)
diff --git a/config.c b/config.c
index fbbf0f8e9f..a0a6ae1980 100644
--- a/config.c
+++ b/config.c
@@ -3245,3 +3245,16 @@ enum config_scope current_config_scope(void)
 	else
 		return current_parsing_scope;
 }
+
+int lookup_config(const char **mapping, int nr_mapping, const char *var)
+{
+	int i;
+
+	for (i = 0; i < nr_mapping; i++) {
+		const char *name = mapping[i];
+
+		if (name && !strcasecmp(var, name))
+			return i;
+	}
+	return -1;
+}
diff --git a/config.h b/config.h
index cdac2fc73e..626d4654bd 100644
--- a/config.h
+++ b/config.h
@@ -257,4 +257,8 @@ struct key_value_info {
 extern NORETURN void git_die_config(const char *key, const char *err, ...) __attribute__((format(printf, 2, 3)));
 extern NORETURN void git_die_config_linenr(const char *key, const char *filename, int linenr);
 
+#define LOOKUP_CONFIG(mapping, var) \
+	lookup_config(mapping, ARRAY_SIZE(mapping), var)
+int lookup_config(const char **mapping, int nr_mapping, const char *var);
+
 #endif /* CONFIG_H */
diff --git a/diff.c b/diff.c
index efa6c1b82f..2e5e1404a5 100644
--- a/diff.c
+++ b/diff.c
@@ -69,6 +69,25 @@ static char diff_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_FAINT_ITALIC,	/* NEW_MOVED_ALTERNATIVE_DIM */
 };
 
+static const char *color_diff_slots[] = {
+	[DIFF_CONTEXT]		      = "context",
+	[DIFF_METAINFO]		      = "meta",
+	[DIFF_FRAGINFO]		      = "frag",
+	[DIFF_FILE_OLD]		      = "old",
+	[DIFF_FILE_NEW]		      = "new",
+	[DIFF_COMMIT]		      = "commit",
+	[DIFF_WHITESPACE]	      = "whitespace",
+	[DIFF_FUNCINFO]		      = "func",
+	[DIFF_FILE_OLD_MOVED]	      = "oldMoved",
+	[DIFF_FILE_OLD_MOVED_ALT]     = "oldMovedAlternative",
+	[DIFF_FILE_OLD_MOVED_DIM]     = "oldMovedDimmed",
+	[DIFF_FILE_OLD_MOVED_ALT_DIM] = "oldMovedAlternativeDimmed",
+	[DIFF_FILE_NEW_MOVED]	      = "newMoved",
+	[DIFF_FILE_NEW_MOVED_ALT]     = "newMovedAlternative",
+	[DIFF_FILE_NEW_MOVED_DIM]     = "newMovedDimmed",
+	[DIFF_FILE_NEW_MOVED_ALT_DIM] = "newMovedAlternativeDimmed",
+};
+
 static NORETURN void die_want_option(const char *option_name)
 {
 	die(_("option '%s' requires a value"), option_name);
@@ -76,39 +95,9 @@ static NORETURN void die_want_option(const char *option_name)
 
 static int parse_diff_color_slot(const char *var)
 {
-	if (!strcasecmp(var, "context") || !strcasecmp(var, "plain"))
+	if (!strcasecmp(var, "plain"))
 		return DIFF_CONTEXT;
-	if (!strcasecmp(var, "meta"))
-		return DIFF_METAINFO;
-	if (!strcasecmp(var, "frag"))
-		return DIFF_FRAGINFO;
-	if (!strcasecmp(var, "old"))
-		return DIFF_FILE_OLD;
-	if (!strcasecmp(var, "new"))
-		return DIFF_FILE_NEW;
-	if (!strcasecmp(var, "commit"))
-		return DIFF_COMMIT;
-	if (!strcasecmp(var, "whitespace"))
-		return DIFF_WHITESPACE;
-	if (!strcasecmp(var, "func"))
-		return DIFF_FUNCINFO;
-	if (!strcasecmp(var, "oldmoved"))
-		return DIFF_FILE_OLD_MOVED;
-	if (!strcasecmp(var, "oldmovedalternative"))
-		return DIFF_FILE_OLD_MOVED_ALT;
-	if (!strcasecmp(var, "oldmoveddimmed"))
-		return DIFF_FILE_OLD_MOVED_DIM;
-	if (!strcasecmp(var, "oldmovedalternativedimmed"))
-		return DIFF_FILE_OLD_MOVED_ALT_DIM;
-	if (!strcasecmp(var, "newmoved"))
-		return DIFF_FILE_NEW_MOVED;
-	if (!strcasecmp(var, "newmovedalternative"))
-		return DIFF_FILE_NEW_MOVED_ALT;
-	if (!strcasecmp(var, "newmoveddimmed"))
-		return DIFF_FILE_NEW_MOVED_DIM;
-	if (!strcasecmp(var, "newmovedalternativedimmed"))
-		return DIFF_FILE_NEW_MOVED_ALT_DIM;
-	return -1;
+	return LOOKUP_CONFIG(color_diff_slots, var);
 }
 
 static int parse_dirstat_params(struct diff_options *options, const char *params_string,
diff --git a/log-tree.c b/log-tree.c
index 0b97de5e87..33be7f7a3d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -27,6 +27,14 @@ static char decoration_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_BOLD_BLUE,	/* GRAFTED */
 };
 
+static const char *color_decorate_slots[] = {
+	[DECORATION_REF_LOCAL]	= "branch",
+	[DECORATION_REF_REMOTE] = "remoteBranch",
+	[DECORATION_REF_TAG]	= "tag",
+	[DECORATION_REF_STASH]	= "stash",
+	[DECORATION_REF_HEAD]	= "HEAD",
+};
+
 static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)
 {
 	if (want_color(decorate_use_color))
@@ -34,34 +42,9 @@ static const char *decorate_get_color(int decorate_use_color, enum decoration_ty
 	return "";
 }
 
-static int parse_decorate_color_slot(const char *slot)
-{
-	/*
-	 * We're comparing with 'ignore-case' on
-	 * (because config.c sets them all tolower),
-	 * but let's match the letters in the literal
-	 * string values here with how they are
-	 * documented in Documentation/config.txt, for
-	 * consistency.
-	 *
-	 * We love being consistent, don't we?
-	 */
-	if (!strcasecmp(slot, "branch"))
-		return DECORATION_REF_LOCAL;
-	if (!strcasecmp(slot, "remoteBranch"))
-		return DECORATION_REF_REMOTE;
-	if (!strcasecmp(slot, "tag"))
-		return DECORATION_REF_TAG;
-	if (!strcasecmp(slot, "stash"))
-		return DECORATION_REF_STASH;
-	if (!strcasecmp(slot, "HEAD"))
-		return DECORATION_REF_HEAD;
-	return -1;
-}
-
 int parse_decorate_color_config(const char *var, const char *slot_name, const char *value)
 {
-	int slot = parse_decorate_color_slot(slot_name);
+	int slot = LOOKUP_CONFIG(color_decorate_slots, slot_name);
 	if (slot < 0)
 		return 0;
 	if (!value)
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 02/11] grep: keep all colors in an array
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 01/11] Add and use generic name->id mapping code for color slot parsing Nguyễn Thái Ngọc Duy
@ 2018-05-26 13:55   ` Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 03/11] fsck: factor out msg_id_info[] lazy initialization code Nguyễn Thái Ngọc Duy
                     ` (8 subsequent siblings)
  10 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

This is more inline with how we handle color slots in other code. It
also allows us to get the list of configurable color slots later.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 grep.c | 106 ++++++++++++++++++++++++++-------------------------------
 grep.h |  21 +++++++-----
 2 files changed, 62 insertions(+), 65 deletions(-)

diff --git a/grep.c b/grep.c
index 45ec7e636c..d94e2c4aeb 100644
--- a/grep.c
+++ b/grep.c
@@ -13,6 +13,17 @@ static int grep_source_is_binary(struct grep_source *gs);
 
 static struct grep_opt grep_defaults;
 
+static const char *color_grep_slots[] = {
+	[GREP_COLOR_CONTEXT]	    = "context",
+	[GREP_COLOR_FILENAME]	    = "filename",
+	[GREP_COLOR_FUNCTION]	    = "function",
+	[GREP_COLOR_LINENO]	    = "lineNumber",
+	[GREP_COLOR_MATCH_CONTEXT]  = "matchContext",
+	[GREP_COLOR_MATCH_SELECTED] = "matchSelected",
+	[GREP_COLOR_SELECTED]	    = "selected",
+	[GREP_COLOR_SEP]	    = "separator",
+};
+
 static void std_output(struct grep_opt *opt, const void *buf, size_t size)
 {
 	fwrite(buf, size, 1, stdout);
@@ -42,14 +53,14 @@ void init_grep_defaults(void)
 	opt->pathname = 1;
 	opt->max_depth = -1;
 	opt->pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED;
-	color_set(opt->color_context, "");
-	color_set(opt->color_filename, "");
-	color_set(opt->color_function, "");
-	color_set(opt->color_lineno, "");
-	color_set(opt->color_match_context, GIT_COLOR_BOLD_RED);
-	color_set(opt->color_match_selected, GIT_COLOR_BOLD_RED);
-	color_set(opt->color_selected, "");
-	color_set(opt->color_sep, GIT_COLOR_CYAN);
+	color_set(opt->colors[GREP_COLOR_CONTEXT], "");
+	color_set(opt->colors[GREP_COLOR_FILENAME], "");
+	color_set(opt->colors[GREP_COLOR_FUNCTION], "");
+	color_set(opt->colors[GREP_COLOR_LINENO], "");
+	color_set(opt->colors[GREP_COLOR_MATCH_CONTEXT], GIT_COLOR_BOLD_RED);
+	color_set(opt->colors[GREP_COLOR_MATCH_SELECTED], GIT_COLOR_BOLD_RED);
+	color_set(opt->colors[GREP_COLOR_SELECTED], "");
+	color_set(opt->colors[GREP_COLOR_SEP], GIT_COLOR_CYAN);
 	opt->color = -1;
 	opt->output = std_output;
 }
@@ -76,7 +87,7 @@ static int parse_pattern_type_arg(const char *opt, const char *arg)
 int grep_config(const char *var, const char *value, void *cb)
 {
 	struct grep_opt *opt = &grep_defaults;
-	char *color = NULL;
+	const char *slot;
 
 	if (userdiff_config(var, value) < 0)
 		return -1;
@@ -103,32 +114,18 @@ int grep_config(const char *var, const char *value, void *cb)
 
 	if (!strcmp(var, "color.grep"))
 		opt->color = git_config_colorbool(var, value);
-	else if (!strcmp(var, "color.grep.context"))
-		color = opt->color_context;
-	else if (!strcmp(var, "color.grep.filename"))
-		color = opt->color_filename;
-	else if (!strcmp(var, "color.grep.function"))
-		color = opt->color_function;
-	else if (!strcmp(var, "color.grep.linenumber"))
-		color = opt->color_lineno;
-	else if (!strcmp(var, "color.grep.matchcontext"))
-		color = opt->color_match_context;
-	else if (!strcmp(var, "color.grep.matchselected"))
-		color = opt->color_match_selected;
-	else if (!strcmp(var, "color.grep.selected"))
-		color = opt->color_selected;
-	else if (!strcmp(var, "color.grep.separator"))
-		color = opt->color_sep;
-	else if (!strcmp(var, "color.grep.match")) {
-		int rc = 0;
-		if (!value)
-			return config_error_nonbool(var);
-		rc |= color_parse(value, opt->color_match_context);
-		rc |= color_parse(value, opt->color_match_selected);
-		return rc;
-	}
-
-	if (color) {
+	if (!strcmp(var, "color.grep.match")) {
+		if (grep_config("color.grep.matchcontext", value, cb) < 0)
+			return -1;
+		if (grep_config("color.grep.matchselected", value, cb) < 0)
+			return -1;
+	} else if (skip_prefix(var, "color.grep.", &slot)) {
+		int i = LOOKUP_CONFIG(color_grep_slots, slot);
+		char *color;
+
+		if (i < 0)
+			return -1;
+		color = opt->colors[i];
 		if (!value)
 			return config_error_nonbool(var);
 		return color_parse(value, color);
@@ -144,6 +141,7 @@ int grep_config(const char *var, const char *value, void *cb)
 void grep_init(struct grep_opt *opt, const char *prefix)
 {
 	struct grep_opt *def = &grep_defaults;
+	int i;
 
 	memset(opt, 0, sizeof(*opt));
 	opt->prefix = prefix;
@@ -160,14 +158,8 @@ void grep_init(struct grep_opt *opt, const char *prefix)
 	opt->relative = def->relative;
 	opt->output = def->output;
 
-	color_set(opt->color_context, def->color_context);
-	color_set(opt->color_filename, def->color_filename);
-	color_set(opt->color_function, def->color_function);
-	color_set(opt->color_lineno, def->color_lineno);
-	color_set(opt->color_match_context, def->color_match_context);
-	color_set(opt->color_match_selected, def->color_match_selected);
-	color_set(opt->color_selected, def->color_selected);
-	color_set(opt->color_sep, def->color_sep);
+	for (i = 0; i < NR_GREP_COLORS; i++)
+		color_set(opt->colors[i], def->colors[i]);
 }
 
 static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, struct grep_opt *opt)
@@ -1098,12 +1090,12 @@ static void output_sep(struct grep_opt *opt, char sign)
 	if (opt->null_following_name)
 		opt->output(opt, "\0", 1);
 	else
-		output_color(opt, &sign, 1, opt->color_sep);
+		output_color(opt, &sign, 1, opt->colors[GREP_COLOR_SEP]);
 }
 
 static void show_name(struct grep_opt *opt, const char *name)
 {
-	output_color(opt, name, strlen(name), opt->color_filename);
+	output_color(opt, name, strlen(name), opt->colors[GREP_COLOR_FILENAME]);
 	opt->output(opt, opt->null_following_name ? "\0" : "\n", 1);
 }
 
@@ -1370,28 +1362,28 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	} else if (opt->pre_context || opt->post_context || opt->funcbody) {
 		if (opt->last_shown == 0) {
 			if (opt->show_hunk_mark) {
-				output_color(opt, "--", 2, opt->color_sep);
+				output_color(opt, "--", 2, opt->colors[GREP_COLOR_SEP]);
 				opt->output(opt, "\n", 1);
 			}
 		} else if (lno > opt->last_shown + 1) {
-			output_color(opt, "--", 2, opt->color_sep);
+			output_color(opt, "--", 2, opt->colors[GREP_COLOR_SEP]);
 			opt->output(opt, "\n", 1);
 		}
 	}
 	if (opt->heading && opt->last_shown == 0) {
-		output_color(opt, name, strlen(name), opt->color_filename);
+		output_color(opt, name, strlen(name), opt->colors[GREP_COLOR_FILENAME]);
 		opt->output(opt, "\n", 1);
 	}
 	opt->last_shown = lno;
 
 	if (!opt->heading && opt->pathname) {
-		output_color(opt, name, strlen(name), opt->color_filename);
+		output_color(opt, name, strlen(name), opt->colors[GREP_COLOR_FILENAME]);
 		output_sep(opt, sign);
 	}
 	if (opt->linenum) {
 		char buf[32];
 		xsnprintf(buf, sizeof(buf), "%d", lno);
-		output_color(opt, buf, strlen(buf), opt->color_lineno);
+		output_color(opt, buf, strlen(buf), opt->colors[GREP_COLOR_LINENO]);
 		output_sep(opt, sign);
 	}
 	if (opt->color) {
@@ -1401,15 +1393,15 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		int eflags = 0;
 
 		if (sign == ':')
-			match_color = opt->color_match_selected;
+			match_color = opt->colors[GREP_COLOR_MATCH_SELECTED];
 		else
-			match_color = opt->color_match_context;
+			match_color = opt->colors[GREP_COLOR_MATCH_CONTEXT];
 		if (sign == ':')
-			line_color = opt->color_selected;
+			line_color = opt->colors[GREP_COLOR_SELECTED];
 		else if (sign == '-')
-			line_color = opt->color_context;
+			line_color = opt->colors[GREP_COLOR_CONTEXT];
 		else if (sign == '=')
-			line_color = opt->color_function;
+			line_color = opt->colors[GREP_COLOR_FUNCTION];
 		*eol = '\0';
 		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
 			if (match.rm_so == match.rm_eo)
@@ -1816,7 +1808,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 			if (binary_match_only) {
 				opt->output(opt, "Binary file ", 12);
 				output_color(opt, gs->name, strlen(gs->name),
-					     opt->color_filename);
+					     opt->colors[GREP_COLOR_FILENAME]);
 				opt->output(opt, " matches\n", 9);
 				return 1;
 			}
@@ -1890,7 +1882,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 		char buf[32];
 		if (opt->pathname) {
 			output_color(opt, gs->name, strlen(gs->name),
-				     opt->color_filename);
+				     opt->colors[GREP_COLOR_FILENAME]);
 			output_sep(opt, ':');
 		}
 		xsnprintf(buf, sizeof(buf), "%u\n", count);
diff --git a/grep.h b/grep.h
index 399381c908..ed25be271b 100644
--- a/grep.h
+++ b/grep.h
@@ -62,6 +62,18 @@ enum grep_header_field {
 	GREP_HEADER_FIELD_MAX
 };
 
+enum grep_color {
+	GREP_COLOR_CONTEXT,
+	GREP_COLOR_FILENAME,
+	GREP_COLOR_FUNCTION,
+	GREP_COLOR_LINENO,
+	GREP_COLOR_MATCH_CONTEXT,
+	GREP_COLOR_MATCH_SELECTED,
+	GREP_COLOR_SELECTED,
+	GREP_COLOR_SEP,
+	NR_GREP_COLORS
+};
+
 struct grep_pat {
 	struct grep_pat *next;
 	const char *origin;
@@ -155,14 +167,7 @@ struct grep_opt {
 	int funcbody;
 	int extended_regexp_option;
 	int pattern_type_option;
-	char color_context[COLOR_MAXLEN];
-	char color_filename[COLOR_MAXLEN];
-	char color_function[COLOR_MAXLEN];
-	char color_lineno[COLOR_MAXLEN];
-	char color_match_context[COLOR_MAXLEN];
-	char color_match_selected[COLOR_MAXLEN];
-	char color_selected[COLOR_MAXLEN];
-	char color_sep[COLOR_MAXLEN];
+	char colors[NR_GREP_COLORS][COLOR_MAXLEN];
 	unsigned pre_context;
 	unsigned post_context;
 	unsigned last_shown;
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 03/11] fsck: factor out msg_id_info[] lazy initialization code
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 01/11] Add and use generic name->id mapping code for color slot parsing Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 02/11] grep: keep all colors in an array Nguyễn Thái Ngọc Duy
@ 2018-05-26 13:55   ` Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 04/11] help: add --config to list all available config Nguyễn Thái Ngọc Duy
                     ` (7 subsequent siblings)
  10 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

This array will be used by some other function than parse_msg_id() in
the following commit. Factor out this prep code so it could be called
from that one.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 fsck.c | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/fsck.c b/fsck.c
index 1722f6cab0..e2a6f33891 100644
--- a/fsck.c
+++ b/fsck.c
@@ -84,26 +84,34 @@ static struct {
 };
 #undef MSG_ID
 
-static int parse_msg_id(const char *text)
+static void prepare_msg_ids(void)
 {
 	int i;
 
-	if (!msg_id_info[0].downcased) {
-		/* convert id_string to lower case, without underscores. */
-		for (i = 0; i < FSCK_MSG_MAX; i++) {
-			const char *p = msg_id_info[i].id_string;
-			int len = strlen(p);
-			char *q = xmalloc(len);
-
-			msg_id_info[i].downcased = q;
-			while (*p)
-				if (*p == '_')
-					p++;
-				else
-					*(q)++ = tolower(*(p)++);
-			*q = '\0';
-		}
+	if (msg_id_info[0].downcased)
+		return;
+
+	/* convert id_string to lower case, without underscores. */
+	for (i = 0; i < FSCK_MSG_MAX; i++) {
+		const char *p = msg_id_info[i].id_string;
+		int len = strlen(p);
+		char *q = xmalloc(len);
+
+		msg_id_info[i].downcased = q;
+		while (*p)
+			if (*p == '_')
+				p++;
+			else
+				*(q)++ = tolower(*(p)++);
+		*q = '\0';
 	}
+}
+
+static int parse_msg_id(const char *text)
+{
+	int i;
+
+	prepare_msg_ids();
 
 	for (i = 0; i < FSCK_MSG_MAX; i++)
 		if (!strcmp(text, msg_id_info[i].downcased))
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 04/11] help: add --config to list all available config
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
                     ` (2 preceding siblings ...)
  2018-05-26 13:55   ` [PATCH v2 03/11] fsck: factor out msg_id_info[] lazy initialization code Nguyễn Thái Ngọc Duy
@ 2018-05-26 13:55   ` Nguyễn Thái Ngọc Duy
  2018-05-27  7:33     ` Eric Sunshine
  2018-05-26 13:55   ` [PATCH v2 05/11] fsck: produce camelCase config key names Nguyễn Thái Ngọc Duy
                     ` (6 subsequent siblings)
  10 siblings, 1 reply; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

Sometimes it helps to list all available config vars so the user can
search for something they want. The config man page can also be used
but it's harder to search if you want to focus on the variable name,
for example.

This is not the best way to collect the available config since it's
not precise. Ideally we should have a centralized list of config in C
code (pretty much like 'struct option'), but that's a lot more work.
This will do for now.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-help.txt |  5 ++++
 advice.c                   |  9 ++++++
 builtin/branch.c           |  3 ++
 builtin/clean.c            |  3 ++
 builtin/commit.c           |  3 ++
 builtin/help.c             |  9 ++++++
 diff.c                     |  3 ++
 fsck.c                     | 12 ++++++++
 generate-cmdlist.sh        | 19 +++++++++++++
 grep.c                     |  3 ++
 help.c                     | 56 ++++++++++++++++++++++++++++++++++++++
 help.h                     | 45 +++++++++++++++++++++++++++++-
 log-tree.c                 |  3 ++
 13 files changed, 172 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index a40fc38d8b..83d25d825a 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -45,6 +45,11 @@ OPTIONS
 	When used with `--verbose` print description for all recognized
 	commands.
 
+-c::
+--config::
+	List all available configuration variables. This is a short
+	summary of the list in linkgit:git-config[1].
+
 -g::
 --guides::
 	Prints a list of useful guides on the standard output. This
diff --git a/advice.c b/advice.c
index 370a56d054..cf6c673ed7 100644
--- a/advice.c
+++ b/advice.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "config.h"
 #include "color.h"
+#include "help.h"
 
 int advice_push_update_rejected = 1;
 int advice_push_non_ff_current = 1;
@@ -131,6 +132,14 @@ int git_default_advice_config(const char *var, const char *value)
 	return 0;
 }
 
+void list_config_advices(struct string_list *list, const char *prefix)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(advice_config); i++)
+		list_config_item(list, prefix, advice_config[i].name);
+}
+
 int error_resolve_conflict(const char *me)
 {
 	if (!strcmp(me, "cherry-pick"))
diff --git a/builtin/branch.c b/builtin/branch.c
index 09232576b6..ddc48ca931 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -22,6 +22,7 @@
 #include "wt-status.h"
 #include "ref-filter.h"
 #include "worktree.h"
+#include "help.h"
 
 static const char * const builtin_branch_usage[] = {
 	N_("git branch [<options>] [-r | -a] [--merged | --no-merged]"),
@@ -67,6 +68,8 @@ static const char *color_branch_slots[] = {
 static struct string_list output = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
+define_list_config_array(color_branch_slots);
+
 static int git_branch_config(const char *var, const char *value, void *cb)
 {
 	const char *slot_name;
diff --git a/builtin/clean.c b/builtin/clean.c
index 0ccd95e693..ab402c204c 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -16,6 +16,7 @@
 #include "column.h"
 #include "color.h"
 #include "pathspec.h"
+#include "help.h"
 
 static int force = -1; /* unset */
 static int interactive;
@@ -91,6 +92,8 @@ struct menu_stuff {
 	void *stuff;
 };
 
+define_list_config_array(color_interactive_slots);
+
 static int git_clean_config(const char *var, const char *value, void *cb)
 {
 	const char *slot_name;
diff --git a/builtin/commit.c b/builtin/commit.c
index 2b43a6c48a..9bcbb0c25c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -32,6 +32,7 @@
 #include "column.h"
 #include "sequencer.h"
 #include "mailmap.h"
+#include "help.h"
 
 static const char * const builtin_commit_usage[] = {
 	N_("git commit [<options>] [--] <pathspec>..."),
@@ -1195,6 +1196,8 @@ static int dry_run_commit(int argc, const char **argv, const char *prefix,
 	return commitable ? 0 : 1;
 }
 
+define_list_config_array_extra(color_status_slots, {"added"});
+
 static int parse_status_slot(const char *slot)
 {
 	if (!strcasecmp(slot, "added"))
diff --git a/builtin/help.c b/builtin/help.c
index 58e0a5507f..ccb206e1d4 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -37,6 +37,7 @@ static const char *html_path;
 
 static int show_all = 0;
 static int show_guides = 0;
+static int show_config;
 static int verbose;
 static unsigned int colopts;
 static enum help_format help_format = HELP_FORMAT_NONE;
@@ -45,6 +46,7 @@ static struct option builtin_help_options[] = {
 	OPT_BOOL('a', "all", &show_all, N_("print all available commands")),
 	OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")),
 	OPT_BOOL('g', "guides", &show_guides, N_("print list of useful guides")),
+	OPT_BOOL('c', "config", &show_config, N_("print all configuration variable names")),
 	OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN),
 	OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
 			HELP_FORMAT_WEB),
@@ -444,6 +446,13 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 		list_commands(colopts, &main_cmds, &other_cmds);
 	}
 
+	if (show_config) {
+		setup_pager();
+		list_config_help();
+		printf("\n%s\n", _("'git help config' for more information"));
+		return 0;
+	}
+
 	if (show_guides)
 		list_common_guides_help();
 
diff --git a/diff.c b/diff.c
index 2e5e1404a5..c79f18869a 100644
--- a/diff.c
+++ b/diff.c
@@ -22,6 +22,7 @@
 #include "argv-array.h"
 #include "graph.h"
 #include "packfile.h"
+#include "help.h"
 
 #ifdef NO_FAST_WORKING_DIRECTORY
 #define FAST_WORKING_DIRECTORY 0
@@ -93,6 +94,8 @@ static NORETURN void die_want_option(const char *option_name)
 	die(_("option '%s' requires a value"), option_name);
 }
 
+define_list_config_array_extra(color_diff_slots, {"plain"});
+
 static int parse_diff_color_slot(const char *var)
 {
 	if (!strcasecmp(var, "plain"))
diff --git a/fsck.c b/fsck.c
index e2a6f33891..b65ff2d856 100644
--- a/fsck.c
+++ b/fsck.c
@@ -10,6 +10,7 @@
 #include "utf8.h"
 #include "sha1-array.h"
 #include "decorate.h"
+#include "help.h"
 
 #define FSCK_FATAL -1
 #define FSCK_INFO -2
@@ -120,6 +121,17 @@ static int parse_msg_id(const char *text)
 	return -1;
 }
 
+void list_config_fsck_msg_ids(struct string_list *list, const char *prefix)
+{
+	int i;
+
+	prepare_msg_ids();
+
+	/* TODO: we can do better by producing camelCase names */
+	for (i = 0; i < FSCK_MSG_MAX; i++)
+		list_config_item(list, prefix, msg_id_info[i].downcased);
+}
+
 static int fsck_msg_type(enum fsck_msg_id msg_id,
 	struct fsck_options *options)
 {
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 8d6d8b45ce..c4124acbe7 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -76,6 +76,23 @@ print_command_list () {
 	echo "};"
 }
 
+print_config_list () {
+	cat <<EOF
+static const char *config_name_list[] = {
+EOF
+	grep '^[a-zA-Z].*\..*::$' Documentation/config.txt |
+	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
+	sort |
+	while read line
+	do
+		echo "	\"$line\","
+	done
+	cat <<EOF
+	NULL,
+};
+EOF
+}
+
 echo "/* Automatically generated by generate-cmdlist.sh */
 struct cmdname_help {
 	const char *name;
@@ -88,3 +105,5 @@ echo
 define_category_names "$1"
 echo
 print_command_list "$1"
+echo
+print_config_list
diff --git a/grep.c b/grep.c
index d94e2c4aeb..7c1b8e2e8b 100644
--- a/grep.c
+++ b/grep.c
@@ -7,6 +7,7 @@
 #include "diffcore.h"
 #include "commit.h"
 #include "quote.h"
+#include "help.h"
 
 static int grep_source_load(struct grep_source *gs);
 static int grep_source_is_binary(struct grep_source *gs);
@@ -80,6 +81,8 @@ static int parse_pattern_type_arg(const char *opt, const char *arg)
 	die("bad %s argument: %s", opt, arg);
 }
 
+define_list_config_array_extra(color_grep_slots, {"match"});
+
 /*
  * Read the configuration file once and store it in
  * the grep_defaults template.
diff --git a/help.c b/help.c
index dd35fcc133..f078dfebad 100644
--- a/help.c
+++ b/help.c
@@ -409,6 +409,62 @@ 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(void)
+{
+	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.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++)
+		puts(keys.items[i].string);
+	string_list_clear(&keys, 0);
+}
+
 void list_all_cmds_help(void)
 {
 	print_cmd_by_category(main_categories);
diff --git a/help.h b/help.h
index 3b38292a1b..b46fe6ee73 100644
--- a/help.h
+++ b/help.h
@@ -1,7 +1,8 @@
 #ifndef HELP_H
 #define HELP_H
 
-struct string_list;
+#include "string-list.h"
+#include "strbuf.h"
 
 struct cmdnames {
 	int alloc;
@@ -21,6 +22,7 @@ static inline void mput_char(char c, unsigned int num)
 extern void list_common_cmds_help(void);
 extern void list_all_cmds_help(void);
 extern void list_common_guides_help(void);
+extern void list_config_help(void);
 
 extern void list_all_main_cmds(struct string_list *list);
 extern void list_all_other_cmds(struct string_list *list);
@@ -42,4 +44,45 @@ extern void list_commands(unsigned int colopts, struct cmdnames *main_cmds, stru
  * ref to the command, to give suggested "correct" refs.
  */
 extern void help_unknown_ref(const char *ref, const char *cmd, const char *error);
+
+static inline void list_config_item(struct string_list *list,
+				    const char *prefix,
+				    const char *str)
+{
+	string_list_append_nodup(list, xstrfmt("%s.%s", prefix, str));
+}
+
+#define define_list_config_array(array)					\
+void list_config_##array(struct string_list *list, const char *prefix)	\
+{									\
+	int i;								\
+	for (i = 0; i < ARRAY_SIZE(array); i++)				\
+		if (array[i])						\
+			list_config_item(list, prefix, array[i]);	\
+}									\
+struct string_list
+
+#define define_list_config_array_extra(array, values)			\
+void list_config_##array(struct string_list *list, const char *prefix)	\
+{									\
+	int i;								\
+	static const char *extra[] = values;				\
+	for (i = 0; i < ARRAY_SIZE(extra); i++)				\
+		list_config_item(list, prefix, extra[i]);		\
+	for (i = 0; i < ARRAY_SIZE(array); i++)				\
+		if (array[i])						\
+			list_config_item(list, prefix, array[i]);	\
+}									\
+struct string_list
+
+/* These are actually scattered over many C files */
+void list_config_advices(struct string_list *list, const char *prefix);
+void list_config_color_branch_slots(struct string_list *list, const char *prefix);
+void list_config_color_decorate_slots(struct string_list *list, const char *prefix);
+void list_config_color_diff_slots(struct string_list *list, const char *prefix);
+void list_config_color_grep_slots(struct string_list *list, const char *prefix);
+void list_config_color_interactive_slots(struct string_list *list, const char *prefix);
+void list_config_color_status_slots(struct string_list *list, const char *prefix);
+void list_config_fsck_msg_ids(struct string_list *list, const char *prefix);
+
 #endif /* HELP_H */
diff --git a/log-tree.c b/log-tree.c
index 33be7f7a3d..284ce0e3c5 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -12,6 +12,7 @@
 #include "gpg-interface.h"
 #include "sequencer.h"
 #include "line-log.h"
+#include "help.h"
 
 static struct decoration name_decoration = { "object names" };
 static int decoration_loaded;
@@ -42,6 +43,8 @@ static const char *decorate_get_color(int decorate_use_color, enum decoration_ty
 	return "";
 }
 
+define_list_config_array(color_decorate_slots);
+
 int parse_decorate_color_config(const char *var, const char *slot_name, const char *value)
 {
 	int slot = LOOKUP_CONFIG(color_decorate_slots, slot_name);
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 05/11] fsck: produce camelCase config key names
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
                     ` (3 preceding siblings ...)
  2018-05-26 13:55   ` [PATCH v2 04/11] help: add --config to list all available config Nguyễn Thái Ngọc Duy
@ 2018-05-26 13:55   ` Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 06/11] advice: keep config name in camelCase in advice_config[] Nguyễn Thái Ngọc Duy
                     ` (5 subsequent siblings)
  10 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 fsck.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/fsck.c b/fsck.c
index b65ff2d856..ff1547d3c7 100644
--- a/fsck.c
+++ b/fsck.c
@@ -74,14 +74,15 @@ enum fsck_msg_id {
 #undef MSG_ID
 
 #define STR(x) #x
-#define MSG_ID(id, msg_type) { STR(id), NULL, FSCK_##msg_type },
+#define MSG_ID(id, msg_type) { STR(id), NULL, NULL, FSCK_##msg_type },
 static struct {
 	const char *id_string;
 	const char *downcased;
+	const char *camelcased;
 	int msg_type;
 } msg_id_info[FSCK_MSG_MAX + 1] = {
 	FOREACH_MSG_ID(MSG_ID)
-	{ NULL, NULL, -1 }
+	{ NULL, NULL, NULL, -1 }
 };
 #undef MSG_ID
 
@@ -105,6 +106,20 @@ static void prepare_msg_ids(void)
 			else
 				*(q)++ = tolower(*(p)++);
 		*q = '\0';
+
+		p = msg_id_info[i].id_string;
+		q = xmalloc(len);
+		msg_id_info[i].camelcased = q;
+		while (*p) {
+			if (*p == '_') {
+				p++;
+				if (*p)
+					*q++ = *p++;
+			} else {
+				*q++ = tolower(*p++);
+			}
+		}
+		*q = '\0';
 	}
 }
 
@@ -127,9 +142,8 @@ void list_config_fsck_msg_ids(struct string_list *list, const char *prefix)
 
 	prepare_msg_ids();
 
-	/* TODO: we can do better by producing camelCase names */
 	for (i = 0; i < FSCK_MSG_MAX; i++)
-		list_config_item(list, prefix, msg_id_info[i].downcased);
+		list_config_item(list, prefix, msg_id_info[i].camelcased);
 }
 
 static int fsck_msg_type(enum fsck_msg_id msg_id,
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 06/11] advice: keep config name in camelCase in advice_config[]
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
                     ` (4 preceding siblings ...)
  2018-05-26 13:55   ` [PATCH v2 05/11] fsck: produce camelCase config key names Nguyễn Thái Ngọc Duy
@ 2018-05-26 13:55   ` Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 07/11] am: move advice.amWorkDir parsing back to advice.c Nguyễn Thái Ngọc Duy
                     ` (4 subsequent siblings)
  10 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

For parsing, we don't really need this because the main config parser
will lowercase everything so we can do exact matching. But this array
now is also used for printing in `git help --config`. Keep camelCase
so we have a nice printout.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 advice.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/advice.c b/advice.c
index cf6c673ed7..2aca11f45e 100644
--- a/advice.c
+++ b/advice.c
@@ -54,28 +54,28 @@ static struct {
 	const char *name;
 	int *preference;
 } advice_config[] = {
-	{ "pushupdaterejected", &advice_push_update_rejected },
-	{ "pushnonffcurrent", &advice_push_non_ff_current },
-	{ "pushnonffmatching", &advice_push_non_ff_matching },
-	{ "pushalreadyexists", &advice_push_already_exists },
-	{ "pushfetchfirst", &advice_push_fetch_first },
-	{ "pushneedsforce", &advice_push_needs_force },
-	{ "statushints", &advice_status_hints },
-	{ "statusuoption", &advice_status_u_option },
-	{ "commitbeforemerge", &advice_commit_before_merge },
-	{ "resolveconflict", &advice_resolve_conflict },
-	{ "implicitidentity", &advice_implicit_identity },
-	{ "detachedhead", &advice_detached_head },
-	{ "setupstreamfailure", &advice_set_upstream_failure },
-	{ "objectnamewarning", &advice_object_name_warning },
-	{ "rmhints", &advice_rm_hints },
-	{ "addembeddedrepo", &advice_add_embedded_repo },
-	{ "ignoredhook", &advice_ignored_hook },
-	{ "waitingforeditor", &advice_waiting_for_editor },
-	{ "graftfiledeprecated", &advice_graft_file_deprecated },
+	{ "pushUpdateRejected", &advice_push_update_rejected },
+	{ "pushNonFFCurrent", &advice_push_non_ff_current },
+	{ "pushNonFFMatching", &advice_push_non_ff_matching },
+	{ "pushAlreadyExists", &advice_push_already_exists },
+	{ "pushFetchFirst", &advice_push_fetch_first },
+	{ "pushNeedsForce", &advice_push_needs_force },
+	{ "statusHints", &advice_status_hints },
+	{ "statusUoption", &advice_status_u_option },
+	{ "commitBeforeMerge", &advice_commit_before_merge },
+	{ "resolveConflict", &advice_resolve_conflict },
+	{ "implicitIdentity", &advice_implicit_identity },
+	{ "detachedHead", &advice_detached_head },
+	{ "setupStreamFailure", &advice_set_upstream_failure },
+	{ "objectNameWarning", &advice_object_name_warning },
+	{ "rmHints", &advice_rm_hints },
+	{ "addEmbeddedRepo", &advice_add_embedded_repo },
+	{ "ignoredHook", &advice_ignored_hook },
+	{ "waitingForEditor", &advice_waiting_for_editor },
+	{ "graftFileDeprecated", &advice_graft_file_deprecated },
 
 	/* make this an alias for backward compatibility */
-	{ "pushnonfastforward", &advice_push_update_rejected }
+	{ "pushNonFastForward", &advice_push_update_rejected }
 };
 
 void advise(const char *advice, ...)
@@ -123,7 +123,7 @@ int git_default_advice_config(const char *var, const char *value)
 		return 0;
 
 	for (i = 0; i < ARRAY_SIZE(advice_config); i++) {
-		if (strcmp(k, advice_config[i].name))
+		if (strcasecmp(k, advice_config[i].name))
 			continue;
 		*advice_config[i].preference = git_config_bool(var, value);
 		return 0;
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 07/11] am: move advice.amWorkDir parsing back to advice.c
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
                     ` (5 preceding siblings ...)
  2018-05-26 13:55   ` [PATCH v2 06/11] advice: keep config name in camelCase in advice_config[] Nguyễn Thái Ngọc Duy
@ 2018-05-26 13:55   ` Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 08/11] completion: drop the hard coded list of config vars Nguyễn Thái Ngọc Duy
                     ` (3 subsequent siblings)
  10 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

The only benefit from this move (apart from cleaner code) is that
advice.amWorkDir should now show up in `git help --config`. There
should be no regression since advice config is always read by the
git_default_config().

While at there, use advise() like other code. We now get "hint: "
prefix and the output is stderr instead of stdout (which is also the
reason for the test update because stderr is checked in a following
test and the extra advice can fail it).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 advice.c              | 2 ++
 advice.h              | 1 +
 builtin/am.c          | 6 +-----
 t/t4254-am-corrupt.sh | 2 +-
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/advice.c b/advice.c
index 2aca11f45e..52aa85bdfd 100644
--- a/advice.c
+++ b/advice.c
@@ -17,6 +17,7 @@ int advice_implicit_identity = 1;
 int advice_detached_head = 1;
 int advice_set_upstream_failure = 1;
 int advice_object_name_warning = 1;
+int advice_amworkdir = 1;
 int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
@@ -68,6 +69,7 @@ static struct {
 	{ "detachedHead", &advice_detached_head },
 	{ "setupStreamFailure", &advice_set_upstream_failure },
 	{ "objectNameWarning", &advice_object_name_warning },
+	{ "amWorkDir", &advice_amworkdir },
 	{ "rmHints", &advice_rm_hints },
 	{ "addEmbeddedRepo", &advice_add_embedded_repo },
 	{ "ignoredHook", &advice_ignored_hook },
diff --git a/advice.h b/advice.h
index 9f5064e82a..7e9377864f 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_implicit_identity;
 extern int advice_detached_head;
 extern int advice_set_upstream_failure;
 extern int advice_object_name_warning;
+extern int advice_amworkdir;
 extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
diff --git a/builtin/am.c b/builtin/am.c
index aa989e7390..735d016525 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1827,15 +1827,11 @@ static void am_run(struct am_state *state, int resume)
 		}
 
 		if (apply_status) {
-			int advice_amworkdir = 1;
-
 			printf_ln(_("Patch failed at %s %.*s"), msgnum(state),
 				linelen(state->msg), state->msg);
 
-			git_config_get_bool("advice.amworkdir", &advice_amworkdir);
-
 			if (advice_amworkdir)
-				printf_ln(_("Use 'git am --show-current-patch' to see the failed patch"));
+				advise(_("Use 'git am --show-current-patch' to see the failed patch"));
 
 			die_user_resolve(state);
 		}
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
index 168739c721..fd3bdbfe2c 100755
--- a/t/t4254-am-corrupt.sh
+++ b/t/t4254-am-corrupt.sh
@@ -25,7 +25,7 @@ test_expect_success setup '
 #   fatal: unable to write file '(null)' mode 100644: Bad address
 # Also, it had the unwanted side-effect of deleting f.
 test_expect_success 'try to apply corrupted patch' '
-	test_must_fail git am bad-patch.diff 2>actual
+	test_must_fail git -c advice.amWorkDir=false am bad-patch.diff 2>actual
 '
 
 test_expect_success 'compare diagnostic; ensure file is still here' '
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 08/11] completion: drop the hard coded list of config vars
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
                     ` (6 preceding siblings ...)
  2018-05-26 13:55   ` [PATCH v2 07/11] am: move advice.amWorkDir parsing back to advice.c Nguyễn Thái Ngọc Duy
@ 2018-05-26 13:55   ` Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 09/11] completion: keep other config var completion in camelCase Nguyễn Thái Ngọc Duy
                     ` (2 subsequent siblings)
  10 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

The new help option --config-for-completion is a machine friendlier
version of --config where all the placeholders and wildcards are
dropped, leaving only the good, completable prefixes for
git-completion.bash to consume.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/help.c                         |   9 +-
 contrib/completion/git-completion.bash | 335 +------------------------
 help.c                                 |  34 ++-
 help.h                                 |   2 +-
 4 files changed, 49 insertions(+), 331 deletions(-)

diff --git a/builtin/help.c b/builtin/help.c
index ccb206e1d4..8d4f6dd301 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -47,6 +47,7 @@ static struct option builtin_help_options[] = {
 	OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")),
 	OPT_BOOL('g', "guides", &show_guides, N_("print list of useful guides")),
 	OPT_BOOL('c', "config", &show_config, N_("print all configuration variable names")),
+	OPT_SET_INT_F(0, "config-for-completion", &show_config, "", 2, PARSE_OPT_HIDDEN),
 	OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN),
 	OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
 			HELP_FORMAT_WEB),
@@ -447,8 +448,14 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	}
 
 	if (show_config) {
+		int for_human = show_config == 1;
+
+		if (!for_human) {
+			list_config_help(for_human);
+			return 0;
+		}
 		setup_pager();
-		list_config_help();
+		list_config_help(for_human);
 		printf("\n%s\n", _("'git help config' for more information"));
 		return 0;
 	}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 12814e9bbf..1b0c30ed9a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2142,6 +2142,13 @@ __git_config_get_set_variables ()
 	__git config $config_file --name-only --list
 }
 
+__git_config_vars=
+__git_compute_config_vars ()
+{
+	test -n "$__git_config_vars" ||
+	__git_config_vars="$(git help --config-for-completion | sort | uniq)"
+}
+
 _git_config ()
 {
 	case "$prev" in
@@ -2300,332 +2307,8 @@ _git_config ()
 		return
 		;;
 	esac
-	__gitcomp "
-		add.ignoreErrors
-		advice.amWorkDir
-		advice.commitBeforeMerge
-		advice.detachedHead
-		advice.implicitIdentity
-		advice.pushAlreadyExists
-		advice.pushFetchFirst
-		advice.pushNeedsForce
-		advice.pushNonFFCurrent
-		advice.pushNonFFMatching
-		advice.pushUpdateRejected
-		advice.resolveConflict
-		advice.rmHints
-		advice.statusHints
-		advice.statusUoption
-		advice.ignoredHook
-		alias.
-		am.keepcr
-		am.threeWay
-		apply.ignorewhitespace
-		apply.whitespace
-		branch.autosetupmerge
-		branch.autosetuprebase
-		browser.
-		clean.requireForce
-		color.branch
-		color.branch.current
-		color.branch.local
-		color.branch.plain
-		color.branch.remote
-		color.decorate.HEAD
-		color.decorate.branch
-		color.decorate.remoteBranch
-		color.decorate.stash
-		color.decorate.tag
-		color.diff
-		color.diff.commit
-		color.diff.frag
-		color.diff.func
-		color.diff.meta
-		color.diff.new
-		color.diff.old
-		color.diff.plain
-		color.diff.whitespace
-		color.grep
-		color.grep.context
-		color.grep.filename
-		color.grep.function
-		color.grep.linenumber
-		color.grep.match
-		color.grep.selected
-		color.grep.separator
-		color.interactive
-		color.interactive.error
-		color.interactive.header
-		color.interactive.help
-		color.interactive.prompt
-		color.pager
-		color.showbranch
-		color.status
-		color.status.added
-		color.status.changed
-		color.status.header
-		color.status.localBranch
-		color.status.nobranch
-		color.status.remoteBranch
-		color.status.unmerged
-		color.status.untracked
-		color.status.updated
-		color.ui
-		commit.cleanup
-		commit.gpgSign
-		commit.status
-		commit.template
-		commit.verbose
-		core.abbrev
-		core.askpass
-		core.attributesfile
-		core.autocrlf
-		core.bare
-		core.bigFileThreshold
-		core.checkStat
-		core.commentChar
-		core.commitGraph
-		core.compression
-		core.createObject
-		core.deltaBaseCacheLimit
-		core.editor
-		core.eol
-		core.excludesfile
-		core.fileMode
-		core.fsyncobjectfiles
-		core.gitProxy
-		core.hideDotFiles
-		core.hooksPath
-		core.ignoreStat
-		core.ignorecase
-		core.logAllRefUpdates
-		core.loosecompression
-		core.notesRef
-		core.packedGitLimit
-		core.packedGitWindowSize
-		core.packedRefsTimeout
-		core.pager
-		core.precomposeUnicode
-		core.preferSymlinkRefs
-		core.preloadindex
-		core.protectHFS
-		core.protectNTFS
-		core.quotepath
-		core.repositoryFormatVersion
-		core.safecrlf
-		core.sharedRepository
-		core.sparseCheckout
-		core.splitIndex
-		core.sshCommand
-		core.symlinks
-		core.trustctime
-		core.untrackedCache
-		core.warnAmbiguousRefs
-		core.whitespace
-		core.worktree
-		credential.helper
-		credential.useHttpPath
-		credential.username
-		credentialCache.ignoreSIGHUP
-		diff.autorefreshindex
-		diff.external
-		diff.ignoreSubmodules
-		diff.mnemonicprefix
-		diff.noprefix
-		diff.renameLimit
-		diff.renames
-		diff.statGraphWidth
-		diff.submodule
-		diff.suppressBlankEmpty
-		diff.tool
-		diff.wordRegex
-		diff.algorithm
-		difftool.
-		difftool.prompt
-		fetch.recurseSubmodules
-		fetch.unpackLimit
-		format.attach
-		format.cc
-		format.coverLetter
-		format.from
-		format.headers
-		format.numbered
-		format.pretty
-		format.signature
-		format.signoff
-		format.subjectprefix
-		format.suffix
-		format.thread
-		format.to
-		gc.
-		gc.aggressiveDepth
-		gc.aggressiveWindow
-		gc.auto
-		gc.autoDetach
-		gc.autopacklimit
-		gc.logExpiry
-		gc.packrefs
-		gc.pruneexpire
-		gc.reflogexpire
-		gc.reflogexpireunreachable
-		gc.rerereresolved
-		gc.rerereunresolved
-		gc.worktreePruneExpire
-		gitcvs.allbinary
-		gitcvs.commitmsgannotation
-		gitcvs.dbTableNamePrefix
-		gitcvs.dbdriver
-		gitcvs.dbname
-		gitcvs.dbpass
-		gitcvs.dbuser
-		gitcvs.enabled
-		gitcvs.logfile
-		gitcvs.usecrlfattr
-		guitool.
-		gui.blamehistoryctx
-		gui.commitmsgwidth
-		gui.copyblamethreshold
-		gui.diffcontext
-		gui.encoding
-		gui.fastcopyblame
-		gui.matchtrackingbranch
-		gui.newbranchtemplate
-		gui.pruneduringfetch
-		gui.spellingdictionary
-		gui.trustmtime
-		help.autocorrect
-		help.browser
-		help.format
-		http.lowSpeedLimit
-		http.lowSpeedTime
-		http.maxRequests
-		http.minSessions
-		http.noEPSV
-		http.postBuffer
-		http.proxy
-		http.sslCipherList
-		http.sslVersion
-		http.sslCAInfo
-		http.sslCAPath
-		http.sslCert
-		http.sslCertPasswordProtected
-		http.sslKey
-		http.sslVerify
-		http.useragent
-		i18n.commitEncoding
-		i18n.logOutputEncoding
-		imap.authMethod
-		imap.folder
-		imap.host
-		imap.pass
-		imap.port
-		imap.preformattedHTML
-		imap.sslverify
-		imap.tunnel
-		imap.user
-		init.templatedir
-		instaweb.browser
-		instaweb.httpd
-		instaweb.local
-		instaweb.modulepath
-		instaweb.port
-		interactive.singlekey
-		log.date
-		log.decorate
-		log.showroot
-		mailmap.file
-		man.
-		man.viewer
-		merge.
-		merge.conflictstyle
-		merge.log
-		merge.renameLimit
-		merge.renormalize
-		merge.stat
-		merge.tool
-		merge.verbosity
-		mergetool.
-		mergetool.keepBackup
-		mergetool.keepTemporaries
-		mergetool.prompt
-		notes.displayRef
-		notes.rewrite.
-		notes.rewrite.amend
-		notes.rewrite.rebase
-		notes.rewriteMode
-		notes.rewriteRef
-		pack.compression
-		pack.deltaCacheLimit
-		pack.deltaCacheSize
-		pack.depth
-		pack.indexVersion
-		pack.packSizeLimit
-		pack.threads
-		pack.window
-		pack.windowMemory
-		pager.
-		pretty.
-		pull.octopus
-		pull.twohead
-		push.default
-		push.followTags
-		rebase.autosquash
-		rebase.stat
-		receive.autogc
-		receive.denyCurrentBranch
-		receive.denyDeleteCurrent
-		receive.denyDeletes
-		receive.denyNonFastForwards
-		receive.fsckObjects
-		receive.unpackLimit
-		receive.updateserverinfo
-		remote.pushdefault
-		remotes.
-		repack.usedeltabaseoffset
-		rerere.autoupdate
-		rerere.enabled
-		sendemail.
-		sendemail.aliasesfile
-		sendemail.aliasfiletype
-		sendemail.bcc
-		sendemail.cc
-		sendemail.cccmd
-		sendemail.chainreplyto
-		sendemail.confirm
-		sendemail.envelopesender
-		sendemail.from
-		sendemail.identity
-		sendemail.multiedit
-		sendemail.signedoffbycc
-		sendemail.smtpdomain
-		sendemail.smtpencryption
-		sendemail.smtppass
-		sendemail.smtpserver
-		sendemail.smtpserveroption
-		sendemail.smtpserverport
-		sendemail.smtpuser
-		sendemail.suppresscc
-		sendemail.suppressfrom
-		sendemail.thread
-		sendemail.to
-		sendemail.tocmd
-		sendemail.validate
-		sendemail.smtpbatchsize
-		sendemail.smtprelogindelay
-		showbranch.default
-		status.relativePaths
-		status.showUntrackedFiles
-		status.submodulesummary
-		submodule.
-		tar.umask
-		transfer.unpackLimit
-		url.
-		user.email
-		user.name
-		user.signingkey
-		web.browser
-		branch. remote.
-	"
+	__git_compute_config_vars
+	__gitcomp "$__git_config_vars"
 }
 
 _git_remote ()
diff --git a/help.c b/help.c
index f078dfebad..3ebf0568db 100644
--- a/help.c
+++ b/help.c
@@ -416,7 +416,7 @@ struct slot_expansion {
 	int found;
 };
 
-void list_config_help(void)
+void list_config_help(int for_human)
 {
 	struct slot_expansion slot_expansions[] = {
 		{ "advice", "*", list_config_advices },
@@ -460,8 +460,36 @@ void list_config_help(void)
 			    e->prefix, e->placeholder);
 
 	string_list_sort(&keys);
-	for (i = 0; i < keys.nr; i++)
-		puts(keys.items[i].string);
+	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);
 }
 
diff --git a/help.h b/help.h
index b46fe6ee73..f8b15323a6 100644
--- a/help.h
+++ b/help.h
@@ -22,7 +22,7 @@ static inline void mput_char(char c, unsigned int num)
 extern void list_common_cmds_help(void);
 extern void list_all_cmds_help(void);
 extern void list_common_guides_help(void);
-extern void list_config_help(void);
+extern void list_config_help(int for_human);
 
 extern void list_all_main_cmds(struct string_list *list);
 extern void list_all_other_cmds(struct string_list *list);
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 09/11] completion: keep other config var completion in camelCase
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
                     ` (7 preceding siblings ...)
  2018-05-26 13:55   ` [PATCH v2 08/11] completion: drop the hard coded list of config vars Nguyễn Thái Ngọc Duy
@ 2018-05-26 13:55   ` Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 10/11] completion: support case-insensitive config vars Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 11/11] log-tree: allow to customize 'grafted' color Nguyễn Thái Ngọc Duy
  10 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

The last patch makes "git config <tab>" shows camelCase names because
that's what's in the source: config.txt. There are still a couple
manual var completion in this code. Let's make them follow the naming
convention as well.

In theory we could automate this part too because we have the
information. But let's stick to one step at a time and leave this for
later.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 contrib/completion/git-completion.bash | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1b0c30ed9a..efc930c9d1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2249,20 +2249,20 @@ _git_config ()
 		;;
 	branch.*.*)
 		local pfx="${cur%.*}." cur_="${cur##*.}"
-		__gitcomp "remote pushremote merge mergeoptions rebase" "$pfx" "$cur_"
+		__gitcomp "remote pushRemote merge mergeOptions rebase" "$pfx" "$cur_"
 		return
 		;;
 	branch.*)
 		local pfx="${cur%.*}." cur_="${cur#*.}"
 		__gitcomp_direct "$(__git_heads "$pfx" "$cur_" ".")"
-		__gitcomp_nl_append $'autosetupmerge\nautosetuprebase\n' "$pfx" "$cur_"
+		__gitcomp_nl_append $'autoSetupMerge\nautoSetupRebase\n' "$pfx" "$cur_"
 		return
 		;;
 	guitool.*.*)
 		local pfx="${cur%.*}." cur_="${cur##*.}"
 		__gitcomp "
-			argprompt cmd confirm needsfile noconsole norescan
-			prompt revprompt revunmerged title
+			argPrompt cmd confirm needsFile noConsole noRescan
+			prompt revPrompt revUnmerged title
 			" "$pfx" "$cur_"
 		return
 		;;
@@ -2291,14 +2291,14 @@ _git_config ()
 		local pfx="${cur%.*}." cur_="${cur##*.}"
 		__gitcomp "
 			url proxy fetch push mirror skipDefaultUpdate
-			receivepack uploadpack tagopt pushurl
+			receivepack uploadpack tagOpt pushurl
 			" "$pfx" "$cur_"
 		return
 		;;
 	remote.*)
 		local pfx="${cur%.*}." cur_="${cur#*.}"
 		__gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
-		__gitcomp_nl_append "pushdefault" "$pfx" "$cur_"
+		__gitcomp_nl_append "pushDefault" "$pfx" "$cur_"
 		return
 		;;
 	url.*.*)
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 10/11] completion: support case-insensitive config vars
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
                     ` (8 preceding siblings ...)
  2018-05-26 13:55   ` [PATCH v2 09/11] completion: keep other config var completion in camelCase Nguyễn Thái Ngọc Duy
@ 2018-05-26 13:55   ` Nguyễn Thái Ngọc Duy
  2018-05-26 13:55   ` [PATCH v2 11/11] log-tree: allow to customize 'grafted' color Nguyễn Thái Ngọc Duy
  10 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

Config variables are case-insensitive but this case/esac construct is
case-sensitive by default. For bash v4, it'll be easy. For platforms
that are stuck with older versions, we need an external command, but
that is not that critical. And where this additional overhead matters
the most is Windows, but luckily Git for Windows ships with Bash v4.

Helped-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 contrib/completion/git-completion.bash | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index efc930c9d1..2cbd14b346 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2151,7 +2151,15 @@ __git_compute_config_vars ()
 
 _git_config ()
 {
-	case "$prev" in
+	local varname
+
+	if [ "${BASH_VERSINFO[0]:-0}" -ge 4 ]; then
+		varname="${prev,,}"
+	else
+		varname="$(echo "$prev" |tr A-Z a-z)"
+	fi
+
+	case "$varname" in
 	branch.*.remote|branch.*.pushremote)
 		__gitcomp_nl "$(__git_remotes)"
 		return
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 11/11] log-tree: allow to customize 'grafted' color
  2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
                     ` (9 preceding siblings ...)
  2018-05-26 13:55   ` [PATCH v2 10/11] completion: support case-insensitive config vars Nguyễn Thái Ngọc Duy
@ 2018-05-26 13:55   ` Nguyễn Thái Ngọc Duy
  2018-05-27 18:28     ` [PATCH v2 12/11] completion: complete general config vars in two steps Nguyễn Thái Ngọc Duy
  10 siblings, 1 reply; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-26 13:55 UTC (permalink / raw)
  To: pclouds
  Cc: git, Stefan Beller, Junio C Hamano, Eric Sunshine,
	SZEDER Gábor

Commit 76f5df305b (log: decorate grafted commits with "grafted" -
2011-08-18) lets us decorate grafted commits but I forgot about the
color.decorate.* config.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt | 3 ++-
 log-tree.c               | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ab641bf5a9..1cc18a828c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1162,7 +1162,8 @@ color.diff.<slot>::
 color.decorate.<slot>::
 	Use customized color for 'git log --decorate' output.  `<slot>` is one
 	of `branch`, `remoteBranch`, `tag`, `stash` or `HEAD` for local
-	branches, remote-tracking branches, tags, stash and HEAD, respectively.
+	branches, remote-tracking branches, tags, stash and HEAD, respectively
+	and `grafted` for grafted commits.
 
 color.grep::
 	When set to `always`, always highlight matches.  When `false` (or
diff --git a/log-tree.c b/log-tree.c
index 284ce0e3c5..d3a43e29cd 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -34,6 +34,7 @@ static const char *color_decorate_slots[] = {
 	[DECORATION_REF_TAG]	= "tag",
 	[DECORATION_REF_STASH]	= "stash",
 	[DECORATION_REF_HEAD]	= "HEAD",
+	[DECORATION_GRAFTED]	= "grafted",
 };
 
 static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 04/11] help: add --config to list all available config
  2018-05-26 13:55   ` [PATCH v2 04/11] help: add --config to list all available config Nguyễn Thái Ngọc Duy
@ 2018-05-27  7:33     ` Eric Sunshine
  0 siblings, 0 replies; 36+ messages in thread
From: Eric Sunshine @ 2018-05-27  7:33 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: Git List, Stefan Beller, Junio C Hamano, SZEDER Gábor

On Sat, May 26, 2018 at 9:55 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Sometimes it helps to list all available config vars so the user can
> search for something they want. The config man page can also be used
> but it's harder to search if you want to focus on the variable name,
> for example.
>
> This is not the best way to collect the available config since it's
> not precise. Ideally we should have a centralized list of config in C
> code (pretty much like 'struct option'), but that's a lot more work.
> This will do for now.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/help.c b/help.c
> @@ -409,6 +409,62 @@ void list_common_guides_help(void)
> +void list_config_help(void)
> +{
> +       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);

Style nit: unwanted blank line

> +                       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);
> +       }

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v2 12/11] completion: complete general config vars in two steps
  2018-05-26 13:55   ` [PATCH v2 11/11] log-tree: allow to customize 'grafted' color Nguyễn Thái Ngọc Duy
@ 2018-05-27 18:28     ` Nguyễn Thái Ngọc Duy
  0 siblings, 0 replies; 36+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-27 18:28 UTC (permalink / raw)
  To: pclouds; +Cc: git, gitster, sbeller, sunshine, szeder.dev

There are 581 config variables as of now when you do "git config
<tab>" which can fill up a few screens and is not very helpful when
you have to look through columns of text to find what you want.

This patch instead shows you only first level when you do

    git config <tab>

There are 78 items, which use up 8 rows in my screen. Compared to
screens of text, it's pretty good. Once you have chosen you first
level, e.g. color:

    git config color.<tab>

will show you all color.*

This is not a new idea. branch.* and remote.* completion already does
this for second and third levels. For those variables, you'll need to
<tab> three times to get full variable name.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 I mentioned about this a while back in the --no-... completion RFC.
 It turns out not that hard to do, especially when branch.* and
 remote.* already kinda set a precedence.

 contrib/completion/git-completion.bash | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2cbd14b346..ab026776df 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2314,9 +2314,14 @@ _git_config ()
 		__gitcomp "insteadOf pushInsteadOf" "$pfx" "$cur_"
 		return
 		;;
+	*.*)
+		__git_compute_config_vars
+		__gitcomp "$__git_config_vars"
+		;;
+	*)
+		__git_compute_config_vars
+		__gitcomp "$(echo "$__git_config_vars" | sed 's/\.[^ ]*/./g')"
 	esac
-	__git_compute_config_vars
-	__gitcomp "$__git_config_vars"
 }
 
 _git_remote ()
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 01/11] Add and use generic name->id mapping code for color slot parsing
  2018-05-26 13:55   ` [PATCH v2 01/11] Add and use generic name->id mapping code for color slot parsing Nguyễn Thái Ngọc Duy
@ 2018-05-29 17:41     ` Stefan Beller
  0 siblings, 0 replies; 36+ messages in thread
From: Stefan Beller @ 2018-05-29 17:41 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Junio C Hamano, Eric Sunshine, SZEDER Gábor

On Sat, May 26, 2018 at 6:55 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Instead of hard coding the name-to-id mapping in C code, keep it in an
> array and use a common function to do the parsing. This reduces code
> and also allows us to list all possible color slots later.
>
> This starts using C99 designated initializers more for convenience
> (the first designated initializers have been introduced in builtin/clean.c
> for some time without complaints)

s/for some time without complaints/
in 512f41cfac5 (clean.c: use designated initializer, 2017-07-14) which
is almost a year without complaint/ maybe?

(just in case a resend is needed; please be precise and mention that
other commit so it's easy to see the reasoning there)

> +
> +int lookup_config(const char **mapping, int nr_mapping, const char *var)
> +{
> +       int i;
> +
> +       for (i = 0; i < nr_mapping; i++) {
> +               const char *name = mapping[i];
> +

    if (!name)
        break;

maybe? We do we need to check for 'name' in the next
condition? What input do we expect/allow?

> +               if (name && !strcasecmp(var, name))
> +                       return i;


> +#define LOOKUP_CONFIG(mapping, var) \
> +       lookup_config(mapping, ARRAY_SIZE(mapping), var)
> +int lookup_config(const char **mapping, int nr_mapping, const char *var);

Can you add a comment here saying what mapping is or should look like?

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2018-05-29 17:41 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-10 14:19 [PATCH 0/9] completion: avoid hard coding config var list Nguyễn Thái Ngọc Duy
2018-05-10 14:19 ` [PATCH 1/9] Add and use generic name->id mapping code for color slot parsing Nguyễn Thái Ngọc Duy
2018-05-10 17:16   ` Stefan Beller
2018-05-11  7:22     ` Duy Nguyen
2018-05-10 14:19 ` [PATCH 2/9] grep: keep all colors in an array Nguyễn Thái Ngọc Duy
2018-05-10 14:19 ` [PATCH 3/9] fsck: factor out msg_id_info[] lazy initialization code Nguyễn Thái Ngọc Duy
2018-05-11 22:23   ` Eric Sunshine
2018-05-10 14:19 ` [PATCH 4/9] help: add --config to list all available config Nguyễn Thái Ngọc Duy
2018-05-11 22:39   ` Eric Sunshine
2018-05-10 14:19 ` [PATCH 5/9] advice: keep config name in camelCase in advice_config[] Nguyễn Thái Ngọc Duy
2018-05-10 14:19 ` [PATCH 6/9] am: move advice.amWorkDir parsing back to advice.c Nguyễn Thái Ngọc Duy
2018-05-11 22:42   ` Eric Sunshine
2018-05-10 14:19 ` [PATCH 7/9] completion: drop the hard coded list of config vars Nguyễn Thái Ngọc Duy
2018-05-11 22:47   ` Eric Sunshine
2018-05-20 16:01   ` SZEDER Gábor
2018-05-10 14:19 ` [PATCH 8/9] completion: support case-insensitive config vars just a bit Nguyễn Thái Ngọc Duy
2018-05-20 16:33   ` SZEDER Gábor
2018-05-10 14:19 ` [PATCH 9/9] log-tree: allow to customize 'grafted' color Nguyễn Thái Ngọc Duy
2018-05-10 14:22 ` [PATCH 0/9] completion: avoid hard coding config var list Duy Nguyen
2018-05-11  6:00 ` Junio C Hamano
2018-05-11  6:24   ` Duy Nguyen
2018-05-26 13:55 ` [PATCH v2 00/11] " Nguyễn Thái Ngọc Duy
2018-05-26 13:55   ` [PATCH v2 01/11] Add and use generic name->id mapping code for color slot parsing Nguyễn Thái Ngọc Duy
2018-05-29 17:41     ` Stefan Beller
2018-05-26 13:55   ` [PATCH v2 02/11] grep: keep all colors in an array Nguyễn Thái Ngọc Duy
2018-05-26 13:55   ` [PATCH v2 03/11] fsck: factor out msg_id_info[] lazy initialization code Nguyễn Thái Ngọc Duy
2018-05-26 13:55   ` [PATCH v2 04/11] help: add --config to list all available config Nguyễn Thái Ngọc Duy
2018-05-27  7:33     ` Eric Sunshine
2018-05-26 13:55   ` [PATCH v2 05/11] fsck: produce camelCase config key names Nguyễn Thái Ngọc Duy
2018-05-26 13:55   ` [PATCH v2 06/11] advice: keep config name in camelCase in advice_config[] Nguyễn Thái Ngọc Duy
2018-05-26 13:55   ` [PATCH v2 07/11] am: move advice.amWorkDir parsing back to advice.c Nguyễn Thái Ngọc Duy
2018-05-26 13:55   ` [PATCH v2 08/11] completion: drop the hard coded list of config vars Nguyễn Thái Ngọc Duy
2018-05-26 13:55   ` [PATCH v2 09/11] completion: keep other config var completion in camelCase Nguyễn Thái Ngọc Duy
2018-05-26 13:55   ` [PATCH v2 10/11] completion: support case-insensitive config vars Nguyễn Thái Ngọc Duy
2018-05-26 13:55   ` [PATCH v2 11/11] log-tree: allow to customize 'grafted' color Nguyễn Thái Ngọc Duy
2018-05-27 18:28     ` [PATCH v2 12/11] completion: complete general config vars in two steps Nguyễn Thái Ngọc Duy

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).