Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions pkg/commands/git_commands/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()).
Expand Down
26 changes: 24 additions & 2 deletions pkg/commands/git_commands/commit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ func TestCommitCommitCmdObj(t *testing.T) {
type scenario struct {
testName string
summary string
forceSkipHooks bool
description string
configSignoff bool
configSkipHookPrefix string
Expand All @@ -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: "",
Expand All @@ -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"},
Expand All @@ -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()
})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/git_commands/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a regression; previously it was possible to move a custom patch into a new commit with a WIP prefix and have this skip the hooks; now this is no longer possible. Keeping the prefix logic in CommitCmdObj would fix this.

Note that there are other problems while working with custom patches currently doesn't work well with WIP prefixes; see #3532. But those are unrelated to your PR and can be fixed separately.

return err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/controllers/helpers/merge_and_rebase_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another regression: when setting the squashMergeMessage config to start with the skipHooksPrefix, then we would previously skip the hooks when committing the result of a squash merge. You might argue whether this was intentional, but I do think it makes sense. Again, keeping the prefix logic in CommitCmdObj fixes this.

if err != nil {
return err
}
Expand Down
27 changes: 13 additions & 14 deletions pkg/gui/controllers/helpers/working_tree_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
Expand All @@ -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),
)
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions pkg/i18n/english.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,6 @@ type TranslationSet struct {
ExecuteShellCommandTooltip string
ShellCommand string
CommitChangesWithoutHook string
SkipHookPrefixNotConfigured string
ResetTo string
ResetSoftTooltip string
ResetMixedTooltip string
Expand Down Expand Up @@ -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",
Expand Down
44 changes: 44 additions & 0 deletions pkg/integration/tests/commit/commit_skip_hooks.go
Original file line number Diff line number Diff line change
@@ -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"))
},
})
64 changes: 64 additions & 0 deletions pkg/integration/tests/commit/commit_switch_to_editor_skip_hooks.go
Original file line number Diff line number Diff line change
@@ -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(""))
},
})
18 changes: 11 additions & 7 deletions pkg/integration/tests/commit/commit_wip_with_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
},
})
Loading