sequencer: get rid of the subcommand field
The subcommands are used exactly once, at the very beginning of sequencer_pick_revisions(), to determine what to do. This is an unnecessary level of indirection: we can simply call the correct function to begin with. So let's do that. While at it, ensure that the subcommands return an error code so that they do not have to die() all over the place (bad practice for library functions...). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin committed
Oct 21, 2016 at 14:24 UTC
2863584f5cf98c5f768e24f4841e3df14cbea59a
3 files changed
+31
-53
builtin/revert.c
+16
-20
@@ -71,7 +71,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
71
die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
72
}
73
74
-static void parse_args(int argc, const char **argv, struct replay_opts *opts)
74
+static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
75
{
76
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
77
const char *me = action_name(opts);
@@ -115,25 +115,15 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
115
if (opts->keep_redundant_commits)
116
opts->allow_empty = 1;
117
118
- /* Set the subcommand */
119
- if (cmd == 'q')
120
- opts->subcommand = REPLAY_REMOVE_STATE;
121
- else if (cmd == 'c')
122
- opts->subcommand = REPLAY_CONTINUE;
123
- else if (cmd == 'a')
124
- opts->subcommand = REPLAY_ROLLBACK;
125
- else
126
- opts->subcommand = REPLAY_NONE;
127
-
118
/* Check for incompatible command line arguments */
129
- if (opts->subcommand != REPLAY_NONE) {
119
+ if (cmd) {
120
char *this_operation;
131
- if (opts->subcommand == REPLAY_REMOVE_STATE)
121
+ if (cmd == 'q')
122
this_operation = "--quit";
133
- else if (opts->subcommand == REPLAY_CONTINUE)
123
+ else if (cmd == 'c')
124
this_operation = "--continue";
125
else {
136
- assert(opts->subcommand == REPLAY_ROLLBACK);
126
+ assert(cmd == 'a');
127
this_operation = "--abort";
128
}
129
@@ -156,7 +146,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
146
"--edit", opts->edit,
147
NULL);
148
159
- if (opts->subcommand != REPLAY_NONE) {
149
+ if (cmd) {
150
opts->revs = NULL;
151
} else {
152
struct setup_revision_opt s_r_opt;
@@ -178,6 +168,14 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
168
/* These option values will be free()d */
169
opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
170
opts->strategy = xstrdup_or_null(opts->strategy);
171
+
172
+ if (cmd == 'q')
173
+ return sequencer_remove_state(opts);
174
+ if (cmd == 'c')
175
+ return sequencer_continue(opts);
176
+ if (cmd == 'a')
177
+ return sequencer_rollback(opts);
178
+ return sequencer_pick_revisions(opts);
179
}
180
181
int cmd_revert(int argc, const char **argv, const char *prefix)
@@ -189,8 +187,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
187
opts.edit = 1;
188
opts.action = REPLAY_REVERT;
189
git_config(git_default_config, NULL);
192
- parse_args(argc, argv, &opts);
193
- res = sequencer_pick_revisions(&opts);
190
+ res = run_sequencer(argc, argv, &opts);
191
if (res < 0)
192
die(_("revert failed"));
193
return res;
@@ -203,8 +200,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
200
201
opts.action = REPLAY_PICK;
202
git_config(git_default_config, NULL);
206
- parse_args(argc, argv, &opts);
207
- res = sequencer_pick_revisions(&opts);
203
+ res = run_sequencer(argc, argv, &opts);
204
if (res < 0)
205
die(_("cherry-pick failed"));
206
return res;
sequencer.c
+11
-24
@@ -119,7 +119,7 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
119
return 1;
120
}
121
122
-static void remove_sequencer_state(const struct replay_opts *opts)
122
+int sequencer_remove_state(struct replay_opts *opts)
123
{
124
struct strbuf dir = STRBUF_INIT;
125
int i;
@@ -133,6 +133,8 @@ static void remove_sequencer_state(const struct replay_opts *opts)
133
strbuf_addf(&dir, "%s", get_dir(opts));
134
remove_dir_recursively(&dir, 0);
135
strbuf_release(&dir);
136
+
137
+ return 0;
138
}
139
140
static const char *action_name(const struct replay_opts *opts)
@@ -975,7 +977,7 @@ static int rollback_single_pick(void)
977
return reset_for_rollback(head_sha1);
978
}
979
978
-static int sequencer_rollback(struct replay_opts *opts)
980
+int sequencer_rollback(struct replay_opts *opts)
981
{
982
FILE *f;
983
unsigned char sha1[20];
@@ -1010,9 +1012,8 @@ static int sequencer_rollback(struct replay_opts *opts)
1012
}
1013
if (reset_for_rollback(sha1))
1014
goto fail;
1013
- remove_sequencer_state(opts);
1015
strbuf_release(&buf);
1015
- return 0;
1016
+ return sequencer_remove_state(opts);
1017
fail:
1018
strbuf_release(&buf);
1019
return -1;
@@ -1097,8 +1098,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
1098
* Sequence of picks finished successfully; cleanup by
1099
* removing the .git/sequencer directory
1100
*/
1100
- remove_sequencer_state(opts);
1101
- return 0;
1101
+ return sequencer_remove_state(opts);
1102
}
1103
1104
static int continue_single_pick(void)
@@ -1111,11 +1111,14 @@ static int continue_single_pick(void)
1111
return run_command_v_opt(argv, RUN_GIT_CMD);
1112
}
1113
1114
-static int sequencer_continue(struct replay_opts *opts)
1114
+int sequencer_continue(struct replay_opts *opts)
1115
{
1116
struct todo_list todo_list = TODO_LIST_INIT;
1117
int res;
1118
1119
+ if (read_and_refresh_cache(opts))
1120
+ return -1;
1121
+
1122
if (!file_exists(get_todo_path(opts)))
1123
return continue_single_pick();
1124
if (read_populate_opts(opts))
@@ -1154,26 +1157,10 @@ int sequencer_pick_revisions(struct replay_opts *opts)
1157
unsigned char sha1[20];
1158
int i, res;
1159
1157
- if (opts->subcommand == REPLAY_NONE)
1158
- assert(opts->revs);
1159
-
1160
+ assert(opts->revs);
1161
if (read_and_refresh_cache(opts))
1162
return -1;
1163
1163
- /*
1164
- * Decide what to do depending on the arguments; a fresh
1165
- * cherry-pick should be handled differently from an existing
1166
- * one that is being continued
1167
- */
1168
- if (opts->subcommand == REPLAY_REMOVE_STATE) {
1169
- remove_sequencer_state(opts);
1170
- return 0;
1171
- }
1172
- if (opts->subcommand == REPLAY_ROLLBACK)
1173
- return sequencer_rollback(opts);
1174
- if (opts->subcommand == REPLAY_CONTINUE)
1175
- return sequencer_continue(opts);
1176
-
1164
for (i = 0; i < opts->revs->pending.nr; i++) {
1165
unsigned char sha1[20];
1166
const char *name = opts->revs->pending.objects[i].name;
sequencer.h
+4
-9
@@ -10,16 +10,8 @@ enum replay_action {
10
REPLAY_PICK
11
};
12
13
-enum replay_subcommand {
14
- REPLAY_NONE,
15
- REPLAY_REMOVE_STATE,
16
- REPLAY_CONTINUE,
17
- REPLAY_ROLLBACK
18
-};
19
-
13
struct replay_opts {
14
enum replay_action action;
22
- enum replay_subcommand subcommand;
15
16
/* Boolean options */
17
int edit;
@@ -44,9 +36,12 @@ struct replay_opts {
36
/* Only used by REPLAY_NONE */
37
struct rev_info *revs;
38
};
47
-#define REPLAY_OPTS_INIT { -1, -1 }
39
+#define REPLAY_OPTS_INIT { -1 }
40
41
int sequencer_pick_revisions(struct replay_opts *opts);
42
+int sequencer_continue(struct replay_opts *opts);
43
+int sequencer_rollback(struct replay_opts *opts);
44
+int sequencer_remove_state(struct replay_opts *opts);
45
46
extern const char sign_off_header[];
47