From 80c5dd321e02f199cf726691416696ec35941dd7 Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Tue, 14 Mar 2023 10:53:43 +0300 Subject: [PATCH 1/3] feat: add stage_fixed option --- internal/config/command.go | 1 + internal/config/script.go | 1 + internal/git/repository.go | 12 ++++++ internal/lefthook/runner/executor.go | 8 ++-- internal/lefthook/runner/prepare_command.go | 19 ++++++--- internal/lefthook/runner/runner.go | 43 ++++++++++++++++++--- 6 files changed, 69 insertions(+), 15 deletions(-) diff --git a/internal/config/command.go b/internal/config/command.go index 78feb709..9abe0919 100644 --- a/internal/config/command.go +++ b/internal/config/command.go @@ -25,6 +25,7 @@ type Command struct { FailText string `mapstructure:"fail_text"` Interactive bool `mapstructure:"interactive"` + StageFixed bool `mapstructure:"stage_fixed"` } func (c Command) Validate() error { diff --git a/internal/config/script.go b/internal/config/script.go index 587a076d..d2041fb6 100644 --- a/internal/config/script.go +++ b/internal/config/script.go @@ -18,6 +18,7 @@ type Script struct { FailText string `mapstructure:"fail_text"` Interactive bool `mapstructure:"interactive"` + StageFixed bool `mapstructure:"stage_fixed"` } func (s Script) DoSkip(gitState git.State) bool { diff --git a/internal/git/repository.go b/internal/git/repository.go index 9c233771..224ce91c 100644 --- a/internal/git/repository.go +++ b/internal/git/repository.go @@ -272,6 +272,18 @@ func (r *Repository) DropUnstagedStash() error { return nil } +func (r *Repository) AddFiles(files []string) error { + if len(files) == 0 { + return nil + } + + _, err := r.Git.CmdArgs( + append([]string{"git", "add"}, files...)..., + ) + + return err +} + // FilesByCommand accepts git command and returns its result as a list of filepaths. func (r *Repository) FilesByCommand(command string) ([]string, error) { lines, err := r.Git.CmdLines(command) diff --git a/internal/lefthook/runner/executor.go b/internal/lefthook/runner/executor.go index 40bf1ccc..9a228d00 100644 --- a/internal/lefthook/runner/executor.go +++ b/internal/lefthook/runner/executor.go @@ -6,10 +6,10 @@ import ( // ExecutorOptions contains the options that control the execution. type ExecuteOptions struct { - name, root, failText string - args []string - interactive bool - env map[string]string + name, root, failText string + args []string + env map[string]string + interactive, stageFixed bool } // Executor provides an interface for command execution. diff --git a/internal/lefthook/runner/prepare_command.go b/internal/lefthook/runner/prepare_command.go index f52d1c3f..e33ce459 100644 --- a/internal/lefthook/runner/prepare_command.go +++ b/internal/lefthook/runner/prepare_command.go @@ -11,7 +11,12 @@ import ( "github.com/evilmartians/lefthook/internal/log" ) -func (r *Runner) prepareCommand(name string, command *config.Command) ([]string, error) { +type commandArgs struct { + all []string + files []string +} + +func (r *Runner) prepareCommand(name string, command *config.Command) (*commandArgs, error) { if command.Skip != nil && command.DoSkip(r.Repo.State()) { return nil, errors.New("settings") } @@ -34,14 +39,14 @@ func (r *Runner) prepareCommand(name string, command *config.Command) ([]string, log.Error(err) return nil, errors.New("error") } - if len(args) == 0 { + if args == nil || len(args.all) == 0 { return nil, errors.New("no files for inspection") } return args, nil } -func (r *Runner) buildCommandArgs(command *config.Command) ([]string, error) { +func (r *Runner) buildCommandArgs(command *config.Command) (*commandArgs, error) { filesCommand := r.Hook.Files if command.Files != "" { filesCommand = command.Files @@ -56,6 +61,7 @@ func (r *Runner) buildCommandArgs(command *config.Command) ([]string, error) { }, } + filteredFiles := []string{} runString := command.Run for filesType, filesFn := range filesTypeToFn { // Checking substitutions and skipping execution if it is empty. @@ -76,7 +82,7 @@ func (r *Runner) buildCommandArgs(command *config.Command) ([]string, error) { if len(filesPrepared) == 0 { return nil, nil } - + filteredFiles = append(filteredFiles, filesPrepared...) runString = replaceQuoted(runString, filesType, filesPrepared) } } @@ -88,7 +94,10 @@ func (r *Runner) buildCommandArgs(command *config.Command) ([]string, error) { log.Debug("[lefthook] executing: ", runString) - return strings.Split(runString, " "), nil + return &commandArgs{ + files: filteredFiles, + all: strings.Split(runString, " "), + }, nil } func prepareFiles(command *config.Command, files []string) []string { diff --git a/internal/lefthook/runner/runner.go b/internal/lefthook/runner/runner.go index bfa57ce5..1c3423d7 100644 --- a/internal/lefthook/runner/runner.go +++ b/internal/lefthook/runner/runner.go @@ -279,14 +279,27 @@ func (r *Runner) runScript(script *config.Script, path string, file os.FileInfo) defer log.StartSpinner() } - r.run(ExecuteOptions{ + finished := r.run(ExecuteOptions{ name: file.Name(), root: r.Repo.RootPath, args: args, failText: script.FailText, interactive: script.Interactive && !r.DisableTTY, env: script.Env, + stageFixed: script.StageFixed, }, r.Hook.Follow) + + if finished && config.HookUsesStagedFiles(r.HookName) && script.StageFixed { + files, err := r.Repo.StagedFiles() + if err != nil { + log.Warn("Couldn't stage fixed files:", err) + return + } + + if err := r.Repo.AddFiles(files); err != nil { + log.Warn("Couldn't stage fixed files:", err) + } + } } func (r *Runner) runCommands() { @@ -346,17 +359,33 @@ func (r *Runner) runCommand(name string, command *config.Command) { defer log.StartSpinner() } - r.run(ExecuteOptions{ + finished := r.run(ExecuteOptions{ name: name, root: filepath.Join(r.Repo.RootPath, command.Root), - args: args, + args: args.all, failText: command.FailText, interactive: command.Interactive && !r.DisableTTY, env: command.Env, }, r.Hook.Follow) + + if finished && config.HookUsesStagedFiles(r.HookName) && command.StageFixed { + files := args.files + if len(files) == 0 { + stagedFiles, err := r.Repo.StagedFiles() + if err != nil { + log.Warn("Couldn't stage fixed files:", err) + return + } + files = prepareFiles(command, stagedFiles) + } + + if err := r.Repo.AddFiles(files); err != nil { + log.Warn("Couldn't stage fixed files:", err) + } + } } -func (r *Runner) run(opts ExecuteOptions, follow bool) { +func (r *Runner) run(opts ExecuteOptions, follow bool) bool { log.SetName(opts.name) defer log.UnsetName(opts.name) @@ -368,7 +397,7 @@ func (r *Runner) run(opts ExecuteOptions, follow bool) { } else { r.success(opts.name) } - return + return err == nil } out := bytes.NewBuffer(make([]byte, 0)) @@ -384,7 +413,7 @@ func (r *Runner) run(opts ExecuteOptions, follow bool) { } if err == nil && r.SkipSettings.SkipExecution() { - return + return false } log.Infof("%s\n%s", execName, out) @@ -392,6 +421,8 @@ func (r *Runner) run(opts ExecuteOptions, follow bool) { log.Infof("%s", err) } log.Infof("\n") + + return err == nil } // Returns whether two arrays have at least one similar element. From 84cac4436f3294a4277d58371298b8f35292e019 Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Wed, 15 Mar 2023 10:18:18 +0300 Subject: [PATCH 2/3] chore: add a test --- internal/lefthook/runner/executor.go | 8 +- internal/lefthook/runner/runner.go | 1 - internal/lefthook/runner/runner_test.go | 140 +++++++++++++++++++++++- 3 files changed, 138 insertions(+), 11 deletions(-) diff --git a/internal/lefthook/runner/executor.go b/internal/lefthook/runner/executor.go index 9a228d00..c3eca880 100644 --- a/internal/lefthook/runner/executor.go +++ b/internal/lefthook/runner/executor.go @@ -6,10 +6,10 @@ import ( // ExecutorOptions contains the options that control the execution. type ExecuteOptions struct { - name, root, failText string - args []string - env map[string]string - interactive, stageFixed bool + name, root, failText string + args []string + env map[string]string + interactive bool } // Executor provides an interface for command execution. diff --git a/internal/lefthook/runner/runner.go b/internal/lefthook/runner/runner.go index 1c3423d7..fc915b51 100644 --- a/internal/lefthook/runner/runner.go +++ b/internal/lefthook/runner/runner.go @@ -286,7 +286,6 @@ func (r *Runner) runScript(script *config.Script, path string, file os.FileInfo) failText: script.FailText, interactive: script.Interactive && !r.DisableTTY, env: script.Env, - stageFixed: script.StageFixed, }, r.Hook.Follow) if finished && config.HookUsesStagedFiles(r.HookName) && script.StageFixed { diff --git a/internal/lefthook/runner/runner_test.go b/internal/lefthook/runner/runner_test.go index ffa3e8ce..3a0dd972 100644 --- a/internal/lefthook/runner/runner_test.go +++ b/internal/lefthook/runner/runner_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "path/filepath" + "strings" "testing" "github.com/spf13/afero" @@ -29,24 +30,38 @@ func (e TestExecutor) RawExecute(command []string, out io.Writer) error { return nil } -type GitMock struct{} +type GitMock struct { + commands []string +} + +func (g *GitMock) Cmd(cmd string) (string, error) { + g.commands = append(g.commands, cmd) -func (g GitMock) Cmd(cmd string) (string, error) { return "", nil } -func (g GitMock) CmdArgs(args ...string) (string, error) { +func (g *GitMock) CmdArgs(args ...string) (string, error) { + g.commands = append(g.commands, strings.Join(args, " ")) + return "", nil } -func (g GitMock) CmdLines(cmd string) ([]string, error) { +func (g *GitMock) CmdLines(cmd string) ([]string, error) { + g.commands = append(g.commands, cmd) + return nil, nil } -func (g GitMock) RawCmd(cmd string) (string, error) { +func (g *GitMock) RawCmd(cmd string) (string, error) { + g.commands = append(g.commands, cmd) + return "", nil } +func (g *GitMock) reset() { + g.commands = []string{} +} + func TestRunAll(t *testing.T) { hookName := "pre-commit" @@ -55,9 +70,10 @@ func TestRunAll(t *testing.T) { t.Errorf("unexpected error: %s", err) } + gitExec := &GitMock{} gitPath := filepath.Join(root, ".git") repo := &git.Repository{ - Git: GitMock{}, + Git: gitExec, HooksPath: filepath.Join(gitPath, "hooks"), RootPath: root, GitPath: gitPath, @@ -71,6 +87,7 @@ func TestRunAll(t *testing.T) { existingFiles []string hook *config.Hook success, fail []Result + gitCommands []string }{ { name: "empty hook", @@ -79,6 +96,11 @@ func TestRunAll(t *testing.T) { Scripts: map[string]*config.Script{}, Piped: true, }, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, { name: "with simple command", @@ -91,6 +113,11 @@ func TestRunAll(t *testing.T) { Scripts: map[string]*config.Script{}, }, success: []Result{{Name: "test", Status: StatusOk}}, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, { name: "with simple command in follow mode", @@ -104,6 +131,11 @@ func TestRunAll(t *testing.T) { Scripts: map[string]*config.Script{}, }, success: []Result{{Name: "test", Status: StatusOk}}, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, { name: "with multiple commands ran in parallel", @@ -127,6 +159,11 @@ func TestRunAll(t *testing.T) { {Name: "lint", Status: StatusOk}, }, fail: []Result{{Name: "type-check", Status: StatusErr}}, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, { name: "with exclude tags", @@ -148,6 +185,11 @@ func TestRunAll(t *testing.T) { Scripts: map[string]*config.Script{}, }, success: []Result{{Name: "lint", Status: StatusOk}}, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, { name: "with skip boolean option", @@ -164,6 +206,11 @@ func TestRunAll(t *testing.T) { Scripts: map[string]*config.Script{}, }, success: []Result{{Name: "lint", Status: StatusOk}}, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, { name: "with skip merge", @@ -183,6 +230,11 @@ func TestRunAll(t *testing.T) { Scripts: map[string]*config.Script{}, }, success: []Result{{Name: "lint", Status: StatusOk}}, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, { name: "with global skip merge", @@ -222,6 +274,11 @@ func TestRunAll(t *testing.T) { Scripts: map[string]*config.Script{}, }, success: []Result{{Name: "lint", Status: StatusOk}}, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, { name: "with global skip on ref", @@ -265,6 +322,11 @@ func TestRunAll(t *testing.T) { {Name: "test", Status: StatusOk}, {Name: "lint", Status: StatusOk}, }, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, { name: "with fail test", @@ -278,6 +340,11 @@ func TestRunAll(t *testing.T) { Scripts: map[string]*config.Script{}, }, fail: []Result{{Name: "test", Status: StatusErr, Text: "try 'success'"}}, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, { name: "with simple scripts", @@ -300,6 +367,11 @@ func TestRunAll(t *testing.T) { }, success: []Result{{Name: "script.sh", Status: StatusOk}}, fail: []Result{{Name: "failing.js", Status: StatusErr, Text: "install node"}}, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, { name: "with interactive and parallel", @@ -331,6 +403,51 @@ func TestRunAll(t *testing.T) { }, success: []Result{}, // script.sh and ok are skipped fail: []Result{{Name: "failing.js", Status: StatusErr}, {Name: "fail", Status: StatusErr}}, + gitCommands: []string{ + "git status --short", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, + }, + { + name: "with stage_fixed in true", + sourceDirs: []string{filepath.Join(root, config.DefaultSourceDir)}, + existingFiles: []string{ + filepath.Join(root, config.DefaultSourceDir, hookName, "success.sh"), + filepath.Join(root, config.DefaultSourceDir, hookName, "failing.js"), + }, + hook: &config.Hook{ + Parallel: true, + Commands: map[string]*config.Command{ + "ok": { + Run: "success", + StageFixed: true, + }, + "fail": { + Run: "fail", + StageFixed: true, + }, + }, + Scripts: map[string]*config.Script{ + "success.sh": { + Runner: "success", + StageFixed: true, + }, + "failing.js": { + Runner: "fail", + StageFixed: true, + }, + }, + }, + success: []Result{{Name: "ok", Status: StatusOk}, {Name: "success.sh", Status: StatusOk}}, + fail: []Result{{Name: "fail", Status: StatusErr}, {Name: "failing.js", Status: StatusErr}}, + gitCommands: []string{ + "git status --short", + "git diff --name-only --cached --diff-filter=ACMR", + "git diff --name-only --cached --diff-filter=ACMR", + "git apply -v --whitespace=nowarn --recount --unidiff-zero ", + "git stash list", + }, }, } { fs := afero.NewMemMapFs() @@ -348,6 +465,7 @@ func TestRunAll(t *testing.T) { }, executor: executor, } + gitExec.reset() for _, file := range tt.existingFiles { if err := fs.MkdirAll(filepath.Dir(file), 0o755); err != nil { @@ -384,6 +502,16 @@ func TestRunAll(t *testing.T) { if !resultsMatch(tt.fail, fail) { t.Errorf("fail results are not matching:\n Need: %v\n Was: %v", tt.fail, fail) } + + if len(gitExec.commands) != len(tt.gitCommands) { + t.Errorf("wrong git commands\nExpected: %#v\nWas: %#v", gitExec.commands, tt.gitCommands) + } else { + for i, command := range gitExec.commands { + if tt.gitCommands[i] != command { + t.Errorf("wrong git command #%d\nExpected: %s\nWas: %s", i, command, tt.gitCommands[i]) + } + } + } }) } } From 708875b535939ce225864900e980f931cf22cd6a Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Wed, 15 Mar 2023 11:00:51 +0300 Subject: [PATCH 3/3] docs: add documentation for stage_fixed --- docs/configuration.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/docs/configuration.md b/docs/configuration.md index 458f529e..b9654cfd 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -32,6 +32,7 @@ - [`root`](#root) - [`exclude`](#exclude) - [`fail_text`](#fail_text) + - [`stage_fixed`](#stage_fixed) - [`interactive`](#interactive) - [Script](#script) - [`runner`](#runner) @@ -39,6 +40,7 @@ - [`tags`](#tags) - [`env`](#env) - [`fail_text`](#fail_text) + - [`stage_fixed`](#stage_fixed) - [`interactive`](#interactive) - [Examples](#examples) - [More info](#more-info) @@ -901,6 +903,26 @@ SUMMARY: (done in 0.01 seconds) 🥊 lint: Add node executable to $PATH env ``` +### `stage_fixed` + +**Default: `false`** + +> Used **only for `pre-commit`** hook. Is ignored for other hooks. + +When set to `true` lefthook will automatically call `git add` on files after running the command or script. For a command if [`files`](#files) option was specified, the specified command will be used to retrieve files for `git add`. For scripts and commands without [`files`](#files) option `{staged_files}` template will be used. All filters ([`glob`](#glob), [`exclude`](#exclude)) will be applied if specified. + +**Example** + +```yml +# lefthook.yml + +pre-commit: + commands: + lint: + run: npm run lint --fix {staged_files} + stage_fixed: true +``` + ### `interactive` **Default: `false`**