Hi Jonathan, Jonathan Nieder writes: > Well, "beautiful public API" means "just what cmd_cherry_pick and > cmd_revert needs", right? So I'd suggest: Yes, but I don't want to put stuff that's too specific to cherry-pick/ revert in the sequencer. > [...] > Luckily step (1) is already done. The functions are parse_args() > and pick_revisions() (though they could presumably use less generic > names). Are you certain about pick_revisions? I've copied over the function here for your reference. My issue is that it's too specific cherry-pick/ revert: 1. See what walk_revs_populate_todo does: It takes all the operands and fills up "pick" as the operator. Why would any other caller want to do this? 2. You mentioned multiple entry points earlier, and that's something I've been meaning to do: In the long run, I don't want callers to fill up an opts structure to specify the subcommand! That'd be butt-ugly. -- 8< -- static int pick_revisions(struct replay_opts *opts) { struct replay_insn_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 */ if (opts->subcommand == REPLAY_RESET) { remove_sequencer_state(1); return 0; } else if (opts->subcommand == REPLAY_CONTINUE) { if (!file_exists(git_path(SEQ_TODO_FILE))) goto error; sequencer_read_opts(&opts); sequencer_read_todo(&todo_list); /* Verify that the conflict has been resolved */ if (!index_differs_from("HEAD", 0)) todo_list = todo_list->next; } else { /* * Start a new cherry-pick/ revert sequence; but * first, make sure that an existing one isn't in * progress */ walk_revs_populate_todo(&todo_list, opts); if (sequencer_create_dir() < 0) { error(_("A cherry-pick or revert is in progress.")); advise(_("Use --continue to continue the operation")); advise(_("or --reset to forget about it")); return -1; } if (get_sha1("HEAD", sha1)) { if (opts->action == REPLAY_REVERT) return error(_("Can't revert as initial commit")); return error(_("Can't cherry-pick into empty head")); } sequencer_save_head(sha1_to_hex(sha1)); sequencer_save_opts(opts); } return pick_commits(todo_list, opts); error: return error(_("No %s in progress"), action_name(opts)); } -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html