lazygit icon indicating copy to clipboard operation
lazygit copied to clipboard

Added Merge keybind in tag submenu, closes #1315

Open RetoranPetra opened this issue 1 year ago • 7 comments

  • PR Description Noticed @adamDilger had made a commit to fix #1315 on his own repository, but never made a pull request. Made some changes to be up to be up to date with master, and made this pull request. First proper pull request so please let me know if there's any issues and I'll do my best to resolve them.

  • Please check if the PR fulfills these requirements

  • [x] Cheatsheets are up-to-date (run go generate ./...)
  • [x] Code has been formatted (see here)
  • [x] Tests have been added/updated (see here for the integration test guide)
  • [x] Text is internationalised (see here)
  • [x] Docs (specifically docs/Config.md) have been updated if necessary
  • [x] You've read through your own file changes for silly mistakes etc

RetoranPetra avatar Mar 13 '24 13:03 RetoranPetra

🙏 thanks for following this up! I never got around to the final polish+PR step, and have since forgotten about it a few times 😆

adamDilger avatar Mar 13 '24 22:03 adamDilger

Good thing you asked for integration tests, found out that the way I changed the view caused the tag merge to stop working, and hadn't verified it before this.

It seems to be that MergeRefIntoCheckedOutBranch() gives a return value before the user hits enter on the popup confirmation window, so when I change the view to local branches it gets rid of the pop up and the merge never occurs.

That'd be my first guess, I can do some digging around with a debugger to figure out what exactly is happening but I'm not too familiar with go or its tools so it may take a while.

RetoranPetra avatar Apr 17 '24 19:04 RetoranPetra

Your guess is exactly right. One way to solve this is to pass a callback to MergeRefIntoCheckedOutBranch that is called at the end of the OnConfirm. Let me know if you need more help.

stefanhaller avatar Apr 20 '24 11:04 stefanhaller