Re: [PATCH 1/9] Add and use generic name->id mapping code for color slot parsing

[Duy Nguyen]

On Thu, May 10, 2018 at 7:16 PM, Stefan Beller  wrote:
> On Thu, May 10, 2018 at 7:19 AM, Nguyễn Thái Ngọc Duy  
> 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

Re: [PATCH 1/9] Add and use generic name->id mapping code for color slot parsing

[Stefan Beller]

On Thu, May 10, 2018 at 7:19 AM, Nguyễn Thái Ngọc Duy  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

[PATCH 1/9] Add and use generic name->id mapping code for color slot parsing

[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 
---
 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.", _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.", _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