Commit without pre-commit hooks is independent on prefix

Add verify flag

Add and update integration tests

Rename verify to forceSkipHooks

Adapt CommitSkipHooks integration test to actually use a hook

Remove forceSkipHooks param from OnConfirm et al

Simplify tests

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, forceSkipHooks 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(forceSkipHooks || (skipHookPrefix != "" && strings.HasPrefix(summary, skipHookPrefix)), "--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, forceSkipHooks bool) oscommands.ICmdObj {
 	return self.cmd.New(NewGitCmd("commit").
+		ArgIf(forceSkipHooks, "--no-verify").
 		Arg("--edit").
 		Arg("--file="+tmpMessageFile).
 		ArgIf(self.signoffFlag() != "", self.signoffFlag()).

pkg/commands/git_commands/commit_test.go

@@ -53,6 +53,7 @@ func TestCommitCommitCmdObj(t *testing.T) {
 	type scenario struct {
 		testName             string
 		summary              string
+		forceSkipHooks       bool
 		description          string
 		configSignoff        bool
 		configSkipHookPrefix string
@@ -63,20 +64,39 @@ func TestCommitCommitCmdObj(t *testing.T) {
 		{
 			testName:             "Commit",
 			summary:              "test",
+			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",
+			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",
+			forceSkipHooks:       false,
 			description:          "line2",
 			configSignoff:        false,
 			configSkipHookPrefix: "",
@@ -85,6 +105,7 @@ func TestCommitCommitCmdObj(t *testing.T) {
 		{
 			testName:             "Commit with signoff",
 			summary:              "test",
+			forceSkipHooks:       false,
 			configSignoff:        true,
 			configSkipHookPrefix: "",
 			expectedArgs:         []string{"commit", "--signoff", "-m", "test"},
@@ -92,6 +113,7 @@ func TestCommitCommitCmdObj(t *testing.T) {
 		{
 			testName:             "Commit with signoff and no-verify",
 			summary:              "WIP: test",
+			forceSkipHooks:       true,
 			configSignoff:        true,
 			configSkipHookPrefix: "WIP",
 			expectedArgs:         []string{"commit", "--no-verify", "--signoff", "-m", "WIP: test"},
@@ -107,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).Run())
+			assert.NoError(t, instance.CommitCmdObj(s.summary, s.description, s.forceSkipHooks).Run())
 			runner.CheckForMissingCalls()
 		})
 	}

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
 	}
 

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
 		}

pkg/gui/controllers/helpers/working_tree_helper.go

@@ -86,7 +86,7 @@ func (self *WorkingTreeHelper) OpenMergeTool() error {
 	return nil
 }
 
-func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage string) error {
+func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage string, forceSkipHooks bool) error {
 	return self.WithEnsureCommittableFiles(func() error {
 		self.commitsHelper.OpenCommitMessagePanel(
 			&OpenCommitMessagePanelOpts{
@@ -95,8 +95,12 @@ func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage strin
 				SummaryTitle:     self.c.Tr.CommitSummaryTitle,
 				DescriptionTitle: self.c.Tr.CommitDescriptionTitle,
 				PreserveMessage:  true,
-				OnConfirm:        self.handleCommit,
-				OnSwitchToEditor: self.switchFromCommitMessagePanelToEditor,
+				OnConfirm: func(summary string, description string) error {
+					return self.handleCommit(summary, description, forceSkipHooks)
+				},
+				OnSwitchToEditor: func(filepath string) error {
+					return self.switchFromCommitMessagePanelToEditor(filepath, forceSkipHooks)
+				},
 			},
 		)
 
@@ -104,8 +108,8 @@ 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, 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()
@@ -113,7 +117,7 @@ func (self *WorkingTreeHelper) handleCommit(summary string, description string)
 	})
 }
 
-func (self *WorkingTreeHelper) switchFromCommitMessagePanelToEditor(filepath string) 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
@@ -124,7 +128,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, forceSkipHooks),
 	)
 }
 
@@ -140,12 +144,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 {
@@ -173,7 +172,7 @@ func (self *WorkingTreeHelper) HandleCommitPress() error {
 		}
 	}
 
-	return self.HandleCommitPressWithMessage(message)
+	return self.HandleCommitPressWithMessage(message, false)
 }
 
 func (self *WorkingTreeHelper) WithEnsureCommittableFiles(handler func() error) error {

pkg/integration/tests/commit/commit_skip_hooks.go

@@ -0,0 +1,44 @@
+package commit
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var blockingHook = `#!/bin/bash
+
+# For this test all we need is a hook that always fails
+exit 1
+`
+
+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", blockingHook)
+		shell.MakeExecutable(".git/hooks/pre-commit")
+
+		shell.CreateFile("file.txt", "content")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		checkBlockingHook(t, keys)
+
+		t.Views().Files().
+			IsFocused().
+			PressPrimaryAction().
+			Lines(
+				Equals("A  file.txt"),
+			).
+			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"))
+	},
+})

