revert: Make pick_commits functionally act on a commit list

Apart from its central objective of calling into the picking
mechanism, pick_commits creates a sequencer directory, prepares a todo
list, and even acts upon the "--reset" subcommand.  This makes for a
bad API since the central worry of callers is to figure out whether or
not any conflicts were encountered during the cherry picking.  The
current API is like:

  if (pick_commits(opts) < 0)
     print "Something failed, we're not sure what"

So, change pick_commits so that it's only responsible for picking
commits in a loop and reporting any errors, leaving the rest to a new
function called pick_revisions.  Consequently, the API of pick_commits
becomes much clearer:

  act_on_subcommand(opts->subcommand);
  todo_list = prepare_todo_list();
  if (pick_commits(todo_list, opts) < 0)
     print "Error encountered while picking commits"

Now, callers can easily call-in to the cherry-picking machinery by
constructing an arbitrary todo list along with some options.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/revert.c b/builtin/revert.c
index 308c1ce2ed3ece2ae773351fa8b4ff09c3b55428..f088e1cc7123442485c5e64d7e3c17aa46332a13 100644 (file)
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -727,11 +727,9 @@ static void save_opts(struct replay_opts *opts)
        }
 }
 
-static int pick_commits(struct replay_opts *opts)
+static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 {
-       struct commit_list *todo_list = NULL;
        struct strbuf buf = STRBUF_INIT;
-       unsigned char sha1[20];
        struct commit_list *cur;
        int res;
 
@@ -741,16 +739,6 @@ static int pick_commits(struct replay_opts *opts)
                                opts->record_origin || opts->edit));
        read_and_refresh_cache(opts);
 
-       walk_revs_populate_todo(&todo_list, opts);
-       create_seq_dir();
-       if (get_sha1("HEAD", sha1)) {
-               if (opts->action == REVERT)
-                       die(_("Can't revert as initial commit"));
-               die(_("Can't cherry-pick into empty head"));
-       }
-       save_head(sha1_to_hex(sha1));
-       save_opts(opts);
-
        for (cur = todo_list; cur; cur = cur->next) {
                save_todo(cur, opts);
                res = do_pick_commit(cur->item, opts);
@@ -768,6 +756,31 @@ static int pick_commits(struct replay_opts *opts)
        return 0;
 }
 
+static int pick_revisions(struct replay_opts *opts)
+{
+       struct commit_list *todo_list = NULL;
+       unsigned char sha1[20];
+
+       read_and_refresh_cache(opts);
+
+       /*
+        * Decide what to do depending on the arguments; a fresh
+        * cherry-pick should be handled differently from an existing
+        * one that is being continued
+        */
+       walk_revs_populate_todo(&todo_list, opts);
+       create_seq_dir();
+       if (get_sha1("HEAD", sha1)) {
+               if (opts->action == REVERT)
+                       die(_("Can't revert as initial commit"));
+               die(_("Can't cherry-pick into empty head"));
+       }
+       save_head(sha1_to_hex(sha1));
+       save_opts(opts);
+
+       return pick_commits(todo_list, opts);
+}
+
 int cmd_revert(int argc, const char **argv, const char *prefix)
 {
        struct replay_opts opts;
@@ -778,7 +791,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
        opts.action = REVERT;
        git_config(git_default_config, NULL);
        parse_args(argc, argv, &opts);
-       return pick_commits(&opts);
+       return pick_revisions(&opts);
 }
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
@@ -789,5 +802,5 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
        opts.action = CHERRY_PICK;
        git_config(git_default_config, NULL);
        parse_args(argc, argv, &opts);
-       return pick_commits(&opts);
+       return pick_revisions(&opts);
 }