Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> checkout operates in three different modes. On top of that it tries to
> be smart by guessing the branch name for switching. This results in
> messy option handling code. This patch reorders it so:
>
> - easy option handling comes first
> - the big chunk of branch name guessing comes next
> - mode detection comes last. Once the mode is known, check again to
> see if users specify any incompatible options
> - the actual action is done
>
> Another slight improvement is always print branch name (or commit
> name) when printing errors related ot them. This helps catch the case
> where an option is mistaken as branch/commit.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> ...
> + /* 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?
> 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?
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.
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).
--
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