Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Spin out/off branch #676

Merged
merged 22 commits into from
Aug 3, 2023

Conversation

morgsmccauley
Copy link
Contributor

This PR adds the Spin out/off branch commands.

Spin off will; create a new branch based on the current branch, reset the current branch to its remote if it exists, and checkout the new branch. Spin out is almost the same, except that it does not checkout the new branch.

I haven't included the FROM functionality mentioned in the Magit docs, which allows resetting to a source different from the current remote. Let me know if this should be included.

Resolves #665

@morgsmccauley morgsmccauley marked this pull request as ready for review July 29, 2023 02:19
Copy link
Member

@CKolkey CKolkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start 😍

lua/neogit/popups/branch/actions.lua Outdated Show resolved Hide resolved
lua/neogit/popups/branch/actions.lua Outdated Show resolved Hide resolved
lua/neogit/popups/branch/actions.lua Outdated Show resolved Hide resolved
lua/neogit/popups/branch/actions.lua Outdated Show resolved Hide resolved
@CKolkey
Copy link
Member

CKolkey commented Jul 29, 2023

Hope you don't mind - I saw some API's that would be useful to you here, so I implemented them: #677

I think that'll make your life a little easier ;)

@morgsmccauley
Copy link
Contributor Author

morgsmccauley commented Jul 30, 2023

Thanks for the help @CKolkey @treatybreaker!

I've made a few more updates, I've left a couple comments unresolved to ensure my reasoning is correct. I believe the implementation is more inline with what Magit has now.

I'll change back to draft for now as there's still the following to do:

  • Choose which branch to reset to, i.e. FROM
  • Manual testing - haven't done so for the latest changes
  • Add/update automated tests

Please comment if I'm not on the right track! 🙏🏼

@morgsmccauley morgsmccauley marked this pull request as draft July 30, 2023 02:37
lua/neogit/status.lua Outdated Show resolved Hide resolved
@CKolkey
Copy link
Member

CKolkey commented Jul 30, 2023

Thanks for the help @CKolkey @treatybreaker!

I've made a few more updates, I've left a couple comments unresolved to ensure my reasoning is correct. I believe the implementation is more inline with what Magit has now.

I'll change back to draft for now as there's still the following to do:

  • Choose which branch to reset to, i.e. FROM
  • Manual testing - haven't done so for the latest changes
  • Add/update automated tests

Please comment if I'm not on the right track! 🙏🏼

It's looking great! Reading emacs lisp is... not very straightforward. Something I've found is useful is to ask an LLM to translate the emacs-lisp to lua - it won't be valid, but it does a good job to sketch out the logic/control flow.

For example, here's magit--branch-spinoff in lua:

local function magit_branch_spinoff(branch, from, checkout)
    if magit_branch_p(branch) then
        error(string.format("Cannot spin off %s. It already exists", branch))
    end

    if not checkout and magit_anything_modified_p() then
        print("Staying on HEAD due to uncommitted changes")
        checkout = true
    end

    local current = magit_get_current_branch()
    if current then
        local tracked = magit_get_upstream_branch(current)
        local base

        if from then
            if not magit_rev_ancestor_p(from, current) then
                error(string.format("Cannot spin off %s. %s is not reachable from %s", branch, from, current))
            end
            if tracked and magit_rev_ancestor_p(from, tracked) then
                error(string.format("Cannot spin off %s. %s is ancestor of upstream %s", branch, from, tracked))
            end
        end

        if checkout then
            magit_call_git("checkout", "-b", branch, current)
        else
            magit_call_git("branch", branch, current)
        end

        local indirect_upstream_branch = magit_get_indirect_upstream_branch(current)
        if indirect_upstream_branch then
            magit_call_git("branch", "--set-upstream-to", indirect_upstream_branch, branch)
        end

        if tracked then
            base = from and from .. "^" or magit_git_string("merge-base", current, tracked)
            if not magit_rev_eq(base, current) then
                if checkout then
                    magit_call_git("update-ref", "-m", string.format("reset: moving to %s", base), "refs/heads/" .. current, base)
                else
                    magit_call_git("reset", "--hard", base)
                end
            end
        end
    else
        if checkout then
            magit_call_git("checkout", "-b", branch)
        else
            magit_call_git("branch", branch)
        end
    end

    magit_refresh()
end

It obviously doesn't work as-is, but it's easier (for me, at least) to read :)

In terms of what you have left, I think you can skip the FROM argument for now and just presume the FROM is your current branch. Some of the magit functions can be used in a library-like way (not from the UI), but thats not an express goal of ours, as this isn't public api.

All said, its looking really great :D

Copy link
Member

@ten3roberts ten3roberts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your work. I am looking forward to being able to forget creating a feature branch before committing to main.

Left some small comments regarding the implementation

lua/neogit/popups/branch/actions.lua Show resolved Hide resolved
lua/neogit/popups/branch/actions.lua Show resolved Hide resolved
lua/neogit/popups/branch/actions.lua Outdated Show resolved Hide resolved
lua/neogit/popups/branch/actions.lua Outdated Show resolved Hide resolved
@morgsmccauley
Copy link
Contributor Author

Some of the magit functions can be used in a library-like way (not from the UI), but thats not an express goal of ours, as this isn't public api.

Ah, that makes more sense now.

Given that we can ignore FROM I believe it's just the tests remaining. I should hopefully have those finished in the next couple days :).

lua/neogit/popups/branch/actions.lua Outdated Show resolved Hide resolved
lua/neogit/popups/branch/actions.lua Outdated Show resolved Hide resolved
@morgsmccauley morgsmccauley marked this pull request as ready for review August 3, 2023 09:06
@CKolkey
Copy link
Member

CKolkey commented Aug 3, 2023

I think #693 should fix the config issue with the spec. Rebase/merge master to grab it. Besides, that, no comments, I think this looks great :D

Copy link
Member

@ten3roberts ten3roberts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. The merged solution looks fantastic

@CKolkey
Copy link
Member

CKolkey commented Aug 3, 2023

Fantastic work :D Thanks for your contribution

@CKolkey CKolkey merged commit f06c256 into NeogitOrg:master Aug 3, 2023
3 checks passed
@morgsmccauley morgsmccauley deleted the feat/spin-off-branch branch August 3, 2023 20:05
@morgsmccauley
Copy link
Contributor Author

Thank you for all the help @CKolkey @ten3roberts @treatybreaker!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spin off branch
4 participants