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>
---
builtin/checkout.c | 153 +++++++++++++++++++++++++++++++----------------------
1 file changed, 89 insertions(+), 64 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3e3f086..3f1a436 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -760,8 +760,7 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
return git_xmerge_config(var, value, NULL);
}
-static int interactive_checkout(const char *revision, const char **pathspec,
- struct checkout_opts *opts)
+static int interactive_checkout(const char *revision, const char **pathspec)
{
return run_add_interactive(revision, "--patch=checkout", pathspec);
}
@@ -937,6 +936,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
char *conflict_style = NULL;
int patch_mode = 0;
int dwim_new_local_branch = 1;
+ const char **pathspec = NULL;
struct option options[] = {
OPT__QUIET(&opts.quiet, "suppress progress reporting"),
OPT_STRING('b', NULL, &opts.new_branch, "branch",
@@ -976,23 +976,45 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
- /* we can assume from now on new_branch = !new_branch_force */
- if (opts.new_branch && opts.new_branch_force)
- die(_("-B cannot be used with -b"));
-
- /* copy -B over to -b, so that we can just check the latter */
- if (opts.new_branch_force)
- opts.new_branch = opts.new_branch_force;
-
- if (patch_mode && (opts.track > 0 || opts.new_branch
- || opts.new_branch_log || opts.merge || opts.force
- || opts.force_detach))
- die (_("--patch is incompatible with all other options"));
-
+ /* 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"));
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);
+ }
+
+ /*
+ * opts.new_branch calculation, it could be from -b or -B or
+ * --track or --orphan or other command line arguments.
+ *
+ * The following code until new branch validation should not
+ * update any fields in opts but opts.new_branch.
+ */
+ if (opts.new_branch_force) {
+ /* we can assume from now on new_branch = !new_branch_force */
+ if (opts.new_branch)
+ die(_("New branch '%s' is already specified by other options"),
+ opts.new_branch);
+
+ /* copy -B over to -b, so that we can just check the latter */
+ if (opts.new_branch_force)
+ opts.new_branch = opts.new_branch_force;
+ }
+
+ if (opts.new_orphan_branch) {
+ if (opts.track > 0)
+ die(_("--orphan cannot be used with -t"));
+ if (opts.new_branch)
+ die(_("New branch '%s' is already specified by other options"),
+ opts.new_branch);
+ opts.new_branch = opts.new_orphan_branch;
+ }
/* --track without -b should DWIM */
if (0 < opts.track && !opts.new_branch) {
@@ -1009,22 +1031,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
opts.new_branch = argv0 + 1;
}
- if (opts.new_orphan_branch) {
- if (opts.new_branch)
- die(_("--orphan and -b|-B are mutually exclusive"));
- if (opts.track > 0)
- die(_("--orphan cannot be used with -t"));
- opts.new_branch = opts.new_orphan_branch;
- }
-
- if (conflict_style) {
- opts.merge = 1; /* implied */
- git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
- }
-
- if (opts.force && opts.merge)
- die(_("git checkout: -f and -m are incompatible"));
-
/*
* Extract branch name from command line arguments, so
* all that is left is pathspecs.
@@ -1050,39 +1056,29 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
argc -= n;
}
- if (opts.track == BRANCH_TRACK_UNSPECIFIED)
- opts.track = git_branch_track;
-
+ /* After DWIM, these must be pathspec */
if (argc) {
- const char **pathspec = get_pathspec(prefix, argv);
+ pathspec = get_pathspec(prefix, argv);
if (!pathspec)
die(_("invalid path specification"));
- if (patch_mode)
- return interactive_checkout(new.name, pathspec, &opts);
-
- /* Checkout paths */
- if (opts.new_branch) {
- if (argc == 1) {
- die(_("git checkout: updating paths is incompatible with switching branches.\nDid you intend to checkout '%s' which can not be resolved as commit?"), argv[0]);
- } else {
- die(_("git checkout: updating paths is incompatible with switching branches."));
- }
- }
+ /* Try to give more helpful suggestion, new_branch &&
+ argc > 1 will be caught later */
+ if (opts.new_branch && argc == 1)
+ die(_("Cannot update paths and switch to branch '%s' at the same time.\n"
+ "Did you intend to checkout '%s' which can not be resolved as commit?"),
+ opts.new_branch, argv[0]);
if (opts.force_detach)
die(_("git checkout: --detach does not take a path argument"));
if (1 < !!opts.writeout_stage + !!opts.force + !!opts.merge)
- die(_("git checkout: --ours/--theirs, --force and --merge are incompatible when\nchecking out of the index."));
-
- return checkout_paths(source_tree, pathspec, prefix, &opts);
+ die(_("git checkout: --ours/--theirs, --force and --merge are incompatible when\n"
+ "checking out of the index."));
}
- if (patch_mode)
- return interactive_checkout(new.name, NULL, &opts);
-
+ /* new branch calculation done, validate it */
if (opts.new_branch) {
struct strbuf buf = STRBUF_INIT;
@@ -1093,19 +1089,48 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
strbuf_release(&buf);
}
- if (new.name && !new.commit) {
- die(_("Cannot switch branch to a non-commit."));
- }
- if (opts.writeout_stage)
- die(_("--ours/--theirs is incompatible with switching branches."));
+ if (patch_mode || pathspec) { /* checking out entries */
+ if (opts.track > 0)
+ die(_("%s cannot be used while checking out paths"),
+ "--track");
+ if (opts.new_branch_log)
+ die(_("%s cannot be used while checking out paths"),
+ "-l");
+ if (opts.merge && patch_mode)
+ die(_("--merge cannot be used with --patch"));
+ if (opts.force && patch_mode)
+ die(_("%s cannot be used while checking out paths"),
+ "-f");
+ if (opts.force_detach)
+ die(_("%s cannot be used while checking out paths"),
+ "--detach");
+ if (opts.new_branch)
+ die(_("Cannot update paths and switch to branch '%s' at the same time."),
+ opts.new_branch);
+
+ if (patch_mode)
+ return interactive_checkout(new.name, pathspec);
+ else
+ return checkout_paths(source_tree, pathspec, prefix, &opts);
+ } else { /* switch branch */
+ if (new.name && !new.commit)
+ die(_("Cannot switch branch to a non-commit '%s'."),
+ new.name);
- if (!new.commit && opts.new_branch) {
- unsigned char rev[20];
- int flag;
+ if (opts.writeout_stage)
+ die(_("--ours/--theirs is incompatible with switching branches."));
- if (!read_ref_full("HEAD", rev, 0, &flag) &&
- (flag & REF_ISSYMREF) && is_null_sha1(rev))
- return switch_unborn_to_new_branch(&opts);
+ if (opts.track == BRANCH_TRACK_UNSPECIFIED)
+ opts.track = git_branch_track;
+
+ if (!new.commit && opts.new_branch) {
+ unsigned char rev[20];
+ int flag;
+
+ if (!read_ref_full("HEAD", rev, 0, &flag) &&
+ (flag & REF_ISSYMREF) && is_null_sha1(rev))
+ return switch_unborn_to_new_branch(&opts);
+ }
+ return switch_branches(&opts, &new);
}
- return switch_branches(&opts, &new);
}
--
1.7.12.rc2.18.g61b472e
--
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