Commit without pre-commit hooks action is independent on prefix (#4374)

As discussed in #1984, this PR modifies the code such that the `w`
command always commits without pre-commit hooks. The `skipHookPrefix` is
still considered when starting the commit messaged with the prefix in
the regular commit command using `c`.

Skipping pre-commits also works when switching to the editor and adding
a co author from the the `w` command

Author: stefanhaller

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,13 +105,15 @@ func TestCommitCommitCmdObj(t *testing.T) {
 		{
 			testName:             "Commit with signoff",
 			summary:              "test",
+			forceSkipHooks:       false,
 			configSignoff:        true,
 			configSkipHookPrefix: "",
 			expectedArgs:         []string{"commit", "--signoff", "-m", "test"},
 		},
 		{
 			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,25 +95,29 @@ 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)
+				},
 			},
 		)
 
 		return nil
 	})
 }
 
-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()
 		return nil
 	})
 }
 
-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/i18n/english.go

@@ -449,7 +449,6 @@ type TranslationSet struct {
 	ExecuteShellCommandTooltip            string
 	ShellCommand                          string
 	CommitChangesWithoutHook              string
-	SkipHookPrefixNotConfigured           string
 	ResetTo                               string
 	ResetSoftTooltip                      string
 	ResetMixedTooltip                     string
@@ -1500,7 +1499,6 @@ func EnglishTranslationSet() *TranslationSet {
 		ExecuteShellCommandTooltip:           "Bring up a prompt where you can enter a shell command to execute.",
 		ShellCommand:                         "Shell command:",
 		CommitChangesWithoutHook:             "Commit changes without pre-commit hook",
-		SkipHookPrefixNotConfigured:          "You have not configured a commit message prefix for skipping hooks. Set `git.skipHookPrefix = 'WIP'` in your config",
 		ResetTo:                              `Reset to`,
 		PressEnterToReturn:                   "Press enter to return to lazygit",
 		ViewStashOptions:                     "View stash options",

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,45 +13,49 @@ 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")
 	},
 	Run: func(t *TestDriver, keys config.KeybindingConfig) {
 		t.Views().Commits().
 			IsEmpty()
 
+		checkBlockingHook(t, keys)
+
 		t.Views().Files().
 			IsFocused().
 			PressPrimaryAction().
 			Press(keys.Files.CommitChangesWithoutHook)
 
 		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"))
 	},
 })