Re: [PATCH v6 3/3] rebase: add a config option for --rebase-merges

[Glen Choo <chooglen@google.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

>> @@ -800,6 +813,15 @@ static int rebase_config(const char *var, const char *value, void *data)
>>   		return 0;
>>   	}
>>   
>> +	if (!strcmp(var, "rebase.rebasemerges")) {
>> +		opts->config_rebase_merges = git_parse_maybe_bool(value);
>> +		if (opts->config_rebase_merges < 0) {
>> +			opts->config_rebase_merges = 1;
>> +			parse_rebase_merges_value(opts, value);
>> +		}
>
> I think we need
>
> 	} else {
> 		opts->rebase_cousins = 0;
> 	}
>
> here. Otherwise if rebase.rebaseMerges is set twice we wont follow the 
> usual "last config wins" convention. For example
>
> 	[rebase]
> 		rebaseMerges=rebase-cousins
> 		rebaseMerges=true
>
> will result in us unexpectedly rebasing cousins

Ah, good catch.

An alternative might be to use the "lookup" functions (e.g.
repo_config_get_maybe_bool()), which already implement "last one wins"
semantics. It's a bigger change, but it may be easier for future readers
to understand. I don't have a strong opinion on which is the 'better'
approach.