From 014fa6329915cf9a7521b024cc896c3e46c6582b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 27 Apr 2025 21:40:17 +0200 Subject: [PATCH 1/4] Add FixupFlag field to Commit This is true for Fixup todos that have their -C flag set. --- pkg/commands/git_commands/commit_loader.go | 10 ++++++---- pkg/commands/models/commit.go | 3 +++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 6b1dfacdf..7383b4173 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -320,6 +320,7 @@ func (self *CommitLoader) getHydratedTodoCommits(hashPool *utils.StringPool, tod hydratedCommits = append(hydratedCommits, rebasingCommit) } else if commit := findFullCommit(rebasingCommit.Hash()); commit != nil { commit.Action = rebasingCommit.Action + commit.FixupFlag = rebasingCommit.FixupFlag commit.Status = rebasingCommit.Status hydratedCommits = append(hydratedCommits, commit) } @@ -366,10 +367,11 @@ func (self *CommitLoader) getRebasingCommits(hashPool *utils.StringPool, addConf continue } commits = utils.Prepend(commits, models.NewCommit(hashPool, models.NewCommitOpts{ - Hash: t.Commit, - Name: t.Msg, - Status: models.StatusRebasing, - Action: t.Command, + Hash: t.Commit, + Name: t.Msg, + Status: models.StatusRebasing, + Action: t.Command, + FixupFlag: t.Command == todo.Fixup && t.Flag == "-C", })) } diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index eca66b3ac..0cc2cf86f 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -56,6 +56,7 @@ type Commit struct { Status CommitStatus Action todo.TodoCommand + FixupFlag bool // Only used for todo.Fixup action: true if the `-C` flag is set Divergence Divergence // set to DivergenceNone unless we are showing the divergence view } @@ -64,6 +65,7 @@ type NewCommitOpts struct { Name string Status CommitStatus Action todo.TodoCommand + FixupFlag bool Tags []string ExtraInfo string AuthorName string @@ -79,6 +81,7 @@ func NewCommit(hashPool *utils.StringPool, opts NewCommitOpts) *Commit { Name: opts.Name, Status: opts.Status, Action: opts.Action, + FixupFlag: opts.FixupFlag, Tags: opts.Tags, ExtraInfo: opts.ExtraInfo, AuthorName: opts.AuthorName, From 70e3df1fa728c3c85978aff27623644425091a49 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 27 Apr 2025 21:41:33 +0200 Subject: [PATCH 2/4] WIP Visualize the fixup flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We show fixup↓ for the normal case where the commit message of the bottom commit is retained, and fixup← for the special case where the message of the upper commit is used (e.g. for 'amend!' commits). --- pkg/gui/presentation/commits.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index 63c2b3c9b..f2ad6524d 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -387,7 +387,15 @@ func displayCommit( actionString := "" if commit.Action != models.ActionNone { - actionString = actionColorMap(commit.Action, commit.Status).Sprint(commit.Action.String()) + actionName := commit.Action.String() + if commit.Action == todo.Fixup { + if commit.FixupFlag { + actionName += "←" + } else { + actionName += "↓" + } + } + actionString = actionColorMap(commit.Action, commit.Status).Sprint(actionName) } tagString := "" From 24cb221aecf78aa871181e3c14ead654a9177e5d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 29 Apr 2025 15:53:47 +0200 Subject: [PATCH 3/4] Pass todo flag to EditRebaseTodo Not used yet, we pass an empty string everywhere, to match the previous behavior. Just extracting this into a separate commit to make the next one smaller. --- pkg/app/daemon/daemon.go | 1 + pkg/app/daemon/rebase.go | 1 + pkg/commands/git_commands/rebase.go | 6 ++-- .../controllers/local_commits_controller.go | 30 +++++++++---------- pkg/utils/rebase_todo.go | 2 ++ 5 files changed, 23 insertions(+), 17 deletions(-) diff --git a/pkg/app/daemon/daemon.go b/pkg/app/daemon/daemon.go index 16b9bf5e5..4210af2c5 100644 --- a/pkg/app/daemon/daemon.go +++ b/pkg/app/daemon/daemon.go @@ -201,6 +201,7 @@ func (self *ChangeTodoActionsInstruction) run(common *common.Common) error { return utils.TodoChange{ Hash: c.Hash, NewAction: c.NewAction, + NewFlag: c.NewFlag, } }) diff --git a/pkg/app/daemon/rebase.go b/pkg/app/daemon/rebase.go index 30740d09c..71c734e37 100644 --- a/pkg/app/daemon/rebase.go +++ b/pkg/app/daemon/rebase.go @@ -37,6 +37,7 @@ func TodoLinesToString(todoLines []TodoLine) string { type ChangeTodoAction struct { Hash string NewAction todo.TodoCommand + NewFlag string } func handleInteractiveRebase(common *common.Common, f func(path string) error) error { diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 94891fa10..1904285a2 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -137,7 +137,7 @@ func (self *RebaseCommands) MoveCommitsUp(commits []*models.Commit, startIdx int }).Run() } -func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, startIdx int, endIdx int, action todo.TodoCommand) error { +func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, startIdx int, endIdx int, action todo.TodoCommand, flag string) error { baseIndex := endIdx + 1 if action == todo.Squash || action == todo.Fixup { baseIndex++ @@ -149,6 +149,7 @@ func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, startIdx return daemon.ChangeTodoAction{ Hash: commit.Hash(), NewAction: action, + NewFlag: flag, }, !commit.IsMerge() }) @@ -331,11 +332,12 @@ func todoFromCommit(commit *models.Commit) utils.Todo { } // Sets the action for the given commits in the git-rebase-todo file -func (self *RebaseCommands) EditRebaseTodo(commits []*models.Commit, action todo.TodoCommand) error { +func (self *RebaseCommands) EditRebaseTodo(commits []*models.Commit, action todo.TodoCommand, flag string) error { commitsWithAction := lo.Map(commits, func(commit *models.Commit, _ int) utils.TodoChange { return utils.TodoChange{ Hash: commit.Hash(), NewAction: action, + NewFlag: flag, } }) diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index ad844f90e..f7c186543 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -322,7 +322,7 @@ func secondaryPatchPanelUpdateOpts(c *ControllerCommon) *types.ViewUpdateOpts { func (self *LocalCommitsController) squashDown(selectedCommits []*models.Commit, startIdx int, endIdx int) error { if self.isRebasing() { - return self.updateTodos(todo.Squash, selectedCommits) + return self.updateTodos(todo.Squash, "", selectedCommits) } self.c.Confirm(types.ConfirmOpts{ @@ -331,7 +331,7 @@ func (self *LocalCommitsController) squashDown(selectedCommits []*models.Commit, HandleConfirm: func() error { return self.c.WithWaitingStatus(self.c.Tr.SquashingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.SquashCommitDown) - return self.interactiveRebase(todo.Squash, startIdx, endIdx) + return self.interactiveRebase(todo.Squash, "", startIdx, endIdx) }) }, }) @@ -341,7 +341,7 @@ func (self *LocalCommitsController) squashDown(selectedCommits []*models.Commit, func (self *LocalCommitsController) fixup(selectedCommits []*models.Commit, startIdx int, endIdx int) error { if self.isRebasing() { - return self.updateTodos(todo.Fixup, selectedCommits) + return self.updateTodos(todo.Fixup, "", selectedCommits) } self.c.Confirm(types.ConfirmOpts{ @@ -350,7 +350,7 @@ func (self *LocalCommitsController) fixup(selectedCommits []*models.Commit, star HandleConfirm: func() error { return self.c.WithWaitingStatus(self.c.Tr.FixingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.FixupCommit) - return self.interactiveRebase(todo.Fixup, startIdx, endIdx) + return self.interactiveRebase(todo.Fixup, "", startIdx, endIdx) }) }, }) @@ -485,14 +485,14 @@ func (self *LocalCommitsController) drop(selectedCommits []*models.Commit, start self.context().SetSelectionRangeAndMode(selectedIdx, rangeStartIdx, rangeSelectMode) - return self.updateTodos(todo.Drop, nonUpdateRefTodos) + return self.updateTodos(todo.Drop, "", nonUpdateRefTodos) }, }) return nil } - return self.updateTodos(todo.Drop, selectedCommits) + return self.updateTodos(todo.Drop, "", selectedCommits) } isMerge := selectedCommits[0].IsMerge() @@ -506,7 +506,7 @@ func (self *LocalCommitsController) drop(selectedCommits []*models.Commit, start if isMerge { return self.dropMergeCommit(startIdx) } - return self.interactiveRebase(todo.Drop, startIdx, endIdx) + return self.interactiveRebase(todo.Drop, "", startIdx, endIdx) }) }, }) @@ -521,13 +521,13 @@ func (self *LocalCommitsController) dropMergeCommit(commitIdx int) error { func (self *LocalCommitsController) edit(selectedCommits []*models.Commit, startIdx int, endIdx int) error { if self.isRebasing() { - return self.updateTodos(todo.Edit, selectedCommits) + return self.updateTodos(todo.Edit, "", selectedCommits) } commits := self.c.Model().Commits if !commits[endIdx].IsMerge() { selectionRangeAndMode := self.getSelectionRangeAndMode() - err := self.c.Git().Rebase.InteractiveRebase(commits, startIdx, endIdx, todo.Edit) + err := self.c.Git().Rebase.InteractiveRebase(commits, startIdx, endIdx, todo.Edit, "") return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( err, types.RefreshOptions{ @@ -568,7 +568,7 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit( } } if len(todos) > 0 { - err := self.updateTodos(todo.Edit, todos) + err := self.updateTodos(todo.Edit, "", todos) if err != nil { return err } @@ -628,7 +628,7 @@ func (self *LocalCommitsController) findCommitForQuickStartInteractiveRebase() ( func (self *LocalCommitsController) pick(selectedCommits []*models.Commit) error { if self.isRebasing() { - return self.updateTodos(todo.Pick, selectedCommits) + return self.updateTodos(todo.Pick, "", selectedCommits) } // at this point we aren't actually rebasing so we will interpret this as an @@ -636,14 +636,14 @@ func (self *LocalCommitsController) pick(selectedCommits []*models.Commit) error return self.pullFiles() } -func (self *LocalCommitsController) interactiveRebase(action todo.TodoCommand, startIdx int, endIdx int) error { +func (self *LocalCommitsController) interactiveRebase(action todo.TodoCommand, flag string, startIdx int, endIdx int) error { // When performing an action that will remove the selected commits, we need to select the // next commit down (which will end up at the start index after the action is performed) if action == todo.Drop || action == todo.Fixup || action == todo.Squash { self.context().SetSelection(startIdx) } - err := self.c.Git().Rebase.InteractiveRebase(self.c.Model().Commits, startIdx, endIdx, action) + err := self.c.Git().Rebase.InteractiveRebase(self.c.Model().Commits, startIdx, endIdx, action, flag) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) } @@ -651,8 +651,8 @@ func (self *LocalCommitsController) interactiveRebase(action todo.TodoCommand, s // updateTodos sees if the selected commit is in fact a rebasing // commit meaning you are trying to edit the todo file rather than actually // begin a rebase. It then updates the todo file with that action -func (self *LocalCommitsController) updateTodos(action todo.TodoCommand, selectedCommits []*models.Commit) error { - if err := self.c.Git().Rebase.EditRebaseTodo(selectedCommits, action); err != nil { +func (self *LocalCommitsController) updateTodos(action todo.TodoCommand, flag string, selectedCommits []*models.Commit) error { + if err := self.c.Git().Rebase.EditRebaseTodo(selectedCommits, action, flag); err != nil { return err } diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go index e2c9dc442..df3f66293 100644 --- a/pkg/utils/rebase_todo.go +++ b/pkg/utils/rebase_todo.go @@ -19,6 +19,7 @@ type Todo struct { type TodoChange struct { Hash string NewAction todo.TodoCommand + NewFlag string } // Read a git-rebase-todo file, change the actions for the given commits, @@ -37,6 +38,7 @@ func EditRebaseTodo(filePath string, changes []TodoChange, commentChar byte) err if equalHash(t.Commit, change.Hash) { matchCount++ t.Command = change.NewAction + t.Flag = change.NewFlag } } } From 8139eb9c86c27b3839e68e270344240d8eb19cb3 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 30 Apr 2025 18:36:10 +0200 Subject: [PATCH 4/4] WIP Menu --- .../controllers/local_commits_controller.go | 35 +++++++++++++------ pkg/i18n/english.go | 2 -- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index f7c186543..8153fcb45 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -340,22 +340,35 @@ func (self *LocalCommitsController) squashDown(selectedCommits []*models.Commit, } func (self *LocalCommitsController) fixup(selectedCommits []*models.Commit, startIdx int, endIdx int) error { - if self.isRebasing() { - return self.updateTodos(todo.Fixup, "", selectedCommits) + f := func(flag string) error { + if self.isRebasing() { + return self.updateTodos(todo.Fixup, flag, selectedCommits) + } + + return self.c.WithWaitingStatus(self.c.Tr.FixingStatus, func(gocui.Task) error { + self.c.LogAction(self.c.Tr.Actions.FixupCommit) + return self.interactiveRebase(todo.Fixup, flag, startIdx, endIdx) + }) } - self.c.Confirm(types.ConfirmOpts{ + return self.c.Menu(types.CreateMenuOptions{ Title: self.c.Tr.Fixup, - Prompt: self.c.Tr.SureFixupThisCommit, - HandleConfirm: func() error { - return self.c.WithWaitingStatus(self.c.Tr.FixingStatus, func(gocui.Task) error { - self.c.LogAction(self.c.Tr.Actions.FixupCommit) - return self.interactiveRebase(todo.Fixup, "", startIdx, endIdx) - }) + Prompt: "This squashes the selected commit(s) into the commit below it. You can decide which commit message to keep:", + Items: []*types.MenuItem{ + { + Label: "Keep the message of the commit below", + OnPress: func() error { + return f("") + }, + }, + { + Label: "Keep the message of the first selected commit", + OnPress: func() error { + return f("-C") + }, + }, }, }) - - return nil } func (self *LocalCommitsController) reword(commit *models.Commit) error { diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index a85f7202f..09239ae50 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -172,7 +172,6 @@ type TranslationSet struct { CannotSquashOrFixupMergeCommit string Fixup string FixupTooltip string - SureFixupThisCommit string SureSquashThisCommit string Squash string SquashMerge string @@ -1252,7 +1251,6 @@ func EnglishTranslationSet() *TranslationSet { CannotSquashOrFixupFirstCommit: "There's no commit below to squash into", CannotSquashOrFixupMergeCommit: "Cannot squash or fixup a merge commit", Fixup: "Fixup", - SureFixupThisCommit: "Are you sure you want to 'fixup' the selected commit(s) into the commit below?", SureSquashThisCommit: "Are you sure you want to squash the selected commit(s) into the commit below?", Squash: "Squash", SquashMerge: "Squash Merge",