Add new command "Move commits to new branch" #3876

stefanhaller · 2024-08-30T13:31:13Z

PR Description

Add a new command "Move commits to new branch" (bound to N by default), which is useful if you have just started some new work, you already made some commits, and then you realize that you forgot to create a new branch first, accidentally making those commits on main or whichever other feature branch you happened to be on.

If you made those commits on main, you are prompted for a branch name for the new branch, and then it creates the new branch right there and hard-resets main to where it was. If you made them on another feature branch though, you are first given the choice whether you want to create the new branch from main, or keep it stacked on that other feature branch. Then it prompts you for the name and proceeds as above.

Inspired by Magit's magit-branch-spinoff command.

The conditions under which the command is available are rather restrictive: the current branch must have an upstream, it must not be behind its upstream, but it must be ahead of it (otherwise there wouldn't be any commits to move, and you might as well just create a new branch normally).

Please check if the PR fulfills these requirements

Cheatsheets are up-to-date (run go generate ./...)
Code has been formatted (see here)
Tests have been added/updated (see here for the integration test guide)
Text is internationalised (see here)
If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
Docs have been updated if necessary
You've read through your own file changes for silly mistakes etc

stefanhaller · 2024-08-30T13:34:06Z

pkg/gui/controllers/branches_controller.go

+			GetDisabledReason: self.canMoveCommitsToNewBranch,
+			Description:       self.c.Tr.MoveCommitsToNewBranch,
+			Tooltip:           self.c.Tr.MoveCommitsToNewBranchTooltip,
+		},


I wasn't totally sure in which views to make the command available. We could consider making it global, because it doesn't depend on the selection at all. However, I found it important that the command shows up next to New Branch in the keybindings panel, so I decided to make it available in local branches and in BasicCommitsController (see code comment there).

codacy-production · 2024-08-30T13:34:49Z

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for `05ae080`¹	✅ 85.71%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (`05ae080`)	Report Missing	Report Missing	Report Missing
Head commit (`67a61e4`)	49902	42730	85.63%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#3876)	175	150	85.71%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

jesseduffield

Code LGTM. One thing that would be good is if we could change the prompt or add a description in a secondary panel so that we're being clear about what we're about to do: right now there's no way to know whether you've pressed 'n' or 'N' after pressing the key, because the two prompts are identical.

stefanhaller · 2024-08-31T06:41:25Z

Code LGTM. One thing that would be good is if we could change the prompt or add a description in a secondary panel so that we're being clear about what we're about to do: right now there's no way to know whether you've pressed 'n' or 'N' after pressing the key, because the two prompts are identical.

I thought about this, or rather about an extra confirmation panel that comes before ("are you sure you want to..."). After all, a hard reset is involved, and this feels like a confirmation is in order. But then I realized that there's really no potential for losing data with the current approach, so I left it out. I can look into changing the prompt text though.

However, while testing it more I noticed that this only really works in the expected way when you start on master. If you start on an unrelated feature branch, it will create the new branch as a stacked branch on top of that one, which might sometimes be what you want if you are into stacked branches, but most of the time it's probably not. You want to create the new branch off of master most of the time.

If we want to achieve this, it has a few consequences:

we can provide the command only in the branches panel, and the selection becomes important. The new branch will be created off of the selected branch. (Probably a good thing because that makes it more similar to n.)
the simple approach of creating the new branch and hard resetting the other one is not enough. We do actually need to cherry-pick the commits to bring them over (except in the stacked branches example, so we need to check if the new branch contains the commits already).
this means we can now get conflicts, both when cherry-picking and when popping the auto-stash. Neither could happen with the current approach.

Very keen to hear your thoughts on this @jesseduffield

jesseduffield · 2024-08-31T07:41:28Z

we can provide the command only in the branches panel, and the selection becomes important. The new branch will be created off of the selected branch. (Probably a good thing because that makes it more similar to n.)

I actually don't think that's a good thing: the purpose is to move the commits from the checked out branch to a new branch. The selection shouldn't matter.

However, while testing it more I noticed that this only really works in the expected way when you start on master. If you start on an unrelated feature branch, it will create the new branch as a stacked branch on top of that one, which might sometimes be what you want if you are into stacked branches, but most of the time it's probably not. You want to create the new branch off of master most of the time.

I'm fine if we just only allow this command from the main branch

stefanhaller · 2024-08-31T08:14:24Z

we can provide the command only in the branches panel, and the selection becomes important. The new branch will be created off of the selected branch. (Probably a good thing because that makes it more similar to n.)

