Jeff King <peff@xxxxxxxx> writes:
> On Sun, Oct 05, 2014 at 03:49:19PM -0700, Jonathan Nieder wrote:
>
>> > --- 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?
>
> I think this is in the same boat as parse_diff_color_slot, which I fixed
> in 9e1a5eb (parse_diff_color_slot: drop ofs parameter, 2014-06-18). The
> short of it is that the function used to want both the full name and the
> slot name, but now needs only the latter.
>
> The fix you proposed below is along the same line, and looks good to me
> (and grepping for 'var *+ *ofs' shows only the two sites you found, so
> hopefully that is the last of it).
So perhaps we would want this change as a preliminary separate
patch?
Thanks
>
>> > @@ -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)
>
> Yeah, agreed.
>
> -Peff
--
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