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>

diff --git a/builtin/revert.c b/builtin/revert.c
index ba5a88cbfd279cc6a408045d8e205d3c68fa9f7c..4ca5b515449a355ae04f0ff496a8ee0b7143e29a 100644 (file)
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -71,7 +71,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
                die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
 }
 
                die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
 }
 

-static void parse_args(int argc, const char **argv, struct replay_opts *opts)
+static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)

 {
        const char * const * usage_str = revert_or_cherry_pick_usage(opts);
        const char *me = action_name(opts);
 {
        const char * const * usage_str = revert_or_cherry_pick_usage(opts);
        const char *me = action_name(opts);


@@ -115,25 +115,15 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
        if (opts->keep_redundant_commits)
                opts->allow_empty = 1;
 
        if (opts->keep_redundant_commits)
                opts->allow_empty = 1;
 

-       /* Set the subcommand */
-       if (cmd == 'q')
-               opts->subcommand = REPLAY_REMOVE_STATE;
-       else if (cmd == 'c')
-               opts->subcommand = REPLAY_CONTINUE;
-       else if (cmd == 'a')
-               opts->subcommand = REPLAY_ROLLBACK;
-       else
-               opts->subcommand = REPLAY_NONE;
-

        /* Check for incompatible command line arguments */
        /* Check for incompatible command line arguments */

-       if (opts->subcommand != REPLAY_NONE) {
+       if (cmd) {

                char *this_operation;
                char *this_operation;

-               if (opts->subcommand == REPLAY_REMOVE_STATE)
+               if (cmd == 'q')

                        this_operation = "--quit";
                        this_operation = "--quit";

-               else if (opts->subcommand == REPLAY_CONTINUE)
+               else if (cmd == 'c')

                        this_operation = "--continue";
                else {
                        this_operation = "--continue";
                else {

-                       assert(opts->subcommand == REPLAY_ROLLBACK);
+                       assert(cmd == 'a');

                        this_operation = "--abort";
                }
 
                        this_operation = "--abort";
                }
 


@@ -156,7 +146,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
                                "--edit", opts->edit,
                                NULL);
 
                                "--edit", opts->edit,
                                NULL);
 

-       if (opts->subcommand != REPLAY_NONE) {
+       if (cmd) {

                opts->revs = NULL;
        } else {
                struct setup_revision_opt s_r_opt;
                opts->revs = NULL;
        } else {
                struct setup_revision_opt s_r_opt;


@@ -178,6 +168,14 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
        /* These option values will be free()d */
        opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
        opts->strategy = xstrdup_or_null(opts->strategy);
        /* These option values will be free()d */
        opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
        opts->strategy = xstrdup_or_null(opts->strategy);

+
+       if (cmd == 'q')
+               return sequencer_remove_state(opts);
+       if (cmd == 'c')
+               return sequencer_continue(opts);
+       if (cmd == 'a')
+               return sequencer_rollback(opts);
+       return sequencer_pick_revisions(opts);

 }
 
 int cmd_revert(int argc, const char **argv, const char *prefix)
 }
 
 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)
                opts.edit = 1;
        opts.action = REPLAY_REVERT;
        git_config(git_default_config, NULL);
                opts.edit = 1;
        opts.action = REPLAY_REVERT;
        git_config(git_default_config, NULL);

-       parse_args(argc, argv, &opts);
-       res = sequencer_pick_revisions(&opts);
+       res = run_sequencer(argc, argv, &opts);

        if (res < 0)
                die(_("revert failed"));
        return res;
        if (res < 0)
                die(_("revert failed"));
        return res;


@@ -203,8 +200,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 
        opts.action = REPLAY_PICK;
        git_config(git_default_config, NULL);
 
        opts.action = REPLAY_PICK;
        git_config(git_default_config, NULL);

-       parse_args(argc, argv, &opts);
-       res = sequencer_pick_revisions(&opts);
+       res = run_sequencer(argc, argv, &opts);

        if (res < 0)
                die(_("cherry-pick failed"));
        return res;
        if (res < 0)
                die(_("cherry-pick failed"));
        return res;

diff --git a/sequencer.c b/sequencer.c
index 120a8ee3512b23662e4b3ff9bfd200b7bfad23cb..9f22c5ec410e40b4725f7f90d24010096ca5cbca 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -119,7 +119,7 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
        return 1;
 }
 
        return 1;
 }
 

-static void remove_sequencer_state(const struct replay_opts *opts)
+int sequencer_remove_state(struct replay_opts *opts)

 {
        struct strbuf dir = STRBUF_INIT;
        int i;
 {
        struct strbuf dir = STRBUF_INIT;
        int i;


@@ -133,6 +133,8 @@ static void remove_sequencer_state(const struct replay_opts *opts)
        strbuf_addf(&dir, "%s", get_dir(opts));
        remove_dir_recursively(&dir, 0);
        strbuf_release(&dir);
        strbuf_addf(&dir, "%s", get_dir(opts));
        remove_dir_recursively(&dir, 0);
        strbuf_release(&dir);

+
+       return 0;

 }
 
 static const char *action_name(const struct replay_opts *opts)
 }
 
 static const char *action_name(const struct replay_opts *opts)