I actually don't think that's a good thing: the purpose is to move the commits from the checked out branch to a new branch. The selection shouldn't matter.

Hm, ok. That means we wouldn't provide the option of creating a new branch stacked on the current one. Maybe that's fine as it should be rather rare, but I do remember that I was in this situation once or twice.

What we need to do then is to create the new branch off of the current branch's base branch.

I'm fine if we just only allow this command from the main branch

This restriction would make it almost useless for me. I almost never have main checked out, so I don't accidentally make commits there; I'm much more likely to run into the problem while I'm on an unrelated feature branch.

jesseduffield · 2024-09-07T01:47:53Z

I see. For whatever reason I seem to only get into this state from the master branch.

If we need to handle the case where this happens on a feature branch, then it seems to me that we'd need to specify two things: the commits we want to move, and the branch we want to use as the new base (defaulting to master). So I'm thinking we could make it that the keybinding is only present from the commits panel (after you've selected the commits to move) and the branch is specified via an input prompt. Or as you say, we could just programmatically work out the base branch; I'm fine with that too.

Then we would:

copy the commit hashes
checkout a new branch off of the specified base
cherry-pick the commits
hard reset the original branch to remove the commits (which we can do without checking it out, see here)

If there are conflicts with the cherry-pick, we could just skip the fourth step and leave that as an exercise for the user.

What do you think of that?

stefanhaller · 2024-09-07T10:53:21Z

Hm, I'm not sure. What you propose would only work if the selected range is at the end of the branch. Otherwise we'd have to drop the commits from the branch, which is not a simple reset, so there's potential for conflicts both at the source and the destination.

I'm not sure we need this much flexibility though; I think I'd be fine with restricting it to moving the unpushed commits. If you have the need to move an arbitrary commit range (whether at the end of the branch or in the middle), you still have to do that manually, like today.

So I'm still in favor of offering the command only in the branches panel; you select the target branch, and it works on the unpushed commits of the current branch. If you're on a branch that doesn't have an upstream (yet), the command is disabled.

My feeling is that this will cover most real-world cases, but of course that's hard to tell without using it for a while and seeing how much it helps.

jesseduffield · 2024-09-14T23:37:16Z

Okay that works for me.

jesseduffield · 2024-09-28T06:28:32Z

@stefanhaller are you waiting on my re-review for this PR or were there more changes to push?

stefanhaller · 2024-09-28T06:31:23Z

No, I was planning to implement the behavior that I proposed above, but didn't get around to it yet. Setting to draft in the meantime to make that clearer.

jesseduffield · 2024-09-28T06:43:49Z

No worries

ar1a · 2025-03-02T08:19:01Z

@stefanhaller hey! i'm really interested in this feature, it's one of the main things i miss from magit.

is there any chance you can take another look? i'd even be happy to pay a bounty for this

no stress if not, i can always reinstall emacs just for this command :p

stefanhaller · 2025-03-02T09:25:53Z

@ar1a I down-prioritized this a bit because I don't need it very often myself. I can't promise when I find the time to take this up again.

I'd be curious what you think about the discussion above. I'm having a hard time working out what magit's behavior is just from the description of its magit-branch-spinoff command (not a magit user myself).

stefanhaller · 2025-03-02T09:31:17Z

Oh, and I'm surprised that you'd rather switch to magit just for that command, instead of using the next best workaround inside of lazygit, which is to select the commits you made on the wrong branch, type shift-C to copy them, reset the current branch (using g h), create the new branch off of wherever you want it to start, and pressing shift-V to paste your commits there. I find this reasonably easy, to the point that I'd rather use this manual, explicit way of doing it than to use a command which I'm not totally sure will behave exactly the way I want it to.

ar1a · 2025-03-02T11:58:50Z

I didn't know about that workflow! It turns out I was still on lazygit ~0.40, which didn't have branch selection iirc. It definitely saves me the hassle of re setting up magit.

Oh, and I'm surprised that you'd rather switch to magit just for that command

to be fair, magit is really good.

if you haven't found it yet, here's magit's source code for this: https://github.com/magit/magit/blob/28d272ce0bcecc2e312d22ed15a48ad4cea564eb/lisp/magit-branch.el#L459-L531

I'm not sure we need this much flexibility though; I think I'd be fine with restricting it to moving the unpushed commits. If you have the need to move an arbitrary commit range (whether at the end of the branch or in the middle), you still have to do that manually, like today.

I agree with this, the behaviour I would expect from magit is a sort of "dumb" solution: Just base it off of the current branch you're on, don't go all the way back to main. If the user wants to do a multi-branch spinoff they will probably need to do it manually via the copying commits strategy as described in previous comments.

I think this feature is definitely still worth implementing. Having an easily pressed key when you fuck up and start working on the wrong branch is much easier to deal with than remembering the key sequence here #3876 (comment)

(in fact, i think the next few times I need this I would have to come back to this thread to remind myself)

stefanhaller · 2025-03-02T14:44:43Z

if you haven't found it yet, here's magit's source code for this: https://github.com/magit/magit/blob/28d272ce0bcecc2e312d22ed15a48ad4cea564eb/lisp/magit-branch.el#L459-L531

Thanks, but I speak very little lisp, so this is of limited usefulness to me...

the behaviour I would expect from magit is a sort of "dumb" solution: Just base it off of the current branch you're on, don't go all the way back to main

Doesn't that mean that you will always get a stack of branches when you are starting on another feature branch? This may occasionally be what I want, but most of the time it's not, so I'd then have to manually rebase the branch onto main, away from that other feature branch. While that's easy using lazygit's shift-B feature, it's still cumbersome.

I'm wondering if we should prompt the user whether they want to stay on the other feature branch, or move off of it onto main.

stefanhaller · 2025-03-02T21:06:45Z

I'm wondering if we should prompt the user whether they want to stay on the other feature branch, or move off of it onto main.

Ok, re-reading the discussion above I realize that I earlier suggested to select the target branch in the branches panel; that would make this prompt unnecessary. However, I actually no longer think this suggestion makes sense, for a number of reasons:

If I want to select master, I first need to make sure it's up to date. For me, since I never use master locally it is usually behind its upstream, so I first have to fast-forward it. While that's easy (f) it can take a bit of time, and it's annoying to have to do that. The prompt suggested above doesn't have this problem. (Incidentally, the same problem exists for just creating a new branch off of master; I sometimes wish we had a command for creating a branch off of origin/master.)
Selecting any other branch besides master or the one that you are currently on doesn't seem to make any sense to me, so this flexibility is unnecessary.

So I would now suggest to offer the command in branches view or commits view, as in the original version, and the selection doesn't matter; then, if the checked out branch is master, just do the simple thing, and if it is some other branch, show the prompt whether we should stay there and create a stack (in which case we do the simple thing here as well), or if we should move to master, in which case we need to do the slightly more complicated cherry-pick approach.

Hope this makes sense; I'll see if I can find some time to prototype this.

ar1a · 2025-03-03T04:28:32Z

Thanks, but I speak very little lisp, so this is of limited usefulness to me...

understandable 😅, it definitely seems indecipherable at a glance. especially since emacs lisp has its own functions for a lot of things, you sort of just need to be using emacs to understand it

So I would now suggest to offer the command in branches view or commits view, as in the original version, and the selection doesn't matter; then, if the checked out branch is master, just do the simple thing, and if it is some other branch, show the prompt whether we should stay there and create a stack (in which case we do the simple thing here as well), or if we should move to master, in which case we need to do the slightly more complicated cherry-pick approach.

i like this! i think this makes a lot of sense and really fits with lazygits design

codacy-production · 2025-04-02T18:15:59Z

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for `5f80980`¹	✅ 90.75%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (`5f80980`)	Report Missing	Report Missing	Report Missing
Head commit (`cae6319`)	54774	47518	86.75%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#3876)	346	314	90.75%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings Change summary preferences

Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

stefanhaller · 2025-04-02T18:20:26Z

I pushed a new version that implements the proposal from my last comment. Would be good to get some thorough testing on this!

When debugging an integration test that involves some behind-the-scenes rebasing, the daemon lazygit would also wait for a debugger to attach. This is very confusing because the test seems to hang; once you figured out what's going on, it's inconvenient because you need to attach a debugger to the daemon every time you debug the test. Now, it would sometimes be useful to be able to debug the daemon itself (whether inside an integration test, or during normal usage), and I have often wished to be able to do that. We might introduce an additional env var (and command-line option?) to enable this; but that's out of scope here.

…ergeAndRebaseHelper It's the same, really, except that GetCheckedOutRef() does a check if any branches exist and returns nil if not. Since we are accessing the returned branch unconditionally without checking for nil, it seems this check is not needed here. (The functions we are touching here are called from handlers that are guarded with itemSelected or singleItemSelected, so we know that at least one branch exists.) The goal is to get rid of the dependency to refsHelper.

We want to make MergeAndRebaseHelper a dependency of RefsHelper instead.

The long story: I want to call this function from RefsHelper; however, I can't make WorkingTreeHelper a field of RefsHelper because RefsHelper is already a field in WorkingTreeHelper, so that would be a circular dependency. The shorter story: there's really little reason to have to instantiate a helper object in order to call a simple function like this. Long term I would like to get to a state where a lot more of these helper functions are free-standing, and you pass in the data they need. While at it, simplify the implementation of AnyStagedFiles and AnyTrackedFiles to one-liners.

stefanhaller added the enhancement New feature or request label Aug 30, 2024

stefanhaller force-pushed the move-commits-to-new-branch branch from aaef833 to 67a61e4 Compare August 30, 2024 13:32

stefanhaller commented Aug 30, 2024

View reviewed changes

jesseduffield reviewed Aug 31, 2024

View reviewed changes

stefanhaller mentioned this pull request Sep 6, 2024

Release 0.44 #3893

Closed

stefanhaller marked this pull request as draft September 28, 2024 06:31

stefanhaller force-pushed the move-commits-to-new-branch branch from 67a61e4 to 41d5b48 Compare April 2, 2025 18:13

stefanhaller marked this pull request as ready for review April 2, 2025 18:20

stefanhaller force-pushed the move-commits-to-new-branch branch 2 times, most recently from 191844c to 3ca4b83 Compare April 10, 2025 09:55

stefanhaller added 6 commits April 16, 2025 17:45

Cleanup: remove unnecessary indirection

fb9859e

Remove MergeAndRebaseHelper's dependency on RefsHelper

8506acf

We want to make MergeAndRebaseHelper a dependency of RefsHelper instead.

Add new command "Move commits to new branch"

aeaec0b

Update config docs and schema

cae6319

stefanhaller force-pushed the move-commits-to-new-branch branch from 3ca4b83 to cae6319 Compare April 16, 2025 15:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new command "Move commits to new branch" #3876

Add new command "Move commits to new branch" #3876

stefanhaller commented Aug 30, 2024 •

edited

Loading

stefanhaller Aug 30, 2024

codacy-production bot commented Aug 30, 2024 •

edited

Loading

jesseduffield left a comment

stefanhaller commented Aug 31, 2024

jesseduffield commented Aug 31, 2024

stefanhaller commented Aug 31, 2024

jesseduffield commented Sep 7, 2024

stefanhaller commented Sep 7, 2024

jesseduffield commented Sep 14, 2024

jesseduffield commented Sep 28, 2024

stefanhaller commented Sep 28, 2024

jesseduffield commented Sep 28, 2024

ar1a commented Mar 2, 2025 •

edited

Loading

stefanhaller commented Mar 2, 2025

stefanhaller commented Mar 2, 2025

ar1a commented Mar 2, 2025

stefanhaller commented Mar 2, 2025

stefanhaller commented Mar 2, 2025

ar1a commented Mar 3, 2025

codacy-production bot commented Apr 2, 2025 •

edited

Loading

stefanhaller commented Apr 2, 2025

Add new command "Move commits to new branch" #3876

Are you sure you want to change the base?

Add new command "Move commits to new branch" #3876

Conversation

stefanhaller commented Aug 30, 2024 • edited Loading

stefanhaller Aug 30, 2024

Choose a reason for hiding this comment

codacy-production bot commented Aug 30, 2024 • edited Loading

Coverage summary from Codacy

See diff coverage on Codacy

See your quality gate settings Change summary preferences

Footnotes

jesseduffield left a comment

Choose a reason for hiding this comment

stefanhaller commented Aug 31, 2024

jesseduffield commented Aug 31, 2024

stefanhaller commented Aug 31, 2024

jesseduffield commented Sep 7, 2024

stefanhaller commented Sep 7, 2024

jesseduffield commented Sep 14, 2024

jesseduffield commented Sep 28, 2024

stefanhaller commented Sep 28, 2024

jesseduffield commented Sep 28, 2024

ar1a commented Mar 2, 2025 • edited Loading

stefanhaller commented Mar 2, 2025

stefanhaller commented Mar 2, 2025

ar1a commented Mar 2, 2025

stefanhaller commented Mar 2, 2025

stefanhaller commented Mar 2, 2025

ar1a commented Mar 3, 2025

codacy-production bot commented Apr 2, 2025 • edited Loading

Coverage summary from Codacy

See diff coverage on Codacy

See your quality gate settings Change summary preferences

Footnotes

stefanhaller commented Apr 2, 2025

stefanhaller commented Aug 30, 2024 •

edited

Loading

codacy-production bot commented Aug 30, 2024 •

edited

Loading

ar1a commented Mar 2, 2025 •

edited

Loading

codacy-production bot commented Apr 2, 2025 •

edited

Loading