From 40e3827784b1b910e53fe660997fd3ca58447e8b Mon Sep 17 00:00:00 2001 From: Korbinian Schweiger Date: Wed, 5 Mar 2025 20:41:29 +0100 Subject: [PATCH 1/4] Add verify flag --- pkg/commands/git_commands/commit.go | 9 +++--- pkg/commands/git_commands/commit_test.go | 8 ++++- pkg/commands/git_commands/patch.go | 2 +- .../custom_patch_options_menu_action.go | 2 +- pkg/gui/controllers/helpers/commits_helper.go | 30 ++++++++++++------- .../helpers/merge_and_rebase_helper.go | 2 +- pkg/gui/controllers/helpers/tags_helper.go | 3 +- .../helpers/working_tree_helper.go | 26 ++++++++-------- .../controllers/local_commits_controller.go | 6 ++-- 9 files changed, 52 insertions(+), 36 deletions(-) diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index 52a65fb6dbd..513538808cd 100644 --- a/pkg/commands/git_commands/commit.go +++ b/pkg/commands/git_commands/commit.go @@ -85,13 +85,11 @@ func (self *CommitCommands) ResetToCommit(hash string, strength string, envVars Run() } -func (self *CommitCommands) CommitCmdObj(summary string, description string) oscommands.ICmdObj { +func (self *CommitCommands) CommitCmdObj(summary string, description string, verify bool) oscommands.ICmdObj { messageArgs := self.commitMessageArgs(summary, description) - skipHookPrefix := self.UserConfig().Git.SkipHookPrefix - cmdArgs := NewGitCmd("commit"). - ArgIf(skipHookPrefix != "" && strings.HasPrefix(summary, skipHookPrefix), "--no-verify"). + ArgIf(verify, "--no-verify"). ArgIf(self.signoffFlag() != "", self.signoffFlag()). Arg(messageArgs...). ToArgv() @@ -108,8 +106,9 @@ func (self *CommitCommands) RewordLastCommitInEditorWithMessageFileCmdObj(tmpMes Arg("--allow-empty", "--amend", "--only", "--edit", "--file="+tmpMessageFile).ToArgv()) } -func (self *CommitCommands) CommitInEditorWithMessageFileCmdObj(tmpMessageFile string) oscommands.ICmdObj { +func (self *CommitCommands) CommitInEditorWithMessageFileCmdObj(tmpMessageFile string, verify bool) oscommands.ICmdObj { return self.cmd.New(NewGitCmd("commit"). + ArgIf(verify, "--no-verify"). Arg("--edit"). Arg("--file="+tmpMessageFile). ArgIf(self.signoffFlag() != "", self.signoffFlag()). diff --git a/pkg/commands/git_commands/commit_test.go b/pkg/commands/git_commands/commit_test.go index a522c81d0d8..2a19f96fbc7 100644 --- a/pkg/commands/git_commands/commit_test.go +++ b/pkg/commands/git_commands/commit_test.go @@ -53,6 +53,7 @@ func TestCommitCommitCmdObj(t *testing.T) { type scenario struct { testName string summary string + verify bool description string configSignoff bool configSkipHookPrefix string @@ -63,6 +64,7 @@ func TestCommitCommitCmdObj(t *testing.T) { { testName: "Commit", summary: "test", + verify: false, configSignoff: false, configSkipHookPrefix: "", expectedArgs: []string{"commit", "-m", "test"}, @@ -70,6 +72,7 @@ func TestCommitCommitCmdObj(t *testing.T) { { testName: "Commit with --no-verify flag", summary: "WIP: test", + verify: true, configSignoff: false, configSkipHookPrefix: "WIP", expectedArgs: []string{"commit", "--no-verify", "-m", "WIP: test"}, @@ -77,6 +80,7 @@ func TestCommitCommitCmdObj(t *testing.T) { { testName: "Commit with multiline message", summary: "line1", + verify: false, description: "line2", configSignoff: false, configSkipHookPrefix: "", @@ -85,6 +89,7 @@ func TestCommitCommitCmdObj(t *testing.T) { { testName: "Commit with signoff", summary: "test", + verify: false, configSignoff: true, configSkipHookPrefix: "", expectedArgs: []string{"commit", "--signoff", "-m", "test"}, @@ -92,6 +97,7 @@ func TestCommitCommitCmdObj(t *testing.T) { { testName: "Commit with signoff and no-verify", summary: "WIP: test", + verify: true, configSignoff: true, configSkipHookPrefix: "WIP", expectedArgs: []string{"commit", "--no-verify", "--signoff", "-m", "WIP: test"}, @@ -107,7 +113,7 @@ func TestCommitCommitCmdObj(t *testing.T) { runner := oscommands.NewFakeRunner(t).ExpectGitArgs(s.expectedArgs, "", nil) instance := buildCommitCommands(commonDeps{userConfig: userConfig, runner: runner}) - assert.NoError(t, instance.CommitCmdObj(s.summary, s.description).Run()) + assert.NoError(t, instance.CommitCmdObj(s.summary, s.description, s.verify).Run()) runner.CheckForMissingCalls() }) } diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index bceaf994365..830ee2cb6d5 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -303,7 +303,7 @@ func (self *PatchCommands) PullPatchIntoNewCommit( return err } - if err := self.commit.CommitCmdObj(commitSummary, commitDescription).Run(); err != nil { + if err := self.commit.CommitCmdObj(commitSummary, commitDescription, false).Run(); err != nil { return err } diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index 04895ed6b42..a387bc9f482 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -205,7 +205,7 @@ func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommit() error { SummaryTitle: self.c.Tr.CommitSummaryTitle, DescriptionTitle: self.c.Tr.CommitDescriptionTitle, PreserveMessage: false, - OnConfirm: func(summary string, description string) error { + OnConfirm: func(summary string, description string, verify bool) error { return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { self.c.Helpers().Commits.CloseCommitMessagePanel() self.c.LogAction(self.c.Tr.Actions.MovePatchIntoNewCommit) diff --git a/pkg/gui/controllers/helpers/commits_helper.go b/pkg/gui/controllers/helpers/commits_helper.go index 9c89da706e0..f098f966294 100644 --- a/pkg/gui/controllers/helpers/commits_helper.go +++ b/pkg/gui/controllers/helpers/commits_helper.go @@ -19,9 +19,10 @@ type ICommitsHelper interface { type CommitsHelper struct { c *HelperCommon - getCommitSummary func() string - setCommitSummary func(string) - getCommitDescription func() string + getCommitSummary func() string + setCommitSummary func(string) + getCommitDescription func() string + // getVerifyFlag func() bool getUnwrappedCommitDescription func() string setCommitDescription func(string) } @@ -33,14 +34,16 @@ func NewCommitsHelper( getCommitSummary func() string, setCommitSummary func(string), getCommitDescription func() string, + // getVerifyFlag func() bool, getUnwrappedCommitDescription func() string, setCommitDescription func(string), ) *CommitsHelper { return &CommitsHelper{ - c: c, - getCommitSummary: getCommitSummary, - setCommitSummary: setCommitSummary, - getCommitDescription: getCommitDescription, + c: c, + getCommitSummary: getCommitSummary, + setCommitSummary: setCommitSummary, + getCommitDescription: getCommitDescription, + // getVerifyFlag: getVerifyFlag, getUnwrappedCommitDescription: getUnwrappedCommitDescription, setCommitDescription: setCommitDescription, } @@ -125,9 +128,10 @@ type OpenCommitMessagePanelOpts struct { CommitIndex int SummaryTitle string DescriptionTitle string + verify bool PreserveMessage bool - OnConfirm func(summary string, description string) error - OnSwitchToEditor func(string) error + OnConfirm func(summary string, description string, verify bool) error + OnSwitchToEditor func(string, bool) error InitialMessage string } @@ -135,7 +139,11 @@ func (self *CommitsHelper) OpenCommitMessagePanel(opts *OpenCommitMessagePanelOp onConfirm := func(summary string, description string) error { self.CloseCommitMessagePanel() - return opts.OnConfirm(summary, description) + return opts.OnConfirm(summary, description, opts.verify) + } + + onSwitchToEditor := func(message string) error { + return opts.OnSwitchToEditor(message, opts.verify) } self.c.Contexts().CommitMessage.SetPanelState( @@ -145,7 +153,7 @@ func (self *CommitsHelper) OpenCommitMessagePanel(opts *OpenCommitMessagePanelOp opts.PreserveMessage, opts.InitialMessage, onConfirm, - opts.OnSwitchToEditor, + onSwitchToEditor, ) self.UpdateCommitPanelView(opts.InitialMessage) diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index 40d9e6df2c6..04592519b3c 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -481,7 +481,7 @@ func (self *MergeAndRebaseHelper) SquashMergeCommitted(refName, checkedOutBranch "selectedRef": refName, "currentBranch": checkedOutBranchName, }) - err = self.c.Git().Commit.CommitCmdObj(message, "").Run() + err = self.c.Git().Commit.CommitCmdObj(message, "", false).Run() if err != nil { return err } diff --git a/pkg/gui/controllers/helpers/tags_helper.go b/pkg/gui/controllers/helpers/tags_helper.go index aa6ff7740ae..de0f91aba1b 100644 --- a/pkg/gui/controllers/helpers/tags_helper.go +++ b/pkg/gui/controllers/helpers/tags_helper.go @@ -42,7 +42,7 @@ func (self *TagsHelper) OpenCreateTagPrompt(ref string, onCreate func()) error { }) } - onConfirm := func(tagName string, description string) error { + onConfirm := func(tagName string, description string, verify bool) error { if self.c.Git().Tag.HasTag(tagName) { prompt := utils.ResolvePlaceholderString( self.c.Tr.ForceTagPrompt, @@ -72,6 +72,7 @@ func (self *TagsHelper) OpenCreateTagPrompt(ref string, onCreate func()) error { InitialMessage: "", SummaryTitle: self.c.Tr.TagNameTitle, DescriptionTitle: self.c.Tr.TagMessageTitle, + verify: false, PreserveMessage: false, OnConfirm: onConfirm, }, diff --git a/pkg/gui/controllers/helpers/working_tree_helper.go b/pkg/gui/controllers/helpers/working_tree_helper.go index c967fab92cf..87ecc98e5c7 100644 --- a/pkg/gui/controllers/helpers/working_tree_helper.go +++ b/pkg/gui/controllers/helpers/working_tree_helper.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "regexp" + "strings" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/config" @@ -86,7 +87,7 @@ func (self *WorkingTreeHelper) OpenMergeTool() error { return nil } -func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage string) error { +func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage string, verify bool) error { return self.WithEnsureCommittableFiles(func() error { self.commitsHelper.OpenCommitMessagePanel( &OpenCommitMessagePanelOpts{ @@ -94,6 +95,7 @@ func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage strin InitialMessage: initialMessage, SummaryTitle: self.c.Tr.CommitSummaryTitle, DescriptionTitle: self.c.Tr.CommitDescriptionTitle, + verify: verify, PreserveMessage: true, OnConfirm: self.handleCommit, OnSwitchToEditor: self.switchFromCommitMessagePanelToEditor, @@ -104,8 +106,12 @@ func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage strin }) } -func (self *WorkingTreeHelper) handleCommit(summary string, description string) error { - cmdObj := self.c.Git().Commit.CommitCmdObj(summary, description) +func (self *WorkingTreeHelper) handleCommit(summary string, description string, verify bool) error { + skipHookPrefix := self.c.UserConfig().Git.SkipHookPrefix + if !verify && skipHookPrefix != "" { + verify = strings.HasPrefix(summary, skipHookPrefix) + } + cmdObj := self.c.Git().Commit.CommitCmdObj(summary, description, verify) self.c.LogAction(self.c.Tr.Actions.Commit) return self.gpgHelper.WithGpgHandling(cmdObj, self.c.Tr.CommittingStatus, func() error { self.commitsHelper.OnCommitSuccess() @@ -113,7 +119,7 @@ func (self *WorkingTreeHelper) handleCommit(summary string, description string) }) } -func (self *WorkingTreeHelper) switchFromCommitMessagePanelToEditor(filepath string) error { +func (self *WorkingTreeHelper) switchFromCommitMessagePanelToEditor(filepath string, verify bool) error { // We won't be able to tell whether the commit was successful, because // RunSubprocessAndRefresh doesn't return the error (it opens an error alert // itself and returns nil on error). But even if we could, we wouldn't have @@ -124,7 +130,7 @@ func (self *WorkingTreeHelper) switchFromCommitMessagePanelToEditor(filepath str self.c.LogAction(self.c.Tr.Actions.Commit) return self.c.RunSubprocessAndRefresh( - self.c.Git().Commit.CommitInEditorWithMessageFileCmdObj(filepath), + self.c.Git().Commit.CommitInEditorWithMessageFileCmdObj(filepath, verify), ) } @@ -140,12 +146,7 @@ func (self *WorkingTreeHelper) HandleCommitEditorPress() error { } func (self *WorkingTreeHelper) HandleWIPCommitPress() error { - skipHookPrefix := self.c.UserConfig().Git.SkipHookPrefix - if skipHookPrefix == "" { - return errors.New(self.c.Tr.SkipHookPrefixNotConfigured) - } - - return self.HandleCommitPressWithMessage(skipHookPrefix) + return self.HandleCommitPressWithMessage("", true) } func (self *WorkingTreeHelper) HandleCommitPress() error { @@ -170,7 +171,8 @@ func (self *WorkingTreeHelper) HandleCommitPress() error { } } - return self.HandleCommitPressWithMessage(message) + verify := false + return self.HandleCommitPressWithMessage(message, verify) } func (self *WorkingTreeHelper) WithEnsureCommittableFiles(handler func() error) error { diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 53885c8f5ff..318b82264b9 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -382,7 +382,7 @@ func (self *LocalCommitsController) reword(commit *models.Commit) error { return nil } -func (self *LocalCommitsController) switchFromCommitMessagePanelToEditor(filepath string) error { +func (self *LocalCommitsController) switchFromCommitMessagePanelToEditor(filepath string, verify bool) error { if self.isSelectedHeadCommit() { return self.c.RunSubprocessAndRefresh( self.c.Git().Commit.RewordLastCommitInEditorWithMessageFileCmdObj(filepath)) @@ -408,7 +408,7 @@ func (self *LocalCommitsController) switchFromCommitMessagePanelToEditor(filepat return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) } -func (self *LocalCommitsController) handleReword(summary string, description string) error { +func (self *LocalCommitsController) handleReword(summary string, description string, verify bool) error { if models.IsHeadCommit(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx()) { // we've selected the top commit so no rebase is required return self.c.Helpers().GPG.WithGpgHandling(self.c.Git().Commit.RewordLastCommit(summary, description), @@ -1026,7 +1026,7 @@ func (self *LocalCommitsController) createAmendCommit(commit *models.Commit, inc SummaryTitle: self.c.Tr.CreateAmendCommit, DescriptionTitle: self.c.Tr.CommitDescriptionTitle, PreserveMessage: false, - OnConfirm: func(summary string, description string) error { + OnConfirm: func(summary string, description string, verify bool) 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.CreateAmendCommit(originalSubject, summary, description, includeFileChanges); err != nil { From d6ecdd499eefd1b424589595c198cbb4c17db9aa Mon Sep 17 00:00:00 2001 From: Korbinian Schweiger Date: Fri, 7 Mar 2025 20:03:51 +0100 Subject: [PATCH 2/4] Add and update integration tests --- pkg/gui/controllers/helpers/commits_helper.go | 17 +++--- pkg/integration/components/views.go | 4 ++ .../tests/commit/commit_skip_hooks.go | 35 +++++++++++ .../commit_switch_to_editor_skip_hooks.go | 55 +++++++++++++++++ .../tests/commit/commit_wip_with_prefix.go | 14 ++--- .../commit/fail_hooks_then_commit_no_hooks.go | 59 +++++++++++++++++++ .../tests/commit/staged_without_hooks.go | 7 ++- pkg/integration/tests/test_list.go | 3 + 8 files changed, 174 insertions(+), 20 deletions(-) create mode 100644 pkg/integration/tests/commit/commit_skip_hooks.go create mode 100644 pkg/integration/tests/commit/commit_switch_to_editor_skip_hooks.go create mode 100644 pkg/integration/tests/commit/fail_hooks_then_commit_no_hooks.go diff --git a/pkg/gui/controllers/helpers/commits_helper.go b/pkg/gui/controllers/helpers/commits_helper.go index f098f966294..12b0ca7d499 100644 --- a/pkg/gui/controllers/helpers/commits_helper.go +++ b/pkg/gui/controllers/helpers/commits_helper.go @@ -19,10 +19,9 @@ type ICommitsHelper interface { type CommitsHelper struct { c *HelperCommon - getCommitSummary func() string - setCommitSummary func(string) - getCommitDescription func() string - // getVerifyFlag func() bool + getCommitSummary func() string + setCommitSummary func(string) + getCommitDescription func() string getUnwrappedCommitDescription func() string setCommitDescription func(string) } @@ -34,16 +33,14 @@ func NewCommitsHelper( getCommitSummary func() string, setCommitSummary func(string), getCommitDescription func() string, - // getVerifyFlag func() bool, getUnwrappedCommitDescription func() string, setCommitDescription func(string), ) *CommitsHelper { return &CommitsHelper{ - c: c, - getCommitSummary: getCommitSummary, - setCommitSummary: setCommitSummary, - getCommitDescription: getCommitDescription, - // getVerifyFlag: getVerifyFlag, + c: c, + getCommitSummary: getCommitSummary, + setCommitSummary: setCommitSummary, + getCommitDescription: getCommitDescription, getUnwrappedCommitDescription: getUnwrappedCommitDescription, setCommitDescription: setCommitDescription, } diff --git a/pkg/integration/components/views.go b/pkg/integration/components/views.go index 873aca6500c..80bc0bb947f 100644 --- a/pkg/integration/components/views.go +++ b/pkg/integration/components/views.go @@ -42,6 +42,10 @@ func (self *Views) MergeConflicts() *ViewDriver { return self.regularView("mergeConflicts") } +func (self *Views) Extras() *ViewDriver { + return self.regularView("extras") +} + func (self *Views) Commits() *ViewDriver { return self.regularView("commits") } diff --git a/pkg/integration/tests/commit/commit_skip_hooks.go b/pkg/integration/tests/commit/commit_skip_hooks.go new file mode 100644 index 00000000000..3687012e76f --- /dev/null +++ b/pkg/integration/tests/commit/commit_skip_hooks.go @@ -0,0 +1,35 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var CommitSkipHooks = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Commit with skip hook using CommitChangesWithoutHook", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.NewBranch("feature/TEST-002") + shell.CreateFile("test-wip-commit-prefix", "This is foo bar") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + IsEmpty() + + t.Views().Files(). + IsFocused(). + PressPrimaryAction(). + Press(keys.Files.CommitChangesWithoutHook) + + t.ExpectPopup().CommitMessagePanel(). + Title(Equals("Commit summary")). + Type("foo bar"). + Confirm() + + t.Views().Commits().Focus() + t.Views().Main().Content(Contains("foo bar")) + t.Views().Extras().Content(Contains("--no-verify")) + }, +}) diff --git a/pkg/integration/tests/commit/commit_switch_to_editor_skip_hooks.go b/pkg/integration/tests/commit/commit_switch_to_editor_skip_hooks.go new file mode 100644 index 00000000000..94f7de1f949 --- /dev/null +++ b/pkg/integration/tests/commit/commit_switch_to_editor_skip_hooks.go @@ -0,0 +1,55 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var CommitSwitchToEditorSkipHooks = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Commit, then switch from built-in commit message panel to editor", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFile("file1", "file1 content") + shell.CreateFile("file2", "file2 content") + + // Set an editor that appends a line to the existing message. Since + // git adds all this "# Please enter the commit message for your changes" + // stuff, this will result in an extra blank line before the added line. + shell.SetConfig("core.editor", "sh -c 'echo third line >>.git/COMMIT_EDITMSG'") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + IsEmpty() + + t.Views().Files(). + IsFocused(). + PressPrimaryAction(). // stage one of the files + Press(keys.Files.CommitChangesWithoutHook) + + t.ExpectPopup().CommitMessagePanel(). + Type("first line"). + SwitchToDescription(). + Type("second line"). + SwitchToSummary(). + SwitchToEditor() + t.Views().Commits(). + Lines( + Contains("first line"), + ) + + t.Views().Commits().Focus() + t.Views().Main().Content(MatchesRegexp(`first line\n\s*\n\s*second line\n\s*\n\s*third line`)) + t.Views().Extras().Content(Contains("--no-verify")) + + // Now check that the preserved commit message was cleared: + t.Views().Files(). + Focus(). + PressPrimaryAction(). // stage the other file + Press(keys.Files.CommitChanges) + + t.ExpectPopup().CommitMessagePanel(). + InitialText(Equals("")) + }, +}) diff --git a/pkg/integration/tests/commit/commit_wip_with_prefix.go b/pkg/integration/tests/commit/commit_wip_with_prefix.go index 6223de04fc4..2d051f92d80 100644 --- a/pkg/integration/tests/commit/commit_wip_with_prefix.go +++ b/pkg/integration/tests/commit/commit_wip_with_prefix.go @@ -27,31 +27,31 @@ var CommitWipWithPrefix = NewIntegrationTest(NewIntegrationTestArgs{ t.ExpectPopup().CommitMessagePanel(). Title(Equals("Commit summary")). - InitialText(Equals("WIP")). - Type(" foo"). + Type("foo"). Cancel() t.Views().Files(). IsFocused(). - Press(keys.Files.CommitChanges) + Press(keys.Files.CommitChangesWithoutHook) t.ExpectPopup().CommitMessagePanel(). Title(Equals("Commit summary")). - InitialText(Equals("WIP foo")). + InitialText(Equals("foo")). Type(" bar"). Cancel() t.Views().Files(). IsFocused(). - Press(keys.Files.CommitChanges) + Press(keys.Files.CommitChangesWithoutHook) t.ExpectPopup().CommitMessagePanel(). Title(Equals("Commit summary")). - InitialText(Equals("WIP foo bar")). + InitialText(Equals("foo bar")). Type(". Added something else"). Confirm() t.Views().Commits().Focus() - t.Views().Main().Content(Contains("WIP foo bar. Added something else")) + t.Views().Main().Content(Contains("foo bar. Added something else")) + t.Views().Extras().Content(Contains("--no-verify")) }, }) diff --git a/pkg/integration/tests/commit/fail_hooks_then_commit_no_hooks.go b/pkg/integration/tests/commit/fail_hooks_then_commit_no_hooks.go new file mode 100644 index 00000000000..cb1397e85be --- /dev/null +++ b/pkg/integration/tests/commit/fail_hooks_then_commit_no_hooks.go @@ -0,0 +1,59 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var preCommitHook = `#!/bin/bash + +if [[ -f bad ]]; then + exit 1 +fi +` + +var FailHooksThenCommitNoHooks = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Verify that commit message can be reused in commit without hook after failing commit with hooks", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + }, + SetupRepo: func(shell *Shell) { + shell.CreateFile(".git/hooks/pre-commit", preCommitHook) + shell.MakeExecutable(".git/hooks/pre-commit") + + shell.CreateFileAndAdd("one", "one") + + // the presence of this file will cause the pre-commit hook to fail + shell.CreateFileAndAdd("bad", "bad") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Lines( + Contains("bad"), + Contains("one"), + ). + Press(keys.Files.CommitChanges). + Tap(func() { + t.ExpectPopup().CommitMessagePanel().Type("my message").Confirm() + + t.ExpectPopup().Alert().Title(Equals("Error")).Content(Contains("Git command failed")).Confirm() + }). + Press(keys.Files.CommitChangesWithoutHook). + Tap(func() { + t.ExpectPopup().CommitMessagePanel(). + InitialText(Equals("my message")). // it remembered the commit message + Confirm() + + t.Views().Commits(). + Lines( + Contains("my message"), + ) + }) + t.Views().Commits().Focus() + t.Views().Main().Content(Contains("my message")) + + t.Views().Extras().Content(Contains("--no-verify")) + }, +}) diff --git a/pkg/integration/tests/commit/staged_without_hooks.go b/pkg/integration/tests/commit/staged_without_hooks.go index fcf53dd22a3..94a9aacf2a1 100644 --- a/pkg/integration/tests/commit/staged_without_hooks.go +++ b/pkg/integration/tests/commit/staged_without_hooks.go @@ -45,12 +45,13 @@ var StagedWithoutHooks = NewIntegrationTest(NewIntegrationTestArgs{ Content(DoesNotContain("+myfile content").Contains("+with a second line")). Press(keys.Files.CommitChangesWithoutHook) - commitMessage := ": my commit message" - t.ExpectPopup().CommitMessagePanel().InitialText(Contains("WIP")).Type(commitMessage).Confirm() + commitMessage := "my commit message" + t.ExpectPopup().CommitMessagePanel().Type(commitMessage).Confirm() + t.Views().Extras().Content(Contains("--no-verify")) t.Views().Commits(). Lines( - Contains("WIP" + commitMessage), + Contains(commitMessage), ) t.Views().StagingSecondary(). diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 69a9ed5ec3a..73dda6ef45b 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -91,7 +91,9 @@ var tests = []*components.IntegrationTest{ commit.Checkout, commit.Commit, commit.CommitMultiline, + commit.CommitSkipHooks, commit.CommitSwitchToEditor, + commit.CommitSwitchToEditorSkipHooks, commit.CommitWipWithPrefix, commit.CommitWithFallthroughPrefix, commit.CommitWithGlobalPrefix, @@ -105,6 +107,7 @@ var tests = []*components.IntegrationTest{ commit.CreateTag, commit.DisableCopyCommitMessageBody, commit.DiscardOldFileChanges, + commit.FailHooksThenCommitNoHooks, commit.FindBaseCommitForFixup, commit.FindBaseCommitForFixupDisregardMainBranch, commit.FindBaseCommitForFixupOnlyAddedLines, From 97e05e5febcf61b298c60ab156ccf7beb1fd2a6f Mon Sep 17 00:00:00 2001 From: Korbinian Schweiger Date: Mon, 10 Mar 2025 19:52:19 +0100 Subject: [PATCH 3/4] Rename verify to forceSkipHooks --- pkg/commands/git_commands/commit.go | 10 +++--- pkg/commands/git_commands/commit_test.go | 32 ++++++++++++++----- .../custom_patch_options_menu_action.go | 2 +- pkg/gui/controllers/helpers/commits_helper.go | 8 ++--- pkg/gui/controllers/helpers/tags_helper.go | 4 +-- .../helpers/working_tree_helper.go | 20 ++++-------- .../controllers/local_commits_controller.go | 6 ++-- 7 files changed, 46 insertions(+), 36 deletions(-) diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index 513538808cd..1fd01dd12d4 100644 --- a/pkg/commands/git_commands/commit.go +++ b/pkg/commands/git_commands/commit.go @@ -85,11 +85,11 @@ func (self *CommitCommands) ResetToCommit(hash string, strength string, envVars Run() } -func (self *CommitCommands) CommitCmdObj(summary string, description string, verify bool) oscommands.ICmdObj { +func (self *CommitCommands) CommitCmdObj(summary string, description string, forceSkipHooks bool) oscommands.ICmdObj { messageArgs := self.commitMessageArgs(summary, description) - + skipHookPrefix := self.UserConfig().Git.SkipHookPrefix cmdArgs := NewGitCmd("commit"). - ArgIf(verify, "--no-verify"). + ArgIf(forceSkipHooks || (skipHookPrefix != "" && strings.HasPrefix(summary, skipHookPrefix)), "--no-verify"). ArgIf(self.signoffFlag() != "", self.signoffFlag()). Arg(messageArgs...). ToArgv() @@ -106,9 +106,9 @@ func (self *CommitCommands) RewordLastCommitInEditorWithMessageFileCmdObj(tmpMes Arg("--allow-empty", "--amend", "--only", "--edit", "--file="+tmpMessageFile).ToArgv()) } -func (self *CommitCommands) CommitInEditorWithMessageFileCmdObj(tmpMessageFile string, verify bool) oscommands.ICmdObj { +func (self *CommitCommands) CommitInEditorWithMessageFileCmdObj(tmpMessageFile string, forceSkipHooks bool) oscommands.ICmdObj { return self.cmd.New(NewGitCmd("commit"). - ArgIf(verify, "--no-verify"). + ArgIf(forceSkipHooks, "--no-verify"). Arg("--edit"). Arg("--file="+tmpMessageFile). ArgIf(self.signoffFlag() != "", self.signoffFlag()). diff --git a/pkg/commands/git_commands/commit_test.go b/pkg/commands/git_commands/commit_test.go index 2a19f96fbc7..a00a4ff237c 100644 --- a/pkg/commands/git_commands/commit_test.go +++ b/pkg/commands/git_commands/commit_test.go @@ -53,7 +53,7 @@ func TestCommitCommitCmdObj(t *testing.T) { type scenario struct { testName string summary string - verify bool + forceSkipHooks bool description string configSignoff bool configSkipHookPrefix string @@ -64,23 +64,39 @@ func TestCommitCommitCmdObj(t *testing.T) { { testName: "Commit", summary: "test", - verify: false, + forceSkipHooks: false, configSignoff: false, configSkipHookPrefix: "", expectedArgs: []string{"commit", "-m", "test"}, }, { - testName: "Commit with --no-verify flag", + testName: "Commit with --no-verify flag < only prefix", summary: "WIP: test", - verify: true, + forceSkipHooks: false, configSignoff: false, configSkipHookPrefix: "WIP", expectedArgs: []string{"commit", "--no-verify", "-m", "WIP: test"}, }, + { + testName: "Commit with --no-verify flag < skip flag and prefix", + summary: "WIP: test", + forceSkipHooks: true, + configSignoff: false, + configSkipHookPrefix: "WIP", + expectedArgs: []string{"commit", "--no-verify", "-m", "WIP: test"}, + }, + { + testName: "Commit with --no-verify flag < skip flag no prefix", + summary: "test", + forceSkipHooks: true, + configSignoff: false, + configSkipHookPrefix: "WIP", + expectedArgs: []string{"commit", "--no-verify", "-m", "test"}, + }, { testName: "Commit with multiline message", summary: "line1", - verify: false, + forceSkipHooks: false, description: "line2", configSignoff: false, configSkipHookPrefix: "", @@ -89,7 +105,7 @@ func TestCommitCommitCmdObj(t *testing.T) { { testName: "Commit with signoff", summary: "test", - verify: false, + forceSkipHooks: false, configSignoff: true, configSkipHookPrefix: "", expectedArgs: []string{"commit", "--signoff", "-m", "test"}, @@ -97,7 +113,7 @@ func TestCommitCommitCmdObj(t *testing.T) { { testName: "Commit with signoff and no-verify", summary: "WIP: test", - verify: true, + forceSkipHooks: true, configSignoff: true, configSkipHookPrefix: "WIP", expectedArgs: []string{"commit", "--no-verify", "--signoff", "-m", "WIP: test"}, @@ -113,7 +129,7 @@ func TestCommitCommitCmdObj(t *testing.T) { runner := oscommands.NewFakeRunner(t).ExpectGitArgs(s.expectedArgs, "", nil) instance := buildCommitCommands(commonDeps{userConfig: userConfig, runner: runner}) - assert.NoError(t, instance.CommitCmdObj(s.summary, s.description, s.verify).Run()) + assert.NoError(t, instance.CommitCmdObj(s.summary, s.description, s.forceSkipHooks).Run()) runner.CheckForMissingCalls() }) } diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index a387bc9f482..c46498a0bcd 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -205,7 +205,7 @@ func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommit() error { SummaryTitle: self.c.Tr.CommitSummaryTitle, DescriptionTitle: self.c.Tr.CommitDescriptionTitle, PreserveMessage: false, - OnConfirm: func(summary string, description string, verify bool) error { + OnConfirm: func(summary string, description string, forceSkipHooks bool) error { return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { self.c.Helpers().Commits.CloseCommitMessagePanel() self.c.LogAction(self.c.Tr.Actions.MovePatchIntoNewCommit) diff --git a/pkg/gui/controllers/helpers/commits_helper.go b/pkg/gui/controllers/helpers/commits_helper.go index 12b0ca7d499..762e6b3043b 100644 --- a/pkg/gui/controllers/helpers/commits_helper.go +++ b/pkg/gui/controllers/helpers/commits_helper.go @@ -125,9 +125,9 @@ type OpenCommitMessagePanelOpts struct { CommitIndex int SummaryTitle string DescriptionTitle string - verify bool + forceSkipHooks bool PreserveMessage bool - OnConfirm func(summary string, description string, verify bool) error + OnConfirm func(summary string, description string, forceSkipHooks bool) error OnSwitchToEditor func(string, bool) error InitialMessage string } @@ -136,11 +136,11 @@ func (self *CommitsHelper) OpenCommitMessagePanel(opts *OpenCommitMessagePanelOp onConfirm := func(summary string, description string) error { self.CloseCommitMessagePanel() - return opts.OnConfirm(summary, description, opts.verify) + return opts.OnConfirm(summary, description, opts.forceSkipHooks) } onSwitchToEditor := func(message string) error { - return opts.OnSwitchToEditor(message, opts.verify) + return opts.OnSwitchToEditor(message, opts.forceSkipHooks) } self.c.Contexts().CommitMessage.SetPanelState( diff --git a/pkg/gui/controllers/helpers/tags_helper.go b/pkg/gui/controllers/helpers/tags_helper.go index de0f91aba1b..d5c50ccab8b 100644 --- a/pkg/gui/controllers/helpers/tags_helper.go +++ b/pkg/gui/controllers/helpers/tags_helper.go @@ -42,7 +42,7 @@ func (self *TagsHelper) OpenCreateTagPrompt(ref string, onCreate func()) error { }) } - onConfirm := func(tagName string, description string, verify bool) error { + onConfirm := func(tagName string, description string, forceSkipHooks bool) error { if self.c.Git().Tag.HasTag(tagName) { prompt := utils.ResolvePlaceholderString( self.c.Tr.ForceTagPrompt, @@ -72,7 +72,7 @@ func (self *TagsHelper) OpenCreateTagPrompt(ref string, onCreate func()) error { InitialMessage: "", SummaryTitle: self.c.Tr.TagNameTitle, DescriptionTitle: self.c.Tr.TagMessageTitle, - verify: false, + forceSkipHooks: false, PreserveMessage: false, OnConfirm: onConfirm, }, diff --git a/pkg/gui/controllers/helpers/working_tree_helper.go b/pkg/gui/controllers/helpers/working_tree_helper.go index 87ecc98e5c7..2c5b98569fd 100644 --- a/pkg/gui/controllers/helpers/working_tree_helper.go +++ b/pkg/gui/controllers/helpers/working_tree_helper.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "regexp" - "strings" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/config" @@ -87,7 +86,7 @@ func (self *WorkingTreeHelper) OpenMergeTool() error { return nil } -func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage string, verify bool) error { +func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage string, forceSkipHooks bool) error { return self.WithEnsureCommittableFiles(func() error { self.commitsHelper.OpenCommitMessagePanel( &OpenCommitMessagePanelOpts{ @@ -95,7 +94,7 @@ func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage strin InitialMessage: initialMessage, SummaryTitle: self.c.Tr.CommitSummaryTitle, DescriptionTitle: self.c.Tr.CommitDescriptionTitle, - verify: verify, + forceSkipHooks: forceSkipHooks, PreserveMessage: true, OnConfirm: self.handleCommit, OnSwitchToEditor: self.switchFromCommitMessagePanelToEditor, @@ -106,12 +105,8 @@ func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage strin }) } -func (self *WorkingTreeHelper) handleCommit(summary string, description string, verify bool) error { - skipHookPrefix := self.c.UserConfig().Git.SkipHookPrefix - if !verify && skipHookPrefix != "" { - verify = strings.HasPrefix(summary, skipHookPrefix) - } - cmdObj := self.c.Git().Commit.CommitCmdObj(summary, description, verify) +func (self *WorkingTreeHelper) handleCommit(summary string, description string, forceSkipHooks bool) error { + cmdObj := self.c.Git().Commit.CommitCmdObj(summary, description, forceSkipHooks) self.c.LogAction(self.c.Tr.Actions.Commit) return self.gpgHelper.WithGpgHandling(cmdObj, self.c.Tr.CommittingStatus, func() error { self.commitsHelper.OnCommitSuccess() @@ -119,7 +114,7 @@ func (self *WorkingTreeHelper) handleCommit(summary string, description string, }) } -func (self *WorkingTreeHelper) switchFromCommitMessagePanelToEditor(filepath string, verify bool) error { +func (self *WorkingTreeHelper) switchFromCommitMessagePanelToEditor(filepath string, forceSkipHooks bool) error { // We won't be able to tell whether the commit was successful, because // RunSubprocessAndRefresh doesn't return the error (it opens an error alert // itself and returns nil on error). But even if we could, we wouldn't have @@ -130,7 +125,7 @@ func (self *WorkingTreeHelper) switchFromCommitMessagePanelToEditor(filepath str self.c.LogAction(self.c.Tr.Actions.Commit) return self.c.RunSubprocessAndRefresh( - self.c.Git().Commit.CommitInEditorWithMessageFileCmdObj(filepath, verify), + self.c.Git().Commit.CommitInEditorWithMessageFileCmdObj(filepath, forceSkipHooks), ) } @@ -171,8 +166,7 @@ func (self *WorkingTreeHelper) HandleCommitPress() error { } } - verify := false - return self.HandleCommitPressWithMessage(message, verify) + return self.HandleCommitPressWithMessage(message, false) } func (self *WorkingTreeHelper) WithEnsureCommittableFiles(handler func() error) error { diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 318b82264b9..a31b958a078 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -382,7 +382,7 @@ func (self *LocalCommitsController) reword(commit *models.Commit) error { return nil } -func (self *LocalCommitsController) switchFromCommitMessagePanelToEditor(filepath string, verify bool) error { +func (self *LocalCommitsController) switchFromCommitMessagePanelToEditor(filepath string, forceSkipHooks bool) error { if self.isSelectedHeadCommit() { return self.c.RunSubprocessAndRefresh( self.c.Git().Commit.RewordLastCommitInEditorWithMessageFileCmdObj(filepath)) @@ -408,7 +408,7 @@ func (self *LocalCommitsController) switchFromCommitMessagePanelToEditor(filepat return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) } -func (self *LocalCommitsController) handleReword(summary string, description string, verify bool) error { +func (self *LocalCommitsController) handleReword(summary string, description string, forceSkipHooks bool) error { if models.IsHeadCommit(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx()) { // we've selected the top commit so no rebase is required return self.c.Helpers().GPG.WithGpgHandling(self.c.Git().Commit.RewordLastCommit(summary, description), @@ -1026,7 +1026,7 @@ func (self *LocalCommitsController) createAmendCommit(commit *models.Commit, inc SummaryTitle: self.c.Tr.CreateAmendCommit, DescriptionTitle: self.c.Tr.CommitDescriptionTitle, PreserveMessage: false, - OnConfirm: func(summary string, description string, verify bool) error { + OnConfirm: func(summary string, description string, forceSkipHooks bool) 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.CreateAmendCommit(originalSubject, summary, description, includeFileChanges); err != nil { From 2a4911c68f450d33377bda8e7c0471bf28394fee Mon Sep 17 00:00:00 2001 From: Korbinian Schweiger Date: Mon, 10 Mar 2025 21:29:02 +0100 Subject: [PATCH 4/4] Adapt CommitSkipHooks integration test to actually use a hook --- pkg/integration/tests/commit/commit_skip_hooks.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/integration/tests/commit/commit_skip_hooks.go b/pkg/integration/tests/commit/commit_skip_hooks.go index 3687012e76f..1cff20eb669 100644 --- a/pkg/integration/tests/commit/commit_skip_hooks.go +++ b/pkg/integration/tests/commit/commit_skip_hooks.go @@ -5,14 +5,24 @@ import ( . "github.com/jesseduffield/lazygit/pkg/integration/components" ) +var fileModHook = `#!/bin/bash + +if [[ -f test-wip-commit-prefix ]]; then + echo "Modified text" > test-wip-commit-prefix +fi +` + var CommitSkipHooks = NewIntegrationTest(NewIntegrationTestArgs{ Description: "Commit with skip hook using CommitChangesWithoutHook", ExtraCmdArgs: []string{}, Skip: false, SetupConfig: func(config *config.AppConfig) {}, SetupRepo: func(shell *Shell) { + shell.CreateFile(".git/hooks/pre-commit", fileModHook) + shell.MakeExecutable(".git/hooks/pre-commit") + shell.NewBranch("feature/TEST-002") - shell.CreateFile("test-wip-commit-prefix", "This is foo bar") + shell.CreateFile("test-wip-commit-prefix", "Initial text") }, Run: func(t *TestDriver, keys config.KeybindingConfig) { t.Views().Commits(). @@ -28,6 +38,8 @@ var CommitSkipHooks = NewIntegrationTest(NewIntegrationTestArgs{ Type("foo bar"). Confirm() + t.FileSystem().FileContent("test-wip-commit-prefix", Equals("Initial text")) + t.Views().Commits().Focus() t.Views().Main().Content(Contains("foo bar")) t.Views().Extras().Content(Contains("--no-verify"))