@@ -975,7 +977,7 @@ static int rollback_single_pick(void)
        return reset_for_rollback(head_sha1);
 }
 
        return reset_for_rollback(head_sha1);
 }
 

-static int sequencer_rollback(struct replay_opts *opts)
+int sequencer_rollback(struct replay_opts *opts)

 {
        FILE *f;
        unsigned char sha1[20];
 {
        FILE *f;
        unsigned char sha1[20];


@@ -1010,9 +1012,8 @@ static int sequencer_rollback(struct replay_opts *opts)
        }
        if (reset_for_rollback(sha1))
                goto fail;
        }
        if (reset_for_rollback(sha1))
                goto fail;

-       remove_sequencer_state(opts);

        strbuf_release(&buf);
        strbuf_release(&buf);

-       return 0;
+       return sequencer_remove_state(opts);

 fail:
        strbuf_release(&buf);
        return -1;
 fail:
        strbuf_release(&buf);
        return -1;


@@ -1097,8 +1098,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
         * Sequence of picks finished successfully; cleanup by
         * removing the .git/sequencer directory
         */
         * Sequence of picks finished successfully; cleanup by
         * removing the .git/sequencer directory
         */

-       remove_sequencer_state(opts);
-       return 0;
+       return sequencer_remove_state(opts);

 }
 
 static int continue_single_pick(void)
 }
 
 static int continue_single_pick(void)


@@ -1111,11 +1111,14 @@ static int continue_single_pick(void)
        return run_command_v_opt(argv, RUN_GIT_CMD);
 }
 
        return run_command_v_opt(argv, RUN_GIT_CMD);
 }
 

-static int sequencer_continue(struct replay_opts *opts)
+int sequencer_continue(struct replay_opts *opts)

 {
        struct todo_list todo_list = TODO_LIST_INIT;
        int res;
 
 {
        struct todo_list todo_list = TODO_LIST_INIT;
        int res;
 

+       if (read_and_refresh_cache(opts))
+               return -1;
+

        if (!file_exists(get_todo_path(opts)))
                return continue_single_pick();
        if (read_populate_opts(opts))
        if (!file_exists(get_todo_path(opts)))
                return continue_single_pick();
        if (read_populate_opts(opts))


@@ -1154,26 +1157,10 @@ int sequencer_pick_revisions(struct replay_opts *opts)
        unsigned char sha1[20];
        int i, res;
 
        unsigned char sha1[20];
        int i, res;
 

-       if (opts->subcommand == REPLAY_NONE)
-               assert(opts->revs);
-
+       assert(opts->revs);

        if (read_and_refresh_cache(opts))
                return -1;
 
        if (read_and_refresh_cache(opts))
                return -1;
 

-       /*
-        * Decide what to do depending on the arguments; a fresh
-        * cherry-pick should be handled differently from an existing
-        * one that is being continued
-        */
-       if (opts->subcommand == REPLAY_REMOVE_STATE) {
-               remove_sequencer_state(opts);
-               return 0;
-       }
-       if (opts->subcommand == REPLAY_ROLLBACK)
-               return sequencer_rollback(opts);
-       if (opts->subcommand == REPLAY_CONTINUE)
-               return sequencer_continue(opts);
-

        for (i = 0; i < opts->revs->pending.nr; i++) {
                unsigned char sha1[20];
                const char *name = opts->revs->pending.objects[i].name;
        for (i = 0; i < opts->revs->pending.nr; i++) {
                unsigned char sha1[20];
                const char *name = opts->revs->pending.objects[i].name;

diff --git a/sequencer.h b/sequencer.h
index 8453669765b38cca7203fc7634c511a3ee652d68..7a513c576bdccf8730828fdaa586ec9d0af4af6b 100644 (file)
--- a/sequencer.h
+++ b/sequencer.h
@@ -10,16 +10,8 @@ enum replay_action {
        REPLAY_PICK
 };
 
        REPLAY_PICK
 };
 

-enum replay_subcommand {
-       REPLAY_NONE,
-       REPLAY_REMOVE_STATE,
-       REPLAY_CONTINUE,
-       REPLAY_ROLLBACK
-};
-

 struct replay_opts {
        enum replay_action action;
 struct replay_opts {
        enum replay_action action;

-       enum replay_subcommand subcommand;

 
        /* Boolean options */
        int edit;
 
        /* Boolean options */
        int edit;


@@ -44,9 +36,12 @@ struct replay_opts {
        /* Only used by REPLAY_NONE */
        struct rev_info *revs;
 };
        /* Only used by REPLAY_NONE */
        struct rev_info *revs;
 };

-#define REPLAY_OPTS_INIT { -1, -1 }
+#define REPLAY_OPTS_INIT { -1 }

 
 int sequencer_pick_revisions(struct replay_opts *opts);
 
 int sequencer_pick_revisions(struct replay_opts *opts);

+int sequencer_continue(struct replay_opts *opts);
+int sequencer_rollback(struct replay_opts *opts);
+int sequencer_remove_state(struct replay_opts *opts);

 
 extern const char sign_off_header[];
 
 
 extern const char sign_off_header[];