Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 11, 2012, 09:52 UTC
Message-ID: <20120111095236.GB31670@burratino>
In-Reply-To: <CALkWK0=o+KkV08G9JuTaG8Vtb-AzHQVMQPzMy8td_iBVnGX4Dg@mail.gmail.com>

Ramkumar Ramachandra wrote:

Show 6 quoted lines

>                                                              how does
> the sequencer know what to do with this hypothetical command string
> (say "cherry-pick") on a fresh invocation?  It needs to translate this
> into a replay_action at some point, right?  There are atleast three
> places where this happens: prepare_revs(), walk_revs_populate_todo(),
> and single_pick().

I see.

Perhaps cherry-pick and revert should be different values for replay_subcommand, to avoid conflating the mechanics and the command name? Resulting in something like this:

	enum replay_subcommand {
		REPLAY_PICK_REVISIONS,
		REPLAY_REVERT_REVISIONS,
		REPLAY_EDIT_SEQUENCE,
		REPLAY_REMOVE_STATE,
		REPLAY_CONTINUE,
		REPLAY_SKIP,
		REPLAY_ROLLBACK
	};

Though this dispatcher on an enum to perform many different actions already felt a bit awkward, so an alternative could be

	extern int pick_revisions(struct replay_opts *opts);
	extern int revert_revisions(struct replay_opts *opts);

	extern int launch_sequence_editor(struct replay_opts *opts);
	extern void remove_sequencer_state(void);
	extern int sequencer_continue(struct replay_opts *opts);
	extern int sequencer_skip(struct replay_opts *opts);
	extern int sequencer_rollback(struct replay_opts *opts);

which would make it easier to add arguments specific to any one of the routines as appropriate.