Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:
> Glen Choo <chooglen@xxxxxxxxxx> 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.
