From 01f27f5089fea561b767db8e16e567b7c728e1d5 Mon Sep 17 00:00:00 2001 From: Alexander Slavschik Date: Wed, 8 Apr 2026 22:03:32 +0200 Subject: [PATCH] refactor(commit): adjust skip hook prefix logic for amend and fixup commits --- pkg/commands/git_commands/commit.go | 6 +- pkg/commands/git_commands/commit_test.go | 55 ++++++++++--------- pkg/commands/git_commands/rebase.go | 2 +- .../controllers/local_commits_controller.go | 2 +- 4 files changed, 35 insertions(+), 30 deletions(-) diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index 1c575f669..801a1ecbe 100644 --- a/pkg/commands/git_commands/commit.go +++ b/pkg/commands/git_commands/commit.go @@ -292,9 +292,9 @@ func (self *CommitCommands) Revert(hashes []string, isMerge bool) error { } // CreateFixupCommit creates a commit that fixes up a previous commit -func (self *CommitCommands) CreateFixupCommit(hash string) error { +func (self *CommitCommands) CreateFixupCommit(hash string, originalSubject string) error { cmdArgs := NewGitCmd("commit"). - ArgIf(self.hasSkipHookPrefix("fixup! "), "--no-verify"). + ArgIf(self.hasSkipHookPrefix("fixup! "+originalSubject), "--no-verify"). Arg("--fixup=" + hash). ToArgv() @@ -308,7 +308,7 @@ func (self *CommitCommands) CreateAmendCommit(originalSubject, newSubject, newDe description += "\n\n" + newDescription } cmdArgs := NewGitCmd("commit"). - ArgIf(self.hasSkipHookPrefix("amend! "), "--no-verify"). + ArgIf(self.hasSkipHookPrefix(newSubject), "--no-verify"). Arg("-m", "amend! "+originalSubject). Arg("-m", description). ArgIf(!includeFileChanges, "--only", "--allow-empty"). diff --git a/pkg/commands/git_commands/commit_test.go b/pkg/commands/git_commands/commit_test.go index 67d2b8de3..97ef91e86 100644 --- a/pkg/commands/git_commands/commit_test.go +++ b/pkg/commands/git_commands/commit_test.go @@ -179,17 +179,19 @@ func TestCommitCommitEditorCmdObj(t *testing.T) { func TestCommitCreateFixupCommit(t *testing.T) { type scenario struct { - testName string - hash string - userConfig *config.UserConfig - runner *oscommands.FakeCmdObjRunner - test func(error) + testName string + hash string + originalSubject string + userConfig *config.UserConfig + runner *oscommands.FakeCmdObjRunner + test func(error) } scenarios := []scenario{ { - testName: "valid case", - hash: "12345", + testName: "valid case", + hash: "12345", + originalSubject: "some commit", runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"commit", "--fixup=12345"}, "", nil), test: func(err error) { @@ -197,10 +199,11 @@ func TestCommitCreateFixupCommit(t *testing.T) { }, }, { - testName: "with matching skipHookPrefixes", - hash: "12345", + testName: "with matching skipHookPrefixes for original subject", + hash: "12345", + originalSubject: "WIP do stuff", userConfig: &config.UserConfig{ - Git: config.GitConfig{SkipHookPrefixes: []string{"fixup!"}}, + Git: config.GitConfig{SkipHookPrefixes: []string{"fixup! WIP"}}, }, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"commit", "--no-verify", "--fixup=12345"}, "", nil), @@ -209,8 +212,9 @@ func TestCommitCreateFixupCommit(t *testing.T) { }, }, { - testName: "with non-matching skipHookPrefixes", - hash: "12345", + testName: "with non-matching skipHookPrefixes", + hash: "12345", + originalSubject: "some commit", userConfig: &config.UserConfig{ Git: config.GitConfig{SkipHookPrefixes: []string{"WIP"}}, }, @@ -221,10 +225,11 @@ func TestCommitCreateFixupCommit(t *testing.T) { }, }, { - testName: "with multiple prefixes including fixup!", - hash: "12345", + testName: "with multiple prefixes including fixup! WIP", + hash: "12345", + originalSubject: "WIP my feature", userConfig: &config.UserConfig{ - Git: config.GitConfig{SkipHookPrefixes: []string{"WIP", "fixup!"}}, + Git: config.GitConfig{SkipHookPrefixes: []string{"WIP", "fixup! WIP"}}, }, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"commit", "--no-verify", "--fixup=12345"}, "", nil), @@ -237,7 +242,7 @@ func TestCommitCreateFixupCommit(t *testing.T) { for _, s := range scenarios { t.Run(s.testName, func(t *testing.T) { instance := buildCommitCommands(commonDeps{runner: s.runner, userConfig: s.userConfig}) - s.test(instance.CreateFixupCommit(s.hash)) + s.test(instance.CreateFixupCommit(s.hash, s.originalSubject)) s.runner.CheckForMissingCalls() }) } @@ -283,16 +288,16 @@ func TestCommitCreateAmendCommit(t *testing.T) { ExpectGitArgs([]string{"commit", "-m", "amend! original subject", "-m", "new subject", "--only", "--allow-empty"}, "", nil), }, { - testName: "with matching skipHookPrefixes", + testName: "with matching skipHookPrefixes on new subject", originalSubject: "original subject", - newSubject: "new subject", + newSubject: "WIP new subject", newDescription: "", includeFileChanges: true, userConfig: &config.UserConfig{ - Git: config.GitConfig{SkipHookPrefixes: []string{"amend!"}}, + Git: config.GitConfig{SkipHookPrefixes: []string{"WIP"}}, }, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"commit", "--no-verify", "-m", "amend! original subject", "-m", "new subject"}, "", nil), + ExpectGitArgs([]string{"commit", "--no-verify", "-m", "amend! original subject", "-m", "WIP new subject"}, "", nil), }, { testName: "with non-matching skipHookPrefixes", @@ -307,16 +312,16 @@ func TestCommitCreateAmendCommit(t *testing.T) { ExpectGitArgs([]string{"commit", "-m", "amend! original subject", "-m", "new subject"}, "", nil), }, { - testName: "with multiple prefixes including amend!", - originalSubject: "original subject", - newSubject: "new subject", + testName: "renaming WIP commit to non-WIP runs hooks", + originalSubject: "WIP my feature", + newSubject: "Implement my feature", newDescription: "", includeFileChanges: true, userConfig: &config.UserConfig{ - Git: config.GitConfig{SkipHookPrefixes: []string{"WIP", "amend!"}}, + Git: config.GitConfig{SkipHookPrefixes: []string{"WIP"}}, }, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"commit", "--no-verify", "-m", "amend! original subject", "-m", "new subject"}, "", nil), + ExpectGitArgs([]string{"commit", "-m", "amend! WIP my feature", "-m", "Implement my feature"}, "", nil), }, } diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 97d48a1a0..12ff9c9ba 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -298,7 +298,7 @@ func (self *RebaseCommands) getHashOfLastCommitMade() (string, error) { func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) error { commit := commits[commitIndex] - if err := self.commit.CreateFixupCommit(commit.Hash()); err != nil { + if err := self.commit.CreateFixupCommit(commit.Hash(), commit.Name); err != nil { return err } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index e4f88b3ca..4e772da6c 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -1006,7 +1006,7 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err return self.c.Helpers().WorkingTree.WithEnsureCommittableFiles(func() error { self.c.LogAction(self.c.Tr.Actions.CreateFixupCommit) return self.c.WithWaitingStatusSync(self.c.Tr.CreatingFixupCommitStatus, func() error { - if err := self.c.Git().Commit.CreateFixupCommit(commit.Hash()); err != nil { + if err := self.c.Git().Commit.CreateFixupCommit(commit.Hash(), commit.Name); err != nil { return err }