Skip to content

Commit 21b1477

Browse files
artagnongitster
authored andcommitted
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 <[email protected]> Signed-off-by: Jonathan Nieder <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 6f03226 commit 21b1477

File tree

1 file changed

+28
-15
lines changed

1 file changed

+28
-15
lines changed

builtin/revert.c

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -727,11 +727,9 @@ static void save_opts(struct replay_opts *opts)
727727
}
728728
}
729729

730-
static int pick_commits(struct replay_opts *opts)
730+
static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
731731
{
732-
struct commit_list *todo_list = NULL;
733732
struct strbuf buf = STRBUF_INIT;
734-
unsigned char sha1[20];
735733
struct commit_list *cur;
736734
int res;
737735

@@ -741,16 +739,6 @@ static int pick_commits(struct replay_opts *opts)
741739
opts->record_origin || opts->edit));
742740
read_and_refresh_cache(opts);
743741

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-
754742
for (cur = todo_list; cur; cur = cur->next) {
755743
save_todo(cur, opts);
756744
res = do_pick_commit(cur->item, opts);
@@ -768,6 +756,31 @@ static int pick_commits(struct replay_opts *opts)
768756
return 0;
769757
}
770758

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+
771784
int cmd_revert(int argc, const char **argv, const char *prefix)
772785
{
773786
struct replay_opts opts;
@@ -778,7 +791,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
778791
opts.action = REVERT;
779792
git_config(git_default_config, NULL);
780793
parse_args(argc, argv, &opts);
781-
return pick_commits(&opts);
794+
return pick_revisions(&opts);
782795
}
783796

784797
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)
789802
opts.action = CHERRY_PICK;
790803
git_config(git_default_config, NULL);
791804
parse_args(argc, argv, &opts);
792-
return pick_commits(&opts);
805+
return pick_revisions(&opts);
793806
}

0 commit comments

Comments
 (0)