Interactive rebase: using "pick" for merge commits

> When I do an interactive rebase, and manually enter a "pick" with the
> commit hash of a merge commit, I get the following confusing error message:
> 
> error: commit fa1afe1 is a merge but no -m option was given.
> hint: Could not execute the todo command
> hint:
> hint:     pick fa1afe1 some subject
> hint:
> hint: It has been rescheduled; [rest of message snipped]
> 
> This error message makes it sound like I could somehow add "-m1" after
> the "pick" to make it work (which is actually what I would like to be
> able to do). I had to go read the source code to find out that that's
> not the case, and the error message only comes from the fact that the
> code is shared with the cherry-pick and revert commands, which do have
> the -m option.

> On 09/02/2024 15:52, Stefan Haller wrote:
>> When I do an interactive rebase, and manually enter a "pick" with the
>> commit hash of a merge commit, I get the following confusing error
>> message:
>>
>> error: commit fa1afe1 is a merge but no -m option was given.
>> 
>> Is it crazy to want pick to work like this? Should it be supported?
> 
> It causes problems trying to maintain the topology. In the past there
> was a "--preserve-merges" option that allowed one to "pick" merges but
> it broke if the user edited the todo list. The "--rebase-merges" option
> was introduced with the "label", "reset" and "merge" todo list
> instructions to allow the user to control the topology.

> On 09.02.24 17:24, Phillip Wood wrote:
> > On 09/02/2024 15:52, Stefan Haller wrote:
> >> When I do an interactive rebase, and manually enter a "pick" with the
> >> commit hash of a merge commit, I get the following confusing error
> >> message:
> >>
> >> error: commit fa1afe1 is a merge but no -m option was given.
> >> 
> >> Is it crazy to want pick to work like this? Should it be supported?
> > 
> > It causes problems trying to maintain the topology. In the past there
> > was a "--preserve-merges" option that allowed one to "pick" merges but
> > it broke if the user edited the todo list. The "--rebase-merges" option
> > was introduced with the "label", "reset" and "merge" todo list
> > instructions to allow the user to control the topology.
> 
> Yes, I'm familiar with all this, but that's not what I mean. I don't
> want to maintain the topology here, and I'm also not suggesting that git
> itself generates such "pick" entries with -mX arguments (maybe I wasn't
> clear on that). What I want to do is to add such entries myself, as a
> user, resulting in the equivalent of doing a "break" at that point in
> the rebase and doing a "git cherry-pick -mX <hash-of-merge-commit>"
> manually.

> On Sat, Feb 10, 2024 at 10:23:16AM +0100, Stefan Haller wrote:
>> On 09.02.24 17:24, Phillip Wood wrote:
>> Yes, I'm familiar with all this, but that's not what I mean. I don't
>> want to maintain the topology here, and I'm also not suggesting that git
>> itself generates such "pick" entries with -mX arguments (maybe I wasn't
>> clear on that). What I want to do is to add such entries myself, as a
>> user, resulting in the equivalent of doing a "break" at that point in
>> the rebase and doing a "git cherry-pick -mX <hash-of-merge-commit>"
>> manually.
> 
> It would be neat indeed if this could be specified in the instruction
> sheet. We already support options for the "merge" instruction, so
> extending "pick" to support options isn't that far-fetched. Then it
> would become possible to say "pick -m1 fa1afe1".

> Hi Patrick and Stefan
> 
> On 12/02/2024 07:15, Patrick Steinhardt wrote:
>> On Sat, Feb 10, 2024 at 10:23:16AM +0100, Stefan Haller wrote:
>>> On 09.02.24 17:24, Phillip Wood wrote:
>>> Yes, I'm familiar with all this, but that's not what I mean. I don't
>>> want to maintain the topology here, and I'm also not suggesting that git
>>> itself generates such "pick" entries with -mX arguments (maybe I wasn't
>>> clear on that). What I want to do is to add such entries myself, as a
>>> user, resulting in the equivalent of doing a "break" at that point in
>>> the rebase and doing a "git cherry-pick -mX <hash-of-merge-commit>"
>>> manually.
>>
>> It would be neat indeed if this could be specified in the instruction
>> sheet. We already support options for the "merge" instruction, so
>> extending "pick" to support options isn't that far-fetched. Then it
>> would become possible to say "pick -m1 fa1afe1".
> 
> It would certainly be possible to extend the sequencer to do that but
> I'm not familiar with why people use "git cherry-pick -m" [1] so I'm
> wondering what this would be used for. It would involve a bit of extra
> complexity so I think we'd want a compelling reason as to why
> cherry-picking merges without maintaining the topology is useful
> especially as one can currently do that via "exec git cherry-pick -m ..."

