René Scharfe wrote:
> Continue where ae021d87 (use skip_prefix to avoid magic numbers) left off
> and use skip_prefix() in more places for determining the lengths of prefix
> strings to avoid using dependent constants and other indirect methods.
Sounds promising.
[...]
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -81,14 +81,16 @@ static int parse_branch_color_slot(const char *var, int ofs)
>
> static int git_branch_config(const char *var, const char *value, void *cb)
> {
> + const char *slot_name;
> +
> if (starts_with(var, "column."))
> return git_column_config(var, value, "branch", &colopts);
> if (!strcmp(var, "color.branch")) {
> branch_use_color = git_config_colorbool(var, value);
> return 0;
> }
> - if (starts_with(var, "color.branch.")) {
> - int slot = parse_branch_color_slot(var, 13);
> + if (skip_prefix(var, "color.branch.", &slot_name)) {
> + int slot = parse_branch_color_slot(var, slot_name - var);
I wonder why the separate var and offset parameters exist in the first
place. Wouldn't taking a single const char * so the old code could use
'var + 13' instead of 'var, 13' have worked?
At first glance I thought this should be
int slot = parse_branch_color_slot(slot_name, 0);
to be simpler. At second glance, how about something like the following
on top?
[...]
> --- a/builtin/log.c
> +++ b/builtin/log.c
[...]
> @@ -388,8 +390,8 @@ static int git_log_config(const char *var, const char *value, void *cb)
> default_show_root = git_config_bool(var, value);
> return 0;
> }
> - if (starts_with(var, "color.decorate."))
> - return parse_decorate_color_config(var, 15, value);
> + if (skip_prefix(var, "color.decorate.", &slot_name))
> + return parse_decorate_color_config(var, slot_name - var, value);
Likewise: the offset-based API seems odd now here, too.
[...]
> --- a/pretty.c
> +++ b/pretty.c
[...]
> @@ -809,18 +808,19 @@ static void parse_commit_header(struct format_commit_context *context)
> int i;
>
> for (i = 0; msg[i]; i++) {
> + const char *name;
ident instead of name, maybe? (since it's a 'name <email> timestamp'
field)
[...]
> @@ -1522,7 +1518,7 @@ static void pp_header(struct pretty_print_context *pp,
> int parents_shown = 0;
>
> for (;;) {
> - const char *line = *msg_p;
> + const char *name, *line = *msg_p;
Likewise.
With or without the changes below,
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
diff --git i/builtin/branch.c w/builtin/branch.c
index 6785097..022a88e 100644
--- i/builtin/branch.c
+++ w/builtin/branch.c
@@ -62,19 +62,19 @@ static unsigned char merge_filter_ref[20];
static struct string_list output = STRING_LIST_INIT_DUP;
static unsigned int colopts;
-static int parse_branch_color_slot(const char *var, int ofs)
+static int parse_branch_color_slot(const char *slot)
{
- if (!strcasecmp(var+ofs, "plain"))
+ if (!strcasecmp(slot, "plain"))
return BRANCH_COLOR_PLAIN;
- if (!strcasecmp(var+ofs, "reset"))
+ if (!strcasecmp(slot, "reset"))
return BRANCH_COLOR_RESET;
- if (!strcasecmp(var+ofs, "remote"))
+ if (!strcasecmp(slot, "remote"))
return BRANCH_COLOR_REMOTE;
- if (!strcasecmp(var+ofs, "local"))
+ if (!strcasecmp(slot, "local"))
return BRANCH_COLOR_LOCAL;
- if (!strcasecmp(var+ofs, "current"))
+ if (!strcasecmp(slot, "current"))
return BRANCH_COLOR_CURRENT;
- if (!strcasecmp(var+ofs, "upstream"))
+ if (!strcasecmp(slot, "upstream"))
return BRANCH_COLOR_UPSTREAM;
return -1;
}
@@ -90,7 +90,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(var, slot_name - var);
+ int slot = parse_branch_color_slot(slot_name);
if (slot < 0)
return 0;
if (!value)
diff --git i/builtin/log.c w/builtin/log.c
index 1202eba..68d5d30 100644
--- i/builtin/log.c
+++ w/builtin/log.c
@@ -391,7 +391,7 @@ static int git_log_config(const char *var, const char *value, void *cb)
return 0;
}
if (skip_prefix(var, "color.decorate.", &slot_name))
- return parse_decorate_color_config(var, slot_name - var, value);
+ return parse_decorate_color_config(var, slot_name, value);
if (!strcmp(var, "log.mailmap")) {
use_mailmap_config = git_config_bool(var, value);
return 0;
diff --git i/log-tree.c w/log-tree.c
index cff7ac1..6402c19 100644
--- i/log-tree.c
+++ w/log-tree.c
@@ -56,9 +56,9 @@ static int parse_decorate_color_slot(const char *slot)
return -1;
}
-int parse_decorate_color_config(const char *var, const int ofs, const char *value)
+int parse_decorate_color_config(const char *var, const char *slot_name, const char *value)
{
- int slot = parse_decorate_color_slot(var + ofs);
+ int slot = parse_decorate_color_slot(slot_name);
if (slot < 0)
return 0;
if (!value)
diff --git i/log-tree.h w/log-tree.h
index b26160c..d637b04 100644
--- i/log-tree.h
+++ w/log-tree.h
@@ -7,7 +7,8 @@ struct log_info {
struct commit *commit, *parent;
};
-int parse_decorate_color_config(const char *var, const int ofs, const char *value);
+int parse_decorate_color_config(const char *var, const char *slot_name,
+ const char *value);
void init_log_tree_opt(struct rev_info *);
int log_tree_diff_flush(struct rev_info *);
int log_tree_commit(struct rev_info *, struct commit *);
diff --git i/pretty.c w/pretty.c
index a181ac6..e2c59ef 100644
--- i/pretty.c
+++ w/pretty.c
@@ -808,19 +808,19 @@ static void parse_commit_header(struct format_commit_context *context)
int i;
for (i = 0; msg[i]; i++) {
- const char *name;
+ const char *ident;
int eol;
for (eol = i; msg[eol] && msg[eol] != '\n'; eol++)
; /* do nothing */
if (i == eol) {
break;
- } else if (skip_prefix(msg + i, "author ", &name)) {
- context->author.off = name - msg;
- context->author.len = msg + eol - name;
- } else if (skip_prefix(msg + i, "committer ", &name)) {
- context->committer.off = name - msg;
- context->committer.len = msg + eol - name;
+ } else if (skip_prefix(msg + i, "author ", &ident)) {
+ context->author.off = ident - msg;
+ context->author.len = msg + eol - ident;
+ } else if (skip_prefix(msg + i, "committer ", &ident)) {
+ context->committer.off = ident - msg;
+ context->committer.len = msg + eol - ident;
}
i = eol;
}
@@ -1518,7 +1518,7 @@ static void pp_header(struct pretty_print_context *pp,
int parents_shown = 0;
for (;;) {
- const char *name, *line = *msg_p;
+ const char *ident, *line = *msg_p;
int linelen = get_one_line(*msg_p);
if (!linelen)
@@ -1553,14 +1553,14 @@ static void pp_header(struct pretty_print_context *pp,
* FULL shows both authors but not dates.
* FULLER shows both authors and dates.
*/
- if (skip_prefix(line, "author ", &name)) {
+ if (skip_prefix(line, "author ", &ident)) {
strbuf_grow(sb, linelen + 80);
- pp_user_info(pp, "Author", sb, name, encoding);
+ pp_user_info(pp, "Author", sb, ident, encoding);
}
- if (skip_prefix(line, "committer ", &name) &&
+ if (skip_prefix(line, "committer ", &ident) &&
(pp->fmt == CMIT_FMT_FULL || pp->fmt == CMIT_FMT_FULLER)) {
strbuf_grow(sb, linelen + 80);
- pp_user_info(pp, "Commit", sb, name, encoding);
+ pp_user_info(pp, "Commit", sb, ident, encoding);
}
}
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html