Skip to content

Commit

Permalink
chore: refactor the code
Browse files Browse the repository at this point in the history
  • Loading branch information
mrexox committed Aug 29, 2023
1 parent b9fa7e8 commit c420d44
Show file tree
Hide file tree
Showing 13 changed files with 88 additions and 89 deletions.
21 changes: 10 additions & 11 deletions internal/lefthook/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"time"

"github.com/evilmartians/lefthook/internal/config"
"github.com/evilmartians/lefthook/internal/lefthook/runner"
"github.com/evilmartians/lefthook/internal/lefthook/run"
"github.com/evilmartians/lefthook/internal/log"
"github.com/evilmartians/lefthook/internal/version"
)
Expand Down Expand Up @@ -105,11 +105,10 @@ Run 'lefthook install' manually.`,
}

startTime := time.Now()
resultChan := make(chan runner.Result, len(hook.Commands)+len(hook.Scripts))
resultChan := make(chan run.Result, len(hook.Commands)+len(hook.Scripts))

run := runner.NewRunner(
runner.Opts{
Fs: l.Fs,
runner := run.NewRunner(
run.Opts{
Repo: l.repo,
Hook: hook,
HookName: hookName,
Expand Down Expand Up @@ -140,11 +139,11 @@ Run 'lefthook install' manually.`,
}

go func() {
run.RunAll(sourceDirs)
runner.RunAll(sourceDirs)
close(resultChan)
}()

var results []runner.Result
var results []run.Result
for res := range resultChan {
results = append(results, res)
}
Expand All @@ -154,7 +153,7 @@ Run 'lefthook install' manually.`,
}

for _, result := range results {
if result.Status == runner.StatusErr {
if result.Status == run.StatusErr {
return errors.New("") // No error should be printed
}
}
Expand All @@ -164,7 +163,7 @@ Run 'lefthook install' manually.`,

