{"thread":{"id":"60884","subject":"Interactive rebase: using \"pick\" for merge commits","startedAt":"2024-02-09T16:02:09Z","lastAt":"2024-02-27T10:42:04Z","messageCount":10,"participants":["Stefan Haller","Phillip Wood","Patrick Steinhardt","Junio C Hamano","phillip.wood123@gmail.com"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"488295","messageId":"424f2e08-a2ad-4bb2-8a6b-136c426dc127@haller-berlin.de","threadId":"60884","inReplyTo":null,"subject":"Interactive rebase: using \"pick\" for merge commits","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2024-02-09T15:52:42Z","receivedAt":"2024-02-09T16:02:09Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"When I do an interactive rebase, and manually enter a \"pick\" with the\ncommit hash of a merge commit, I get the following confusing error message:\n\nerror: commit fa1afe1 is a merge but no -m option was given.\nhint: Could not execute the todo command\nhint:\nhint: pick fa1afe1 some subject\nhint:\nhint: It has been rescheduled; [rest of message snipped]\n\nThis error message makes it sound like I could somehow add \"-m1\" after\nthe \"pick\" to make it work (which is actually what I would like to be\nable to do). I had to go read the source code to find out that that's\nnot the case, and the error message only comes from the fact that the\ncode is shared with the cherry-pick and revert commands, which do have\nthe -m option.\n\nIs it crazy to want pick to work like this? Should it be supported?\n\n-Stefan\n"},{"id":"488298","messageId":"ad561600-faf6-4d3c-80b2-34b3d1a1b99e@gmail.com","threadId":"60884","inReplyTo":"424f2e08-a2ad-4bb2-8a6b-136c426dc127@haller-berlin.de","subject":"Re: Interactive rebase: using \"pick\" for merge commits","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-09T16:24:10Z","receivedAt":"2024-02-09T16:24:12Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Stefan\n\nOn 09/02/2024 15:52, Stefan Haller wrote:\n> When I do an interactive rebase, and manually enter a \"pick\" with the\n> commit hash of a merge commit, I get the following confusing error message:\n> \n> error: commit fa1afe1 is a merge but no -m option was given.\n> hint: Could not execute the todo command\n> hint:\n> hint: pick fa1afe1 some subject\n> hint:\n> hint: It has been rescheduled; [rest of message snipped]\n> \n> This error message makes it sound like I could somehow add \"-m1\" after\n> the \"pick\" to make it work (which is actually what I would like to be\n> able to do). I had to go read the source code to find out that that's\n> not the case, and the error message only comes from the fact that the\n> code is shared with the cherry-pick and revert commands, which do have\n> the -m option.\n\nOh, that's unfortunate - we should really reject the todo list when we \nparse it at the start of the rebase if it is going to try and \"pick\" a \nmerge.\n\n> Is it crazy to want pick to work like this? Should it be supported?\n\nIt causes problems trying to maintain the topology. In the past there \nwas a \"--preserve-merges\" option that allowed one to \"pick\" merges but \nit broke if the user edited the todo list. The \"--rebase-merges\" option \nwas introduced with the \"label\", \"reset\" and \"merge\" todo list \ninstructions to allow the user to control the topology.\n\nBest Wishes\n\nPhillip\n"},{"id":"488345","messageId":"65c65f6b-5ec8-4fa0-a17c-0f2c0d32b390@haller-berlin.de","threadId":"60884","inReplyTo":"ad561600-faf6-4d3c-80b2-34b3d1a1b99e@gmail.com","subject":"Re: Interactive rebase: using \"pick\" for merge commits","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2024-02-10T09:23:16Z","receivedAt":"2024-02-10T09:23:19Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"On 09.02.24 17:24, Phillip Wood wrote:\n> On 09/02/2024 15:52, Stefan Haller wrote:\n>> When I do an interactive rebase, and manually enter a \"pick\" with the\n>> commit hash of a merge commit, I get the following confusing error\n>> message:\n>>\n>> error: commit fa1afe1 is a merge but no -m option was given.\n>> \n>> Is it crazy to want pick to work like this? Should it be supported?\n> \n> It causes problems trying to maintain the topology. In the past there\n> was a \"--preserve-merges\" option that allowed one to \"pick\" merges but\n> it broke if the user edited the todo list. The \"--rebase-merges\" option\n> was introduced with the \"label\", \"reset\" and \"merge\" todo list\n> instructions to allow the user to control the topology.\n\nYes, I'm familiar with all this, but that's not what I mean. I don't\nwant to maintain the topology here, and I'm also not suggesting that git\nitself generates such \"pick\" entries with -mX arguments (maybe I wasn't\nclear on that). What I want to do is to add such entries myself, as a\nuser, resulting in the equivalent of doing a \"break\" at that point in\nthe rebase and doing a \"git cherry-pick -mX \"\nmanually.\n\n-Stefan\n"},{"id":"488429","messageId":"ZcnFl8kypKRYeLo3@tanuki","threadId":"60884","inReplyTo":"65c65f6b-5ec8-4fa0-a17c-0f2c0d32b390@haller-berlin.de","subject":"Re: Interactive rebase: using \"pick\" for merge commits","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-02-12T07:15:35Z","receivedAt":"2024-02-12T07:15:41Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Feb 10, 2024 at 10:23:16AM +0100, Stefan Haller wrote:\n> On 09.02.24 17:24, Phillip Wood wrote:\n> > On 09/02/2024 15:52, Stefan Haller wrote:\n> >> When I do an interactive rebase, and manually enter a \"pick\" with the\n> >> commit hash of a merge commit, I get the following confusing error\n> >> message:\n> >>\n> >> error: commit fa1afe1 is a merge but no -m option was given.\n> >> \n> >> Is it crazy to want pick to work like this? Should it be supported?\n> > \n> > It causes problems trying to maintain the topology. In the past there\n> > was a \"--preserve-merges\" option that allowed one to \"pick\" merges but\n> > it broke if the user edited the todo list. The \"--rebase-merges\" option\n> > was introduced with the \"label\", \"reset\" and \"merge\" todo list\n> > instructions to allow the user to control the topology.\n> \n> Yes, I'm familiar with all this, but that's not what I mean. I don't\n> want to maintain the topology here, and I'm also not suggesting that git\n> itself generates such \"pick\" entries with -mX arguments (maybe I wasn't\n> clear on that). What I want to do is to add such entries myself, as a\n> user, resulting in the equivalent of doing a \"break\" at that point in\n> the rebase and doing a \"git cherry-pick -mX \"\n> manually.\n\nIt would be neat indeed if this could be specified in the instruction\nsheet. We already support options for the \"merge\" instruction, so\nextending \"pick\" to support options isn't that far-fetched. Then it\nwould become possible to say \"pick -m1 fa1afe1\".\n\nPatrick\n"},{"id":"488444","messageId":"040f142c-7ee2-429e-88eb-d328b01a4b8c@gmail.com","threadId":"60884","inReplyTo":"ZcnFl8kypKRYeLo3@tanuki","subject":"Re: Interactive rebase: using \"pick\" for merge commits","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-12T14:38:08Z","receivedAt":"2024-02-12T14:38:14Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Patrick and Stefan\n\nOn 12/02/2024 07:15, Patrick Steinhardt wrote:\n> On Sat, Feb 10, 2024 at 10:23:16AM +0100, Stefan Haller wrote:\n>> On 09.02.24 17:24, Phillip Wood wrote:\n>> Yes, I'm familiar with all this, but that's not what I mean. I don't\n>> want to maintain the topology here, and I'm also not suggesting that git\n>> itself generates such \"pick\" entries with -mX arguments (maybe I wasn't\n>> clear on that). What I want to do is to add such entries myself, as a\n>> user, resulting in the equivalent of doing a \"break\" at that point in\n>> the rebase and doing a \"git cherry-pick -mX \"\n>> manually.\n> \n> It would be neat indeed if this could be specified in the instruction\n> sheet. We already support options for the \"merge\" instruction, so\n> extending \"pick\" to support options isn't that far-fetched. Then it\n> would become possible to say \"pick -m1 fa1afe1\".\n\nIt would certainly be possible to extend the sequencer to do that but \nI'm not familiar with why people use \"git cherry-pick -m\" [1] so I'm \nwondering what this would be used for. It would involve a bit of extra \ncomplexity so I think we'd want a compelling reason as to why \ncherry-picking merges without maintaining the topology is useful \nespecially as one can currently do that via \"exec git cherry-pick -m ...\"\n\nBest Wishes\n\nPhillip\n\n[1] I did a quick web search and the results all seemed to focus on how \nto do it rather than why you'd want to.\n"},{"id":"488448","messageId":"xmqqcyt18wn3.fsf@gitster.g","threadId":"60884","inReplyTo":"ZcnFl8kypKRYeLo3@tanuki","subject":"Re: Interactive rebase: using \"pick\" for merge commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-12T16:39:28Z","receivedAt":"2024-02-12T16:39:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Sat, Feb 10, 2024 at 10:23:16AM +0100, Stefan Haller wrote:\n>> On 09.02.24 17:24, Phillip Wood wrote:\n>> > On 09/02/2024 15:52, Stefan Haller wrote:\n>> >> When I do an interactive rebase, and manually enter a \"pick\" with the\n>> >> commit hash of a merge commit, I get the following confusing error\n>> >> message:\n>> >>\n>> >> error: commit fa1afe1 is a merge but no -m option was given.\n>> >> \n>> >> Is it crazy to want pick to work like this? Should it be supported?\n>> > \n>> > It causes problems trying to maintain the topology. In the past there\n>> > was a \"--preserve-merges\" option that allowed one to \"pick\" merges but\n>> > it broke if the user edited the todo list. The \"--rebase-merges\" option\n>> > was introduced with the \"label\", \"reset\" and \"merge\" todo list\n>> > instructions to allow the user to control the topology.\n>> \n>> Yes, I'm familiar with all this, but that's not what I mean. I don't\n>> want to maintain the topology here, and I'm also not suggesting that git\n>> itself generates such \"pick\" entries with -mX arguments (maybe I wasn't\n>> clear on that). What I want to do is to add such entries myself, as a\n>> user, resulting in the equivalent of doing a \"break\" at that point in\n>> the rebase and doing a \"git cherry-pick -mX \"\n>> manually.\n>\n> It would be neat indeed if this could be specified in the instruction\n> sheet. We already support options for the \"merge\" instruction, so\n> extending \"pick\" to support options isn't that far-fetched. Then it\n> would become possible to say \"pick -m1 fa1afe1\".\n\nWould adding \"x git cherry-pick -m 1 $that_one\" work there?\n"},{"id":"489258","messageId":"2739325d-93b1-445c-aac9-3e0ec54a27e4@haller-berlin.de","threadId":"60884","inReplyTo":"040f142c-7ee2-429e-88eb-d328b01a4b8c@gmail.com","subject":"Re: Interactive rebase: using \"pick\" for merge commits","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2024-02-23T20:59:28Z","receivedAt":"2024-02-23T21:09:20Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"On 12.02.24 15:38, Phillip Wood wrote:\n> Hi Patrick and Stefan\n> \n> On 12/02/2024 07:15, Patrick Steinhardt wrote:\n>> On Sat, Feb 10, 2024 at 10:23:16AM +0100, Stefan Haller wrote:\n>>> On 09.02.24 17:24, Phillip Wood wrote:\n>>> Yes, I'm familiar with all this, but that's not what I mean. I don't\n>>> want to maintain the topology here, and I'm also not suggesting that git\n>>> itself generates such \"pick\" entries with -mX arguments (maybe I wasn't\n>>> clear on that). What I want to do is to add such entries myself, as a\n>>> user, resulting in the equivalent of doing a \"break\" at that point in\n>>> the rebase and doing a \"git cherry-pick -mX \"\n>>> manually.\n>>\n>> It would be neat indeed if this could be specified in the instruction\n>> sheet. We already support options for the \"merge\" instruction, so\n>> extending \"pick\" to support options isn't that far-fetched. Then it\n>> would become possible to say \"pick -m1 fa1afe1\".\n> \n> It would certainly be possible to extend the sequencer to do that but\n> I'm not familiar with why people use \"git cherry-pick -m\" [1] so I'm\n> wondering what this would be used for. It would involve a bit of extra\n> complexity so I think we'd want a compelling reason as to why\n> cherry-picking merges without maintaining the topology is useful\n> especially as one can currently do that via \"exec git cherry-pick -m ...\"\n\nOk, I suppose the answer will probably not count as a compelling reason.\nMy reason for wanting this is that lazygit currently implements\ncherry-picking in terms of an interactive rebase, rather then calling\ngit-cherry-pick. And the reason why it does this is that when you\ncherry-pick multiple commits, and one of them conflicts, then you get\nlazygit's nice visualization of the rebase todo list to show you where\nin the sequence you are, what the conflicting commit is, how many are\nleft etc. It just happens to support this well for\n.git/rebase-merge/git-rebase-todo, but not for .git/sequencer/todo.\n\nIt probably makes more sense to teach lazygit to visualize the\n.git/sequencer/todo file, and then use git cherry-pick.\n\nSee https://github.com/jesseduffield/lazygit/issues/3317\n\n-Stefan\n"},{"id":"489368","messageId":"b4781808-f722-4be5-906f-4c3409c3295c@gmail.com","threadId":"60884","inReplyTo":"2739325d-93b1-445c-aac9-3e0ec54a27e4@haller-berlin.de","subject":"Re: Interactive rebase: using \"pick\" for merge commits","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-26T10:56:51Z","receivedAt":"2024-02-26T10:56:56Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Stefan\n\nOn 23/02/2024 20:59, Stefan Haller wrote:\n> On 12.02.24 15:38, Phillip Wood wrote:\n>> Hi Patrick and Stefan\n >>\n>> It would certainly be possible to extend the sequencer to do that but\n>> I'm not familiar with why people use \"git cherry-pick -m\" [1] so I'm\n>> wondering what this would be used for. It would involve a bit of extra\n>> complexity so I think we'd want a compelling reason as to why\n>> cherry-picking merges without maintaining the topology is useful\n>> especially as one can currently do that via \"exec git cherry-pick -m ...\"\n> \n> Ok, I suppose the answer will probably not count as a compelling reason.\n> My reason for wanting this is that lazygit currently implements\n> cherry-picking in terms of an interactive rebase, rather then calling\n> git-cherry-pick. And the reason why it does this is that when you\n> cherry-pick multiple commits, and one of them conflicts, then you get\n> lazygit's nice visualization of the rebase todo list to show you where\n> in the sequence you are, what the conflicting commit is, how many are\n> left etc. It just happens to support this well for\n> .git/rebase-merge/git-rebase-todo, but not for .git/sequencer/todo.\n\nThanks for the context. I can see how that is convenient for lazygit \n(and makes we think that perhaps we should teach \"git status\" to show \npending cherry-picks) but I'm afraid I don't think that is a good reason \nfor adding the ability to pick merges to git rebase.\n\n> It probably makes more sense to teach lazygit to visualize the\n> .git/sequencer/todo file, and then use git cherry-pick.\n\nIf lazygit is generating the todo list for the cherry-pick could it \ncheck if the commit is a merge and insert \"exec cherry-pick -m ...\" for \nthose commits? The UI could detect that and display something more user \nfriendly for those lines in the todo list. It is still more work for \nlazygit but perhaps less than supporting cherry-picks directly.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"489411","messageId":"6a557891-ffcd-4c42-9768-ec2da0fce92a@haller-berlin.de","threadId":"60884","inReplyTo":"b4781808-f722-4be5-906f-4c3409c3295c@gmail.com","subject":"Re: Interactive rebase: using \"pick\" for merge commits","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2024-02-26T19:07:44Z","receivedAt":"2024-02-26T19:07:53Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"On 26.02.24 11:56, Phillip Wood wrote:\n>> It probably makes more sense to teach lazygit to visualize the\n>> .git/sequencer/todo file, and then use git cherry-pick.\n> \n> If lazygit is generating the todo list for the cherry-pick could it\n> check if the commit is a merge and insert \"exec cherry-pick -m ...\" for\n> those commits?\n\nThat's a good idea, but it wouldn't buy us very much. We'd still have to\nadd support for conflicts during a cherry-pick; when there's a conflict\nduring a rebase, lazygit has this nice visualization of the conflicting\ncommit (we talked about that in [1], and it turned out to be working\nextremely well), so it would have to learn to do the same thing for a\nconflicting cherry-pick (although this does seem to be a lot easier).\nAnd then it would have to learn to call \"cherry-pick --continue\" rather\nthan \"rebase --continue\" after resolving. But if we do all these things,\nthen we're not so far away from being able to just call git cherry-pick\nourselves.\n\n-Stefan\n\n[1] \n"},{"id":"489483","messageId":"2f749aae-697b-4d35-a6ed-7d2a2faa596a@gmail.com","threadId":"60884","inReplyTo":"6a557891-ffcd-4c42-9768-ec2da0fce92a@haller-berlin.de","subject":"Re: Interactive rebase: using \"pick\" for merge commits","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-02-27T10:41:54Z","receivedAt":"2024-02-27T10:42:04Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 26/02/2024 19:07, Stefan Haller wrote:\n> On 26.02.24 11:56, Phillip Wood wrote:\n>>> It probably makes more sense to teach lazygit to visualize the\n>>> .git/sequencer/todo file, and then use git cherry-pick.\n>>\n>> If lazygit is generating the todo list for the cherry-pick could it\n>> check if the commit is a merge and insert \"exec cherry-pick -m ...\" for\n>> those commits?\n> \n> That's a good idea, but it wouldn't buy us very much. We'd still have to\n> add support for conflicts during a cherry-pick; when there's a conflict\n> during a rebase, lazygit has this nice visualization of the conflicting\n> commit (we talked about that in [1], and it turned out to be working\n> extremely well), so it would have to learn to do the same thing for a\n> conflicting cherry-pick (although this does seem to be a lot easier).\n> And then it would have to learn to call \"cherry-pick --continue\" rather\n> than \"rebase --continue\" after resolving. But if we do all these things,\n> then we're not so far away from being able to just call git cherry-pick\n> ourselves.\n\nOh I'd forgotten about handling conflicts - that does make my proposal \nless attractive.\n\nBest Wishes\n\nPhillip\n\n> -Stefan\n> \n> [1] 961e68d7-5f43-c385-10fa-455b8e2f32d0@haller-berlin.de/>\n"}]}