Re: [PATCH 7/7] sequencer: Remove sequencer state after final commit

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 18, 2011, 20:05 UTC
Message-ID: <20110818200520.GI30436@elie.gateway.2wire.net>
In-Reply-To: <CALkWK0=jRAq6s1zQ5gwB4feBgC1eo=VYLWx8bsjs+exqmz0f1A@mail.gmail.com>

Ramkumar Ramachandra wrote:

> In the future, we might want a 'merge' instruction in the sequencer --
> I want to make it clear that we're going for a significant UI change
> so that everyone (including tests, scripts) become comfortable with
> the new UI.

I don't follow. I meant "Why modify tests, rather than adding new ones?" Tests document what is supposed to continue working.

[...]

Show 6 quoted lines

> Okay, here's my problem with the idea: it'll essentially require the
> sequencer to differentiate between one-commit operations and
> many-commit operations.  In the case of one-commit operations, *every*
> new command that calls into the sequencer will will need to persist
> information in its own way using hacks like CHERRY_PICK_HEAD and
> MERGE_HEAD.

Why wouldn't such a backward-compatibility feature apply to cherry-pick/revert only?

> I'm not talking about some hypothetical case: I'm already planning
> to make 'git am' call into the sequencer, so we'll need an AM_HEAD.

Yuck.  How does the UI distinguish between

	git sequencer --continue; # apply the rest of the patches from mbox

and

	git sequencer --continue; # finish up "am" insn and continue

? Does the sequencer need an argument to indicate the level of nesting? I would find it more straightforward for "git am mbox" to create a todo list saying something like

	apply patch 1 from mbox
	apply patch 2 from mbox
	apply patch 3 from mbox
	apply patch 4 from mbox

so there would still be only one pending sequence of basic instructions to think about.

Show 5 quoted lines

> One final resort: Move some code back into cherry-pick, and call into
> a later-function in the sequencer only if it's a many-commit
> operation.  The new commands can enjoy the comfort of calling into an
> earlier-function in the sequencer that'll do all the revision walk
> setup and call the later-function.  I think this is reasonable.

I'm having trouble parsing this; sorry. What is the effect on the user-visible behavior of the program of what you propose?

> Jonathan Nieder writes:

Show 8 quoted lines

>> One part I'm handwaving is what to do about commands like "git
>> cherry-pick foo^..foo" which use a commit range that only happens to
>> contain one commit.  Either behavior seems fine for such commands.
>
> I don't think I follow.  This will be determined as a single-commit
> operation after setting up the revisions.  I don't think it should be
> treated as a multi-commit operation because the literal tree'ish
> contains "..".

I said "Either behavior seems fine", didn't I?

Hope that clarifies a little. Jonathan