Re: [PATCH v6 2/5] branch: make create_branch() always create a branch

[Glen Choo <chooglen@google.com>]

Jonathan Tan <jonathantanmy@google.com> writes:

> Glen Choo <chooglen@google.com> writes:
>> create_branch() was formerly used to set tracking without creating a
>> branch. Since the previous commit replaces this use case with
>> dwim_and_setup_tracking(), we can simplify create_branch() so that it
>> always creates a branch.
>> 
>> Do this simplification, in particular:
>> 
>> * remove the special handling of BRANCH_TRACK_OVERRIDE because it is no
>>   longer used
>> * assert that clobber_head_ok can only be provided with force
>> * check that we're handling clobber_head_ok and force correctly by
>>   introducing tests for `git branch --force`
>
> This might have been more simply explained as:
>
>   With the previous commit, these are true of create_branch():
>    * BRANCH_TRACK_OVERRIDE is no longer passed
>    * if clobber_head_ok is true, force is also true
>
>   Assert these situations, delete dead code, and ensure that we're
>   handling clobber_head_ok and force correctly by introducing tests for
>   `git branch --force`. This also means that create_branch() now always
>   creates a branch.

This is a great suggestion, I'll incorporate some of this.

As an aside, it has always been the case that clobber_head_ok is only true
when force is true, so it might be misleading to include it under the
umbrella of 'With the previous commit'. I'll move things around
accordingly.

>> @@ -426,15 +426,17 @@ void create_branch(struct repository *r, const char *name,
>>  	char *real_ref;
>>  	struct strbuf ref = STRBUF_INIT;
>>  	int forcing = 0;
>> -	int dont_change_ref = 0;
>> -
>> -	if ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok)
>> -	    ? validate_branchname(name, &ref)
>> -	    : validate_new_branchname(name, &ref, force)) {
>> -		if (!force)
>> -			dont_change_ref = 1;
>> -		else
>> -			forcing = 1;
>> +	struct ref_transaction *transaction;
>> +	struct strbuf err = STRBUF_INIT;
>> +	char *msg;
>> +
>> +	if (clobber_head_ok && !force)
>> +		BUG("'clobber_head_ok' can only be used with 'force'");
>> +
>> +	if (clobber_head_ok ?
>> +			  validate_branchname(name, &ref) :
>> +			  validate_new_branchname(name, &ref, force)) {
>> +		forcing = 1;
>>  	}
>
> Also assert that track is not BRANCH_TRACK_OVERRIDE.

Makes sense, I'll do that.