>> It would certainly be possible to extend the sequencer to do that but
>> I'm not familiar with why people use "git cherry-pick -m" [1] so I'm
>> wondering what this would be used for. It would involve a bit of extra
>> complexity so I think we'd want a compelling reason as to why
>> cherry-picking merges without maintaining the topology is useful
>> especially as one can currently do that via "exec git cherry-pick -m ..."
> 
> Ok, I suppose the answer will probably not count as a compelling reason.
> My reason for wanting this is that lazygit currently implements
> cherry-picking in terms of an interactive rebase, rather then calling
> git-cherry-pick. And the reason why it does this is that when you
> cherry-pick multiple commits, and one of them conflicts, then you get
> lazygit's nice visualization of the rebase todo list to show you where
> in the sequence you are, what the conflicting commit is, how many are
> left etc. It just happens to support this well for
> .git/rebase-merge/git-rebase-todo, but not for .git/sequencer/todo.

>> It probably makes more sense to teach lazygit to visualize the
>> .git/sequencer/todo file, and then use git cherry-pick.
> 
> If lazygit is generating the todo list for the cherry-pick could it
> check if the commit is a merge and insert "exec cherry-pick -m ..." for
> those commits?

> On 26.02.24 11:56, Phillip Wood wrote:
>>> It probably makes more sense to teach lazygit to visualize the
>>> .git/sequencer/todo file, and then use git cherry-pick.
>>
>> If lazygit is generating the todo list for the cherry-pick could it
>> check if the commit is a merge and insert "exec cherry-pick -m ..." for
>> those commits?
> 
> That's a good idea, but it wouldn't buy us very much. We'd still have to
> add support for conflicts during a cherry-pick; when there's a conflict
> during a rebase, lazygit has this nice visualization of the conflicting
> commit (we talked about that in [1], and it turned out to be working
> extremely well), so it would have to learn to do the same thing for a
> conflicting cherry-pick (although this does seem to be a lot easier).
> And then it would have to learn to call "cherry-pick --continue" rather
> than "rebase --continue" after resolving. But if we do all these things,
> then we're not so far away from being able to just call git cherry-pick
> ourselves.

> On Sat, Feb 10, 2024 at 10:23:16AM +0100, Stefan Haller wrote:
>> On 09.02.24 17:24, Phillip Wood wrote:
>> > On 09/02/2024 15:52, Stefan Haller wrote:
>> >> When I do an interactive rebase, and manually enter a "pick" with the
>> >> commit hash of a merge commit, I get the following confusing error
>> >> message:
>> >>
>> >> error: commit fa1afe1 is a merge but no -m option was given.
>> >> 
>> >> Is it crazy to want pick to work like this? Should it be supported?
>> > 
>> > It causes problems trying to maintain the topology. In the past there
>> > was a "--preserve-merges" option that allowed one to "pick" merges but
>> > it broke if the user edited the todo list. The "--rebase-merges" option
>> > was introduced with the "label", "reset" and "merge" todo list
>> > instructions to allow the user to control the topology.
>> 
>> Yes, I'm familiar with all this, but that's not what I mean. I don't
>> want to maintain the topology here, and I'm also not suggesting that git
>> itself generates such "pick" entries with -mX arguments (maybe I wasn't
>> clear on that). What I want to do is to add such entries myself, as a
>> user, resulting in the equivalent of doing a "break" at that point in
>> the rebase and doing a "git cherry-pick -mX <hash-of-merge-commit>"
>> manually.
>
> It would be neat indeed if this could be specified in the instruction
> sheet. We already support options for the "merge" instruction, so
> extending "pick" to support options isn't that far-fetched. Then it
> would become possible to say "pick -m1 fa1afe1".