pkg/integration/tests/commit/commit_switch_to_editor_skip_hooks.go

@@ -0,0 +1,64 @@
+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(".git/hooks/pre-commit", blockingHook)
+		shell.MakeExecutable(".git/hooks/pre-commit")
+		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()
+
+		checkBlockingHook(t, keys)
+
+		t.Views().Files().
+			IsFocused().
+			Lines(
+				Equals("▼ /").IsSelected(),
+				Equals("  ?? file1"),
+				Equals("  ?? file2"),
+			).
+			SelectNextItem().
+			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`))
+
+		// 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(""))
+	},
+})

pkg/integration/tests/commit/commit_wip_with_prefix.go

@@ -13,6 +13,9 @@ var CommitWipWithPrefix = NewIntegrationTest(NewIntegrationTestArgs{
 		cfg.GetUserConfig().Git.CommitPrefixes = map[string][]config.CommitPrefixConfig{"repo": {{Pattern: "^\\w+\\/(\\w+-\\w+).*", Replace: "[$1]: "}}}
 	},
 	SetupRepo: func(shell *Shell) {
+		shell.CreateFile(".git/hooks/pre-commit", blockingHook)
+		shell.MakeExecutable(".git/hooks/pre-commit")
+
 		shell.NewBranch("feature/TEST-002")
 		shell.CreateFile("test-wip-commit-prefix", "This is foo bar")
 	},
@@ -20,6 +23,8 @@ var CommitWipWithPrefix = NewIntegrationTest(NewIntegrationTestArgs{
 		t.Views().Commits().
 			IsEmpty()
 
+		checkBlockingHook(t, keys)
+
 		t.Views().Files().
 			IsFocused().
 			PressPrimaryAction().
@@ -27,31 +32,30 @@ 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"))
 	},
 })

pkg/integration/tests/commit/fail_hooks_then_commit_no_hooks.go

@@ -0,0 +1,46 @@
+package commit
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+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", blockingHook)
+		shell.MakeExecutable(".git/hooks/pre-commit")
+
+		shell.CreateFileAndAdd("one", "one")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Files().
+			IsFocused().
+			Lines(
+				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"))
+	},
+})

pkg/integration/tests/commit/shared.go

@@ -72,3 +72,38 @@ func checkCommitContainsChange(t *TestDriver, commitSubject string, change strin
 	t.Views().Main().
 		Content(Contains(change))
 }
+
+func checkBlockingHook(t *TestDriver, keys config.KeybindingConfig) {
+	// Shared function for tests using the blockingHook pre-commit hook for testing hook skipping
+	// Stage first file
+	t.Views().Files().
+		IsFocused().
+		PressPrimaryAction().
+		Press(keys.Files.CommitChanges)
+
+	// Try to commit with hook
+	t.ExpectPopup().CommitMessagePanel().
+		Title(Equals("Commit summary")).
+		Type("Commit should fail").
+		Confirm()
+
+	t.ExpectPopup().Alert().
+		Title(Equals("Error")).
+		Content(Contains("Git command failed.")).
+		Confirm()
+
+	// Clear the message
+	t.Views().Files().
+		IsFocused().
+		Press(keys.Files.CommitChanges)
+
+	t.ExpectPopup().CommitMessagePanel().
+		Title(Equals("Commit summary")).
+		Clear().
+		Cancel()
+
+	// Unstage the file
+	t.Views().Files().
+		IsFocused().
+		PressPrimaryAction()
+}

pkg/integration/tests/commit/staged_without_hooks.go

@@ -11,6 +11,9 @@ var StagedWithoutHooks = NewIntegrationTest(NewIntegrationTestArgs{
 	Skip:         false,
 	SetupConfig:  func(config *config.AppConfig) {},
 	SetupRepo: func(shell *Shell) {
+		shell.CreateFile(".git/hooks/pre-commit", blockingHook)
+		shell.MakeExecutable(".git/hooks/pre-commit")
+
 		shell.
 			CreateFile("myfile", "myfile content\nwith a second line").
 			CreateFile("myfile2", "myfile2 content")
@@ -19,6 +22,8 @@ var StagedWithoutHooks = NewIntegrationTest(NewIntegrationTestArgs{
 		t.Views().Commits().
 			IsEmpty()
 
+		checkBlockingHook(t, keys)
+
 		// stage the file
 		t.Views().Files().
 			IsFocused().
@@ -50,12 +55,12 @@ 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().Commits().
 			Lines(
-				Contains("WIP" + commitMessage),
+				Contains(commitMessage),
 			)
 
 		t.Views().StagingSecondary().

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,