Skip to content

Commit 649a8b7

Browse files
committed
Improve editing a commit
In 67b8ef4 we changed the "edit" command to insert a "break" after the selected commit, rather than setting the selected todo to "edit". The reason for doing this was that it now works for merge commits too. Back then, I claimed "In most cases the behavior is exactly the same as before." Unfortunately that's not true, there are two reasons why the previous behavior was better (both are demonstrated by tests earlier in this branch): - when editing the last commit of a branch in the middle of a stack of branches, we are now missing the update-ref todo after it, which means that amending the commit breaks the stack - it breaks auto-amending (see the added test earlier in this branch for an explanation) For these reasons, we are going back to the previous approach of setting the selected commit to "edit" whenever possible, i.e. unless it's a merge commit. The only scenario where this could still be a problem is when you have a stack of branches, and the last commit of one of the branches in the stack is a merge commit, and you try to edit that. In my experience with stacked branches this is very unlikely, in almost all cases my stacked branches are linear.
1 parent 345c533 commit 649a8b7

File tree

3 files changed

+16
-10
lines changed

3 files changed

+16
-10
lines changed

pkg/gui/controllers/local_commits_controller.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
116116
},
117117
{
118118
Key: opts.GetKey(editCommitKey),
119-
Handler: self.withItems(self.edit),
119+
Handler: self.withItemsRange(self.edit),
120120
GetDisabledReason: self.require(
121121
self.itemRangeSelected(self.midRebaseCommandEnabled),
122122
),
@@ -511,11 +511,25 @@ func (self *LocalCommitsController) drop(selectedCommits []*models.Commit, start
511511
return nil
512512
}
513513

514-
func (self *LocalCommitsController) edit(selectedCommits []*models.Commit) error {
514+
func (self *LocalCommitsController) edit(selectedCommits []*models.Commit, startIdx int, endIdx int) error {
515515
if self.isRebasing() {
516516
return self.updateTodos(todo.Edit, selectedCommits)
517517
}
518518

519+
commits := self.c.Model().Commits
520+
if !commits[endIdx].IsMerge() {
521+
selectionRangeAndMode := self.getSelectionRangeAndMode()
522+
err := self.c.Git().Rebase.InteractiveRebase(commits, startIdx, endIdx, todo.Edit)
523+
return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions(
524+
err,
525+
types.RefreshOptions{
526+
Mode: types.BLOCK_UI, Then: func() error {
527+
self.restoreSelectionRangeAndMode(selectionRangeAndMode)
528+
return nil
529+
},
530+
})
531+
}
532+
519533
return self.startInteractiveRebaseWithEdit(selectedCommits)
520534
}
521535

pkg/integration/tests/interactive_rebase/edit_and_auto_amend.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@ var EditAndAutoAmend = NewIntegrationTest(NewIntegrationTestArgs{
5050
)
5151

5252
t.Views().Main().
53-
/* EXPECTED:
5453
Content(Contains("fixup content"))
55-
ACTUAL: */
56-
Content(DoesNotContain("fixup content"))
5754
},
5855
})

pkg/integration/tests/interactive_rebase/edit_last_commit_of_stacked_branch.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ var EditLastCommitOfStackedBranch = NewIntegrationTest(NewIntegrationTestArgs{
3939
Lines(
4040
Contains("pick").Contains("CI commit 05"),
4141
Contains("pick").Contains("CI commit 04"),
42-
/* EXPECTED:
4342
Contains("update-ref").Contains("branch1"),
44-
*/
4543
Contains("<-- YOU ARE HERE --- * commit 03").IsSelected(),
4644
Contains("CI commit 02"),
4745
Contains("CI commit 01"),
@@ -68,10 +66,7 @@ var EditLastCommitOfStackedBranch = NewIntegrationTest(NewIntegrationTestArgs{
6866
Lines(
6967
Contains("CI commit 05"),
7068
Contains("CI commit 04"),
71-
/* EXPECTED:
7269
Contains("CI * commit 03"),
73-
ACTUAL: */
74-
Contains("CI commit 03"),
7570
Contains("CI commit 02"),
7671
Contains("CI commit 01"),
7772
)

0 commit comments

Comments
 (0)