checkout: reorder option handling
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 that - cmd_checkout() is responsible for parsing, preparing input and determining mode - Code of each mode is in checkout_paths() and checkout_branch(), where sanity checks are performed 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@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Nguyễn Thái Ngọc Duy committed
Aug 30, 2012 at 19:45 UTC
b6312c27a06f31625d915d44531d5e728ffd8d63
1 file changed
+112
-77
builtin/checkout.c
+112
-77
@@ -217,7 +217,8 @@ static int checkout_merged(int pos, struct checkout *state)
217
return status;
218
}
219
220
-static int checkout_paths(const struct checkout_opts *opts)
220
+static int checkout_paths(const struct checkout_opts *opts,
221
+ const char *revision)
222
{
223
int pos;
224
struct checkout state;
@@ -229,7 +230,35 @@ static int checkout_paths(const struct checkout_opts *opts)
230
int stage = opts->writeout_stage;
231
int merge = opts->merge;
232
int newfd;
232
- struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
233
+ struct lock_file *lock_file;
234
+
235
+ if (opts->track != BRANCH_TRACK_UNSPECIFIED)
236
+ die(_("'%s' cannot be used with updating paths"), "--track");
237
+
238
+ if (opts->new_branch_log)
239
+ die(_("'%s' cannot be used with updating paths"), "-l");
240
+
241
+ if (opts->force && opts->patch_mode)
242
+ die(_("'%s' cannot be used with updating paths"), "-f");
243
+
244
+ if (opts->force_detach)
245
+ die(_("'%s' cannot be used with updating paths"), "--detach");
246
+
247
+ if (opts->merge && opts->patch_mode)
248
+ die(_("'%s' cannot be used with %s"), "--merge", "--patch");
249
+
250
+ if (opts->force && opts->merge)
251
+ die(_("'%s' cannot be used with %s"), "-f", "-m");
252
+
253
+ if (opts->new_branch)
254
+ die(_("Cannot update paths and switch to branch '%s' at the same time."),
255
+ opts->new_branch);
256
+
257
+ if (opts->patch_mode)
258
+ return run_add_interactive(revision, "--patch=checkout",
259
+ opts->pathspec);
260
+
261
+ lock_file = xcalloc(1, sizeof(struct lock_file));
262
263
newfd = hold_locked_index(lock_file, 1);
264
if (read_cache_preload(opts->pathspec) < 0)
@@ -763,11 +792,6 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
792
return git_xmerge_config(var, value, NULL);
793
}
794
766
-static int interactive_checkout(const char *revision, const char **pathspec)
767
-{
768
- return run_add_interactive(revision, "--patch=checkout", pathspec);
769
-}
770
-
795
struct tracking_name_data {
796
const char *name;
797
char *remote;
@@ -930,6 +954,51 @@ static int switch_unborn_to_new_branch(const struct checkout_opts *opts)
954
return status;
955
}
956
957
+static int checkout_branch(struct checkout_opts *opts,
958
+ struct branch_info *new)
959
+{
960
+ if (opts->pathspec)
961
+ die(_("paths cannot be used with switching branches"));
962
+
963
+ if (opts->patch_mode)
964
+ die(_("'%s' cannot be used with switching branches"),
965
+ "--patch");
966
+
967
+ if (opts->writeout_stage)
968
+ die(_("'%s' cannot be used with switching branches"),
969
+ "--ours/--theirs");
970
+
971
+ if (opts->force && opts->merge)
972
+ die(_("'%s' cannot be used with '%s'"), "-f", "-m");
973
+
974
+ if (opts->force_detach && opts->new_branch)
975
+ die(_("'%s' cannot be used with '%s'"),
976
+ "--detach", "-b/-B/--orphan");
977
+
978
+ if (opts->new_orphan_branch) {
979
+ if (opts->track != BRANCH_TRACK_UNSPECIFIED)
980
+ die(_("'%s' cannot be used with '%s'"), "--orphan", "-t");
981
+ } else if (opts->force_detach) {
982
+ if (opts->track != BRANCH_TRACK_UNSPECIFIED)
983
+ die(_("'%s' cannot be used with '%s'"), "--detach", "-t");
984
+ } else if (opts->track == BRANCH_TRACK_UNSPECIFIED)
985
+ opts->track = git_branch_track;
986
+
987
+ if (new->name && !new->commit)
988
+ die(_("Cannot switch branch to a non-commit '%s'"),
989
+ new->name);
990
+
991
+ if (!new->commit && opts->new_branch) {
992
+ unsigned char rev[20];
993
+ int flag;
994
+
995
+ if (!read_ref_full("HEAD", rev, 0, &flag) &&
996
+ (flag & REF_ISSYMREF) && is_null_sha1(rev))
997
+ return switch_unborn_to_new_branch(opts);
998
+ }
999
+ return switch_branches(opts, new);
1000
+}
1001
+
1002
int cmd_checkout(int argc, const char **argv, const char *prefix)
1003
{
1004
struct checkout_opts opts;
@@ -976,26 +1045,27 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
1045
argc = parse_options(argc, argv, prefix, options, checkout_usage,
1046
PARSE_OPT_KEEP_DASHDASH);
1047
979
- /* we can assume from now on new_branch = !new_branch_force */
980
- if (opts.new_branch && opts.new_branch_force)
981
- die(_("-B cannot be used with -b"));
1048
+ if (conflict_style) {
1049
+ opts.merge = 1; /* implied */
1050
+ git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
1051
+ }
1052
983
- /* copy -B over to -b, so that we can just check the latter */
1053
+ if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)
1054
+ die(_("-b, -B and --orphan are mutually exclusive"));
1055
+
1056
+ /*
1057
+ * From here on, new_branch will contain the branch to be checked out,
1058
+ * and new_branch_force and new_orphan_branch will tell us which one of
1059
+ * -b/-B/--orphan is being used.
1060
+ */
1061
if (opts.new_branch_force)
1062
opts.new_branch = opts.new_branch_force;
1063
987
- if (opts.patch_mode && (opts.track > 0 || opts.new_branch
988
- || opts.new_branch_log || opts.merge || opts.force
989
- || opts.force_detach))
990
- die (_("--patch is incompatible with all other options"));
991
-
992
- if (opts.force_detach && (opts.new_branch || opts.new_orphan_branch))
993
- die(_("--detach cannot be used with -b/-B/--orphan"));
994
- if (opts.force_detach && 0 < opts.track)
995
- die(_("--detach cannot be used with -t"));
1064
+ if (opts.new_orphan_branch)
1065
+ opts.new_branch = opts.new_orphan_branch;
1066
997
- /* --track without -b should DWIM */
998
- if (0 < opts.track && !opts.new_branch) {
1067
+ /* --track without -b/-B/--orphan should DWIM */
1068
+ if (opts.track != BRANCH_TRACK_UNSPECIFIED && !opts.new_branch) {
1069
const char *argv0 = argv[0];
1070
if (!argc || !strcmp(argv0, "--"))
1071
die (_("--track needs a branch name"));
@@ -1009,22 +1079,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
1079
opts.new_branch = argv0 + 1;
1080
}
1081
1012
- if (opts.new_orphan_branch) {
1013
- if (opts.new_branch)
1014
- die(_("--orphan and -b|-B are mutually exclusive"));
1015
- if (opts.track > 0)
1016
- die(_("--orphan cannot be used with -t"));
1017
- opts.new_branch = opts.new_orphan_branch;
1018
- }
1019
-
1020
- if (conflict_style) {
1021
- opts.merge = 1; /* implied */
1022
- git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
1023
- }
1024
-
1025
- if (opts.force && opts.merge)
1026
- die(_("git checkout: -f and -m are incompatible"));
1027
-
1082
/*
1083
* Extract branch name from command line arguments, so
1084
* all that is left is pathspecs.
@@ -1052,62 +1106,43 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
1106
argc -= n;
1107
}
1108
1055
- if (opts.track == BRANCH_TRACK_UNSPECIFIED)
1056
- opts.track = git_branch_track;
1057
-
1109
if (argc) {
1110
opts.pathspec = get_pathspec(prefix, argv);
1111
1112
if (!opts.pathspec)
1113
die(_("invalid path specification"));
1114
1064
- if (opts.patch_mode)
1065
- return interactive_checkout(new.name, opts.pathspec);
1066
-
1067
- /* Checkout paths */
1068
- if (opts.new_branch) {
1069
- if (argc == 1) {
1070
- 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]);
1071
- } else {
1072
- die(_("git checkout: updating paths is incompatible with switching branches."));
1073
- }
1074
- }
1115
+ /*
1116
+ * Try to give more helpful suggestion.
1117
+ * new_branch && argc > 1 will be caught later.
1118
+ */
1119
+ if (opts.new_branch && argc == 1)
1120
+ die(_("Cannot update paths and switch to branch '%s' at the same time.\n"
1121
+ "Did you intend to checkout '%s' which can not be resolved as commit?"),
1122
+ opts.new_branch, argv[0]);
1123
1124
if (opts.force_detach)
1077
- die(_("git checkout: --detach does not take a path argument"));
1125
+ die(_("git checkout: --detach does not take a path argument '%s'"),
1126
+ argv[0]);
1127
1128
if (1 < !!opts.writeout_stage + !!opts.force + !!opts.merge)
1080
- die(_("git checkout: --ours/--theirs, --force and --merge are incompatible when\nchecking out of the index."));
1081
-
1082
- return checkout_paths(&opts);
1129
+ die(_("git checkout: --ours/--theirs, --force and --merge are incompatible when\n"
1130
+ "checking out of the index."));
1131
}
1132
1085
- if (opts.patch_mode)
1086
- return interactive_checkout(new.name, NULL);
1087
-
1133
if (opts.new_branch) {
1134
struct strbuf buf = STRBUF_INIT;
1135
1091
- opts.branch_exists = validate_new_branchname(opts.new_branch, &buf,
1092
- !!opts.new_branch_force,
1093
- !!opts.new_branch_force);
1136
+ opts.branch_exists =
1137
+ validate_new_branchname(opts.new_branch, &buf,
1138
+ !!opts.new_branch_force,
1139
+ !!opts.new_branch_force);
1140
1141
strbuf_release(&buf);
1142
}
1143
1098
- if (new.name && !new.commit) {
1099
- die(_("Cannot switch branch to a non-commit."));
1100
- }
1101
- if (opts.writeout_stage)
1102
- die(_("--ours/--theirs is incompatible with switching branches."));
1103
-
1104
- if (!new.commit && opts.new_branch) {
1105
- unsigned char rev[20];
1106
- int flag;
1107
-
1108
- if (!read_ref_full("HEAD", rev, 0, &flag) &&
1109
- (flag & REF_ISSYMREF) && is_null_sha1(rev))
1110
- return switch_unborn_to_new_branch(&opts);
1111
- }
1112
- return switch_branches(&opts, &new);
1144
+ if (opts.patch_mode || opts.pathspec)
1145
+ return checkout_paths(&opts, new.name);
1146
+ else
1147
+ return checkout_branch(&opts, &new);
1148
}