Re: [PATCH 4/5] sequencer: Expose code that handles files in .git/sequencer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]