Added Merge keybind in tag submenu, closes #1315 #3383
Conversation
|
🙏 thanks for following this up! I never got around to the final polish+PR step, and have since forgotten about it a few times 😆 |
RetoranPetra
force-pushed
the
feature/merge-tag
branch
from
2da4824
to
6405912
Compare
| Key: opts.GetKey(opts.Config.Branches.MergeIntoCurrentBranch), | ||
| Handler: opts.Guards.OutsideFilterMode(self.withItem(self.merge)), | ||
| Description: self.c.Tr.Merge, | ||
| Tooltip: self.c.Tr.MergeBranchTooltip, |
There was a problem hiding this comment.
We should add a new tooltip called MergeTagTooltip because the current tooltip only mentions branches
There was a problem hiding this comment.
Got it, I'll work on this when I get some free time along with the other comment.
There was a problem hiding this comment.
Added this in 91a620c, let me know if I should squash these down.
| @@ -236,6 +243,10 @@ func (self *TagsController) create() error { | |||
| }) | |||
| } | |||
|
|
|||
| func (self *TagsController) merge(tag *models.Tag) error { | |||
| return self.c.Helpers().MergeAndRebase.MergeRefIntoCheckedOutBranch(tag.RefName()) | |||
There was a problem hiding this comment.
We should focus the local branches view after merging so that the user has more context on what's going on. Let me know if you need help with that
There was a problem hiding this comment.
Added this in f6a2423, let me know if it's preferred to squash down these commits.
RetoranPetra
force-pushed
the
feature/merge-tag
branch
2 times, most recently
from
be3ee84
to
f6a2423
Compare
|
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. |
|
Your guess is exactly right. One way to solve this is to pass a callback to |
RetoranPetra
force-pushed
the
feature/merge-tag
branch
2 times, most recently
from
b8a113e
to
04fb772
Compare
credit to @adamDilger for original commit, tidied up to match master along with adding integration tests and a view switch on success. closes jesseduffield#1315
RetoranPetra
force-pushed
the
feature/merge-tag
branch
from
04fb772
to
77c791e
Compare
|
Finally got a chance to look at this again, added a simple integration test and got the view change after merging working correctly in 77c791e |
PR Description
Noticed @adamDilger had made a commit to fix Merging a tag into the checked-out branch #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
go generate ./...)docs/Config.md) have been updated if necessary