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>

Ramkumar Ramachandra committed Aug 4, 2011 at 16:09 UTC 21b14778a9b033d80b3e5757d9576da13ba446cd
1 file changed +28 -15
builtin/revert.c
+28 -15
@@ -727,11 +727,9 @@ static void save_opts(struct replay_opts *opts)
727 }
728 }
729
730 -static int pick_commits(struct replay_opts *opts)
730 +static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
731 {
732 - struct commit_list *todo_list = NULL;
732 struct strbuf buf = STRBUF_INIT;
734 - unsigned char sha1[20];
733 struct commit_list *cur;
734 int res;
735
@@ -741,16 +739,6 @@ static int pick_commits(struct replay_opts *opts)
739 opts->record_origin || opts->edit));
740 read_and_refresh_cache(opts);
741
744 - walk_revs_populate_todo(&todo_list, opts);
745 - create_seq_dir();
746 - if (get_sha1("HEAD", sha1)) {
747 - if (opts->action == REVERT)
748 - die(_("Can't revert as initial commit"));
749 - die(_("Can't cherry-pick into empty head"));
750 - }
751 - save_head(sha1_to_hex(sha1));
752 - save_opts(opts);
753 -
742 for (cur = todo_list; cur; cur = cur->next) {
743 save_todo(cur, opts);
744 res = do_pick_commit(cur->item, opts);
@@ -768,6 +756,31 @@ static int pick_commits(struct replay_opts *opts)
756 return 0;
757 }
758
759 +static int pick_revisions(struct replay_opts *opts)
760 +{
761 + struct commit_list *todo_list = NULL;
762 + unsigned char sha1[20];
763 +
764 + read_and_refresh_cache(opts);
765 +
766 + /*
767 + * Decide what to do depending on the arguments; a fresh
768 + * cherry-pick should be handled differently from an existing
769 + * one that is being continued
770 + */
771 + walk_revs_populate_todo(&todo_list, opts);
772 + create_seq_dir();
773 + if (get_sha1("HEAD", sha1)) {
774 + if (opts->action == REVERT)
775 + die(_("Can't revert as initial commit"));
776 + die(_("Can't cherry-pick into empty head"));
777 + }
778 + save_head(sha1_to_hex(sha1));
779 + save_opts(opts);
780 +
781 + return pick_commits(todo_list, opts);
782 +}
783 +
784 int cmd_revert(int argc, const char **argv, const char *prefix)
785 {
786 struct replay_opts opts;
@@ -778,7 +791,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
791 opts.action = REVERT;
792 git_config(git_default_config, NULL);
793 parse_args(argc, argv, &opts);
781 - return pick_commits(&opts);
794 + return pick_revisions(&opts);
795 }
796
797 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)
802 opts.action = CHERRY_PICK;
803 git_config(git_default_config, NULL);
804 parse_args(argc, argv, &opts);
792 - return pick_commits(&opts);
805 + return pick_revisions(&opts);
806 }