func printSummary(
duration time.Duration,
results []runner.Result,
results []run.Result,
logSettings log.SkipSettings,
) {
if len(results) == 0 {
Expand All @@ -180,7 +179,7 @@ func printSummary(

if !logSettings.SkipSuccess() {
for _, result := range results {
if result.Status != runner.StatusOk {
if result.Status != run.StatusOk {
continue
}

Expand All @@ -190,7 +189,7 @@ func printSummary(

if !logSettings.SkipFailure() {
for _, result := range results {
if result.Status != runner.StatusErr {
if result.Status != run.StatusErr {
continue
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//go:build !windows
// +build !windows

package runner
package exec

import (
"fmt"
Expand All @@ -19,9 +19,9 @@ import (

type CommandExecutor struct{}

func (e CommandExecutor) Execute(opts ExecuteOptions, out io.Writer) error {
func (e CommandExecutor) Execute(opts Options, out io.Writer) error {
in := os.Stdin
if opts.interactive && !isatty.IsTerminal(os.Stdin.Fd()) {
if opts.Interactive && !isatty.IsTerminal(os.Stdin.Fd()) {
tty, err := os.Open("/dev/tty")
if err == nil {
defer tty.Close()
Expand All @@ -31,9 +31,9 @@ func (e CommandExecutor) Execute(opts ExecuteOptions, out io.Writer) error {
}
}

root, _ := filepath.Abs(opts.root)
envs := make([]string, len(opts.env))
for name, value := range opts.env {
root, _ := filepath.Abs(opts.Root)
envs := make([]string, len(opts.Env))
for name, value := range opts.Env {
envs = append(
envs,
fmt.Sprintf("%s=%s", strings.ToUpper(name), os.ExpandEnv(value)),
Expand All @@ -42,8 +42,8 @@ func (e CommandExecutor) Execute(opts ExecuteOptions, out io.Writer) error {

// We can have one command split into separate to fit into shell command max length.
// In this case we execute those commands one by one.
for _, args := range opts.commands {
if err := e.executeOne(args, root, envs, opts.interactive, in, out); err != nil {
for _, args := range opts.Commands {
if err := e.executeOne(args, root, envs, opts.Interactive, in, out); err != nil {
return err
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package runner
package exec

import (
"fmt"
Expand All @@ -12,18 +12,18 @@ import (

type CommandExecutor struct{}

func (e CommandExecutor) Execute(opts ExecuteOptions, out io.Writer) error {
root, _ := filepath.Abs(opts.root)
envs := make([]string, len(opts.env))
for name, value := range opts.env {
func (e CommandExecutor) Execute(opts Options, out io.Writer) error {
root, _ := filepath.Abs(opts.Root)
envs := make([]string, len(opts.Env))
for name, value := range opts.Env {
envs = append(
envs,
fmt.Sprintf("%s=%s", strings.ToUpper(name), os.ExpandEnv(value)),
)
}

for _, args := range opts.commands {
if err := e.executeOne(args, root, envs, opts.interactive, os.Stdin, out); err != nil {
for _, args := range opts.Commands {
if err := e.executeOne(args, root, envs, opts.Interactive, os.Stdin, out); err != nil {
return err
}
}
Expand Down
20 changes: 20 additions & 0 deletions internal/lefthook/run/exec/executor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package exec

import (
"io"
)

// Options contains the data that controls the execution.
type Options struct {
Name, Root, FailText string
Commands [][]string
Env map[string]string
Interactive bool
}

// Executor provides an interface for command execution.
// It is used here for testing purpose mostly.
type Executor interface {
Execute(opts Options, out io.Writer) error
RawExecute(command []string, out io.Writer) error
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package runner
package run

import (
"regexp"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package runner
package run

import (
"fmt"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package runner
package run

import (
"errors"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package runner
package run

import (
"fmt"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package runner
package run

import (
"errors"
Expand Down Expand Up @@ -26,7 +26,7 @@ func (r *Runner) prepareScript(script *config.Script, path string, file os.FileI

// Make sure file is executable
if (file.Mode() & executableMask) == 0 {
if err := r.Fs.Chmod(path, executableFileMode); err != nil {
if err := r.Repo.Fs.Chmod(path, executableFileMode); err != nil {
log.Errorf("Couldn't change file mode to make file executable: %s", err)
r.fail(file.Name(), nil)
return nil, errors.New("system error")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package runner
package run

const (
StatusOk status = iota
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package runner
package run

import (
"bytes"
Expand All @@ -19,6 +19,7 @@ import (
"github.com/evilmartians/lefthook/internal/config"
"github.com/evilmartians/lefthook/internal/git"

Check failure on line 20 in internal/lefthook/run/runner.go

View workflow job for this annotation

GitHub Actions / golangci-lint

File is not `gofumpt`-ed (gofumpt)
"github.com/evilmartians/lefthook/internal/log"

Check failure on line 21 in internal/lefthook/run/runner.go

View workflow job for this annotation

GitHub Actions / golangci-lint

File is not `gci`-ed with --skip-generated -s standard -s default (gci)
"github.com/evilmartians/lefthook/internal/lefthook/run/exec"

Check failure on line 22 in internal/lefthook/run/runner.go

View workflow job for this annotation

GitHub Actions / golangci-lint

File is not `gci`-ed with --skip-generated -s standard -s default (gci)
)

type status int8
Expand All @@ -31,7 +32,6 @@ const (
var surroundingQuotesRegexp = regexp.MustCompile(`^'(.*)'$`)

type Opts struct {
Fs afero.Fs
Repo *git.Repository
Hook *config.Hook
HookName string
Expand All @@ -50,13 +50,13 @@ type Runner struct {

partiallyStagedFiles []string
failed atomic.Bool
executor Executor
executor exec.Executor
}

func NewRunner(opts Opts) *Runner {
return &Runner{
Opts: opts,
executor: CommandExecutor{},
executor: exec.CommandExecutor{},
}
}

Expand Down Expand Up @@ -112,11 +112,11 @@ func (r *Runner) runLFSHook() error {
lfsRequiredFile := filepath.Join(r.Repo.RootPath, git.LFSRequiredFile)
lfsConfigFile := filepath.Join(r.Repo.RootPath, git.LFSConfigFile)

requiredExists, err := afero.Exists(r.Fs, lfsRequiredFile)
requiredExists, err := afero.Exists(r.Repo.Fs, lfsRequiredFile)
if err != nil {
return err
}
configExists, err := afero.Exists(r.Fs, lfsConfigFile)
configExists, err := afero.Exists(r.Repo.Fs, lfsConfigFile)
if err != nil {
return err
}
Expand Down Expand Up @@ -223,7 +223,7 @@ func (r *Runner) postHook() {
}

func (r *Runner) runScripts(dir string) {
files, err := afero.ReadDir(r.Fs, dir) // ReadDir already sorts files by .Name()
files, err := afero.ReadDir(r.Repo.Fs, dir) // ReadDir already sorts files by .Name()
if err != nil || len(files) == 0 {
return
}
Expand Down Expand Up @@ -288,13 +288,13 @@ func (r *Runner) runScript(script *config.Script, path string, file os.FileInfo)
defer log.StartSpinner()
}

finished := r.run(ExecuteOptions{
name: file.Name(),
root: r.Repo.RootPath,
commands: [][]string{args},
failText: script.FailText,
interactive: script.Interactive && !r.DisableTTY,
env: script.Env,
finished := r.run(exec.Options{
Name: file.Name(),
Root: r.Repo.RootPath,
Commands: [][]string{args},
FailText: script.FailText,
Interactive: script.Interactive && !r.DisableTTY,
Env: script.Env,
}, r.Hook.Follow)

if finished && config.HookUsesStagedFiles(r.HookName) && script.StageFixed {
Expand Down Expand Up @@ -367,13 +367,13 @@ func (r *Runner) runCommand(name string, command *config.Command) {
defer log.StartSpinner()
}

finished := r.run(ExecuteOptions{
name: name,
root: filepath.Join(r.Repo.RootPath, command.Root),
commands: run.commands,
failText: command.FailText,
interactive: command.Interactive && !r.DisableTTY,
env: command.Env,
finished := r.run(exec.Options{
Name: name,
Root: filepath.Join(r.Repo.RootPath, command.Root),
Commands: run.commands,
FailText: command.FailText,
Interactive: command.Interactive && !r.DisableTTY,
Env: command.Env,
}, r.Hook.Follow)

if finished && config.HookUsesStagedFiles(r.HookName) && command.StageFixed {
Expand Down Expand Up @@ -406,12 +406,12 @@ func (r *Runner) addStagedFiles(files []string) {
}
}

func (r *Runner) run(opts ExecuteOptions, follow bool) bool {
log.SetName(opts.name)
defer log.UnsetName(opts.name)
func (r *Runner) run(opts exec.Options, follow bool) bool {
log.SetName(opts.Name)
defer log.UnsetName(opts.Name)

if (follow || opts.interactive) && !r.SkipSettings.SkipExecution() {
r.logExecute(opts.name, nil, nil)
if (follow || opts.Interactive) && !r.SkipSettings.SkipExecution() {
r.logExecute(opts.Name, nil, nil)

var out io.Writer
if r.SkipSettings.SkipExecutionOutput() {
Expand All @@ -422,9 +422,9 @@ func (r *Runner) run(opts ExecuteOptions, follow bool) bool {

err := r.executor.Execute(opts, out)
if err != nil {
r.fail(opts.name, errors.New(opts.failText))
r.fail(opts.Name, errors.New(opts.FailText))
} else {
r.success(opts.name)
r.success(opts.Name)
}

return err == nil
Expand All @@ -434,12 +434,12 @@ func (r *Runner) run(opts ExecuteOptions, follow bool) bool {
err := r.executor.Execute(opts, out)

if err != nil {
r.fail(opts.name, errors.New(opts.failText))
r.fail(opts.Name, errors.New(opts.FailText))
} else {
r.success(opts.name)
r.success(opts.Name)
}

r.logExecute(opts.name, err, out)
r.logExecute(opts.Name, err, out)

return err == nil
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package runner
package run

import (
"errors"
Expand All @@ -14,15 +14,16 @@ import (

"github.com/evilmartians/lefthook/internal/config"
"github.com/evilmartians/lefthook/internal/git"
"github.com/evilmartians/lefthook/internal/lefthook/run/exec"
)

type TestExecutor struct{}

func (e TestExecutor) Execute(opts ExecuteOptions, _out io.Writer) (err error) {
if opts.commands[0][0] == "success" {
func (e TestExecutor) Execute(opts exec.Options, _out io.Writer) (err error) {
if opts.Commands[0][0] == "success" {
err = nil
} else {
err = errors.New(opts.commands[0][0])
err = errors.New(opts.Commands[0][0])
}

return
Expand Down Expand Up @@ -728,7 +729,6 @@ func TestRunAll(t *testing.T) {
executor := TestExecutor{}
runner := &Runner{
Opts: Opts{
Fs: fs,
Repo: repo,
Hook: tt.hook,
HookName: tt.hookName,
Expand Down
Loading

0 comments on commit c420d44

Please sign in to comment.