git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 01/11] Add and use generic name->id mapping code for color slot parsing

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
May 26, 2018, 13:55 UTC
Message-ID
<20180526135531.4516-2-pclouds@gmail.com>
In-Reply-To
<20180526135531.4516-1-pclouds@gmail.com>

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
Previous: Nguyễn Thái Ngọc DuyNext: Stefan Beller
Message 23 of 36 in “completion: avoid hard coding config var list”
  1. 0/9 completion: avoid hard coding config var listNguyễn Thái Ngọc Duy, May 10, 2018
  2. 1/9 Add and use generic name->id mapping code for color slot parsingNguyễn Thái Ngọc Duy, May 10, 2018
  3. Stefan BellerMay 10, 2018
  4. Duy NguyenMay 11, 2018
  5. 3/9 fsck: factor out msg_id_info[] lazy initialization codeNguyễn Thái Ngọc Duy, May 10, 2018
  6. Eric SunshineMay 11, 2018
  7. 2/9 grep: keep all colors in an arrayNguyễn Thái Ngọc Duy, May 10, 2018
  8. 8/9 completion: support case-insensitive config vars just a bitNguyễn Thái Ngọc Duy, May 10, 2018
  9. SZEDER GáborMay 20, 2018
  10. 7/9 completion: drop the hard coded list of config varsNguyễn Thái Ngọc Duy, May 10, 2018
  11. Eric SunshineMay 11, 2018
  12. SZEDER GáborMay 20, 2018
  13. 6/9 am: move advice.amWorkDir parsing back to advice.cNguyễn Thái Ngọc Duy, May 10, 2018
  14. Eric SunshineMay 11, 2018
  15. 9/9 log-tree: allow to customize 'grafted' colorNguyễn Thái Ngọc Duy, May 10, 2018
  16. 4/9 help: add --config to list all available configNguyễn Thái Ngọc Duy, May 10, 2018
  17. Eric SunshineMay 11, 2018
  18. 5/9 advice: keep config name in camelCase in advice_config[]Nguyễn Thái Ngọc Duy, May 10, 2018
  19. Duy NguyenMay 10, 2018
  20. Junio C HamanoMay 11, 2018
  21. Duy NguyenMay 11, 2018
  22. 00/11 completion: avoid hard coding config var listNguyễn Thái Ngọc Duy, May 26, 2018
  23. 01/11 Add and use generic name->id mapping code for color slot parsingNguyễn Thái Ngọc Duy, May 26, 2018
  24. Stefan BellerMay 29, 2018
  25. 04/11 help: add --config to list all available configNguyễn Thái Ngọc Duy, May 26, 2018
  26. Eric SunshineMay 27, 2018
  27. 09/11 completion: keep other config var completion in camelCaseNguyễn Thái Ngọc Duy, May 26, 2018
  28. 07/11 am: move advice.amWorkDir parsing back to advice.cNguyễn Thái Ngọc Duy, May 26, 2018
  29. 11/11 log-tree: allow to customize 'grafted' colorNguyễn Thái Ngọc Duy, May 26, 2018
  30. 12/11 completion: complete general config vars in two stepsNguyễn Thái Ngọc Duy, May 27, 2018
  31. 10/11 completion: support case-insensitive config varsNguyễn Thái Ngọc Duy, May 26, 2018
  32. 03/11 fsck: factor out msg_id_info[] lazy initialization codeNguyễn Thái Ngọc Duy, May 26, 2018
  33. 08/11 completion: drop the hard coded list of config varsNguyễn Thái Ngọc Duy, May 26, 2018
  34. 02/11 grep: keep all colors in an arrayNguyễn Thái Ngọc Duy, May 26, 2018
  35. 06/11 advice: keep config name in camelCase in advice_config[]Nguyễn Thái Ngọc Duy, May 26, 2018
  36. 05/11 fsck: produce camelCase config key namesNguyễn Thái Ngọc Duy, May 26, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.