Re: [PATCH 2/3] checkout: reorder option handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 29, 2012 at 3:45 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> +     /* Easy mode-independent incompatible checks */
>>       if (opts.force_detach && (opts.new_branch || opts.new_orphan_branch))
>>               die(_("--detach cannot be used with -b/-B/--orphan"));
>
> Did you catch "--detach -B" combination without checking new_branch_force?

I did not. Will move this check further down once opts.new_branch is final.

>>       if (opts.force_detach && 0 < opts.track)
>>               die(_("--detach cannot be used with -t"));
>> +     if (opts.force && opts.merge)
>> +             die(_("git checkout: -f and -m are incompatible"));
>> +
>> +     if (conflict_style) {
>> +             opts.merge = 1; /* implied */
>> +             git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
>> +     }
>
> "checkout --conflict=diff3 -f <branch>" would imply combination of
> "-m" and "-f", which is supposed to be forbidden in the previous
> check, no?

Yep. That last "if" must be moved up.

> I very much like the idea of separating things in phases like your
> proposed log message explains.  But I wonder if the order should be
> to do dwimming and parameter canonicalization first, then decide the
> mode (these might have to be swapped, as the same parameter may dwim
> down to different things depending on the mode), and finally check
> for sanity before performing.

I do a few sanity checks after the mode has been decided and obviously
missed some as you pointed out above. Will move them all down. So
parameter canonicalization, dwim, then separate code paths for each
mode, which includes sanity checks.

> To avoid confusion, it also might not be a bad idea to remove
> new_branch_force and new_orphan_branch from the structure and
> introduce "enum branch_creation_type" or something, and always have
> the new branch name in "new_branch" field (this needs to get various
> pointers into opts out of the parseopt options[] array; parse into
> separate variables and decide what to put in "struct checkout_opts"),
> independent from how that branch is going to be created (either -b,
> -B, or --orphan).

OK
-- 
Duy
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]