From be6c7a70b09659da91277cbd32415d38632dbc0c Mon Sep 17 00:00:00 2001 From: Edwin Kofler Date: Thu, 13 Jul 2023 22:55:55 -0700 Subject: [PATCH 1/6] feat: Search for `.lefthook` before `lefthook` config --- internal/config/load.go | 27 ++++++++--- internal/config/load_test.go | 90 ++++++++++++++++++++++++++++++++++-- 2 files changed, 107 insertions(+), 10 deletions(-) diff --git a/internal/config/load.go b/internal/config/load.go index ae9ea4aa..72e9ea1b 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -34,13 +34,8 @@ func (err NotFoundError) Error() string { // Loads configs from the given directory with extensions. func Load(fs afero.Fs, repo *git.Repository) (*Config, error) { - global, err := read(fs, repo.RootPath, "lefthook") + global, err := readOne(fs, repo.RootPath, []string{".lefthook", "lefthook"}) if err != nil { - var notFoundErr viper.ConfigFileNotFoundError - if ok := errors.As(err, ¬FoundErr); ok { - return nil, NotFoundError{err.Error()} - } - return nil, err } @@ -81,9 +76,27 @@ func read(fs afero.Fs, path string, name string) (*viper.Viper, error) { return v, nil } +func readOne(fs afero.Fs, path string, names []string) (*viper.Viper, error) { + for _, name := range names { + v, err := read(fs, path, name) + if err != nil { + var notFoundErr viper.ConfigFileNotFoundError + if ok := errors.As(err, ¬FoundErr); ok { + continue + } else { + return nil, err + } + } + + return v, nil + } + + return nil, NotFoundError{fmt.Sprintf("No config files with names %q could not be found in \"%s\"", names, path)} +} + // mergeAll merges remotes and extends from .lefthook and .lefthook-local. func mergeAll(fs afero.Fs, repo *git.Repository) (*viper.Viper, error) { - extends, err := read(fs, repo.RootPath, "lefthook") + extends, err := readOne(fs, repo.RootPath, []string{".lefthook", "lefthook"}) if err != nil { return nil, err } diff --git a/internal/config/load_test.go b/internal/config/load_test.go index c65ec338..46b5d221 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -23,9 +23,93 @@ func TestLoad(t *testing.T) { name string global, local, remote string remoteConfigPath string - extends map[string]string + otherFiles map[string]string result *Config }{ + { + name: "with global, dot", + otherFiles: map[string]string{ + ".lefthook.yml": ` +pre-commit: + commands: + tests: + run: yarn test +`, + }, + result: &Config{ + SourceDir: DefaultSourceDir, + SourceDirLocal: DefaultSourceDirLocal, + Colors: nil, + Hooks: map[string]*Hook{ + "pre-commit": { + Parallel: false, + Commands: map[string]*Command{ + "tests": { + Run: "yarn test", + }, + }, + }, + }, + }, + }, + { + name: "with global, nodot", + otherFiles: map[string]string{ + "lefthook.yml": ` +pre-commit: + commands: + tests: + run: yarn test +`, + }, + result: &Config{ + SourceDir: DefaultSourceDir, + SourceDirLocal: DefaultSourceDirLocal, + Colors: nil, + Hooks: map[string]*Hook{ + "pre-commit": { + Parallel: false, + Commands: map[string]*Command{ + "tests": { + Run: "yarn test", + }, + }, + }, + }, + }, + }, + { + name: "with global, dot has priority", + otherFiles: map[string]string{ + ".lefthook.yml": ` +pre-commit: + commands: + tests: + run: yarn test1 +`, + "lefthook.yml": ` +pre-commit: + commands: + tests: + run: yarn test2 +`, + }, + result: &Config{ + SourceDir: DefaultSourceDir, + SourceDirLocal: DefaultSourceDirLocal, + Colors: nil, + Hooks: map[string]*Hook{ + "pre-commit": { + Parallel: false, + Commands: map[string]*Command{ + "tests": { + Run: "yarn test1", + }, + }, + }, + }, + }, + }, { name: "simple", global: ` @@ -356,7 +440,7 @@ pre-push: run: echo remote `, remoteConfigPath: filepath.Join(root, ".git", "info", "lefthook-remotes", "lefthook", "examples", "config.yml"), - extends: map[string]string{ + otherFiles: map[string]string{ "global-extend.yml": ` pre-push: scripts: @@ -440,7 +524,7 @@ pre-push: } } - for name, content := range tt.extends { + for name, content := range tt.otherFiles { path := filepath.Join( root, filepath.Join(strings.Split(name, "/")...), From 47c13607195ba823b3144e29240bf2d03cd4441c Mon Sep 17 00:00:00 2001 From: Edwin Kofler Date: Thu, 13 Jul 2023 21:22:06 -0700 Subject: [PATCH 2/6] feat: Generate default of `.lefthook.yml` instead of `lefthook.yml` --- internal/config/load.go | 1 + internal/lefthook/install.go | 4 ++-- internal/lefthook/install_test.go | 3 ++- testdata/install.txt | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/config/load.go b/internal/config/load.go index 72e9ea1b..e0a1a521 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -15,6 +15,7 @@ import ( ) const ( + DefaultDotConfigName = ".lefthook.yml" DefaultConfigName = "lefthook.yml" DefaultSourceDir = ".lefthook" DefaultSourceDirLocal = ".lefthook-local" diff --git a/internal/lefthook/install.go b/internal/lefthook/install.go index 32504ab2..a4685d1b 100644 --- a/internal/lefthook/install.go +++ b/internal/lefthook/install.go @@ -30,7 +30,7 @@ const ( var ( lefthookChecksumRegexp = regexp.MustCompile(`(\w+)\s+(\d+)`) - configGlob = glob.MustCompile("lefthook.{yml,yaml,json,toml}") + configGlob = glob.MustCompile("{.,}lefthook.{yml,yaml,json,toml}") errNoConfig = fmt.Errorf("no lefthook config found") ) @@ -99,7 +99,7 @@ func (l *Lefthook) configExists(path string) bool { } func (l *Lefthook) createConfig(path string) error { - file := filepath.Join(path, config.DefaultConfigName) + file := filepath.Join(path, config.DefaultDotConfigName) err := afero.WriteFile(l.Fs, file, templates.Config(), configFileMode) if err != nil { diff --git a/internal/lefthook/install_test.go b/internal/lefthook/install_test.go index 6fafcba6..3fa9d37b 100644 --- a/internal/lefthook/install_test.go +++ b/internal/lefthook/install_test.go @@ -18,6 +18,7 @@ func TestLefthookInstall(t *testing.T) { t.Errorf("unexpected error: %s", err) } + dotConfigPath := filepath.Join(root, ".lefthook.yml") configPath := filepath.Join(root, "lefthook.yml") hookPath := func(hook string) string { @@ -43,7 +44,7 @@ func TestLefthookInstall(t *testing.T) { }{ { name: "without a config file", - wantExist: []string{configPath}, + wantExist: []string{dotConfigPath}, }, { name: "simple default config", diff --git a/testdata/install.txt b/testdata/install.txt index 250ac54b..754cd265 100644 --- a/testdata/install.txt +++ b/testdata/install.txt @@ -1,4 +1,4 @@ exec git init exec lefthook install -exists lefthook.yml +exists .lefthook.yml ! stderr . From 704b4c74744c45069d193e7e844899fb4c8680d9 Mon Sep 17 00:00:00 2001 From: Edwin Kofler Date: Fri, 14 Jul 2023 00:23:18 -0700 Subject: [PATCH 3/6] feat: Search for `.lefthook-local` before `lefthook-local` config --- cmd/uninstall.go | 2 +- internal/config/load.go | 21 ++++++- internal/config/load_test.go | 108 +++++++++++++++++++++++++++++++++ internal/lefthook/uninstall.go | 2 + 4 files changed, 130 insertions(+), 3 deletions(-) diff --git a/cmd/uninstall.go b/cmd/uninstall.go index e1d67a25..d49c5a6c 100644 --- a/cmd/uninstall.go +++ b/cmd/uninstall.go @@ -29,7 +29,7 @@ func newUninstallCmd(opts *lefthook.Options) *cobra.Command { uninstallCmd.Flags().BoolVarP( &args.RemoveConfig, "remove-configs", "c", false, - "remove lefthook.yml and lefthook-local.yml", + "remove lefthook.yml, lefthook-local.yml, .lefthook.yml, and .lefthook-local.yml", ) return &uninstallCmd diff --git a/internal/config/load.go b/internal/config/load.go index e0a1a521..e62e9efd 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -95,7 +95,8 @@ func readOne(fs afero.Fs, path string, names []string) (*viper.Viper, error) { return nil, NotFoundError{fmt.Sprintf("No config files with names %q could not be found in \"%s\"", names, path)} } -// mergeAll merges remotes and extends from .lefthook and .lefthook-local. +// mergeAll merges (.lefthook or lefthook) and (extended config) and (remote) +// and (.lefthook-local or .lefthook-local) configs. func mergeAll(fs afero.Fs, repo *git.Repository) (*viper.Viper, error) { extends, err := readOne(fs, repo.RootPath, []string{".lefthook", "lefthook"}) if err != nil { @@ -110,7 +111,7 @@ func mergeAll(fs afero.Fs, repo *git.Repository) (*viper.Viper, error) { return nil, err } - if err := merge("lefthook-local", "", extends); err == nil { + if err := mergeOne([]string{".lefthook-local", "lefthook-local"}, "", extends); err == nil { if err = extend(extends, repo.RootPath); err != nil { return nil, err } @@ -187,6 +188,22 @@ func merge(name, path string, v *viper.Viper) error { return nil } +func mergeOne(names []string, path string, v *viper.Viper) error { + for _, name := range names { + err := merge(name, path, v) + if err == nil { + break + } else { + var notFoundErr viper.ConfigFileNotFoundError + if ok := errors.As(err, ¬FoundErr); !ok { + return err + } + } + } + + return nil +} + func unmarshalConfigs(base, extra *viper.Viper, c *Config) error { c.Hooks = make(map[string]*Hook) diff --git a/internal/config/load_test.go b/internal/config/load_test.go index 46b5d221..aead0dfc 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -228,6 +228,114 @@ pre-push: }, }, }, + { + name: "with overrides, dot", + otherFiles: map[string]string{ + ".lefthook.yml": ` +pre-push: + scripts: + "global-extend": + runner: bash +`, + ".lefthook-local.yml": ` +pre-push: + scripts: + "local-extend": + runner: bash +`, + }, + result: &Config{ + SourceDir: DefaultSourceDir, + SourceDirLocal: DefaultSourceDirLocal, + Colors: nil, + Hooks: map[string]*Hook{ + "pre-push": { + Scripts: map[string]*Script{ + "global-extend": { + Runner: "bash", + }, + "local-extend": { + Runner: "bash", + }, + }, + }, + }, + }, + }, + { + name: "with overrides, dot, nodot", + otherFiles: map[string]string{ + "lefthook.yml": ` +pre-push: + scripts: + "global-extend": + runner: bash +`, + ".lefthook-local.yml": ` +pre-push: + scripts: + "local-extend": + runner: bash +`, + }, + result: &Config{ + SourceDir: DefaultSourceDir, + SourceDirLocal: DefaultSourceDirLocal, + Colors: nil, + Hooks: map[string]*Hook{ + "pre-push": { + Scripts: map[string]*Script{ + "global-extend": { + Runner: "bash", + }, + "local-extend": { + Runner: "bash", + }, + }, + }, + }, + }, + }, + { + name: "with overrides, dot has priority", + otherFiles: map[string]string{ + "lefthook.yml": ` +pre-push: + scripts: + "global-extend": + runner: bash +`, + ".lefthook-local.yml": ` +pre-push: + scripts: + "local-extend": + runner: bash1 +`, + "lefthook-local.yml": ` +pre-push: + scripts: + "local-extend": + runner: bash2 +`, + }, + result: &Config{ + SourceDir: DefaultSourceDir, + SourceDirLocal: DefaultSourceDirLocal, + Colors: nil, + Hooks: map[string]*Hook{ + "pre-push": { + Scripts: map[string]*Script{ + "global-extend": { + Runner: "bash", + }, + "local-extend": { + Runner: "bash1", + }, + }, + }, + }, + }, + }, { name: "with extra hooks", global: ` diff --git a/internal/lefthook/uninstall.go b/internal/lefthook/uninstall.go index 21e8aa43..b90322cd 100644 --- a/internal/lefthook/uninstall.go +++ b/internal/lefthook/uninstall.go @@ -34,7 +34,9 @@ func (l *Lefthook) Uninstall(args *UninstallArgs) error { if args.RemoveConfig { for _, glob := range []string{ + ".lefthook.y*ml", "lefthook.y*ml", + ".lefthook-local.y*ml", "lefthook-local.y*ml", } { l.removeFile(filepath.Join(l.repo.RootPath, glob)) From 2d4fc20b754275993ee3a90e44ef690971e76ca5 Mon Sep 17 00:00:00 2001 From: Edwin Kofler Date: Fri, 14 Jul 2023 15:15:29 -0700 Subject: [PATCH 4/6] fix: Make the dot variant optionally supported --- internal/config/load.go | 6 +++--- internal/config/load_test.go | 20 ++++++++++++-------- internal/lefthook/dump_test.go | 7 +++++++ 3 files changed, 22 insertions(+), 11 deletions(-) create mode 100644 internal/lefthook/dump_test.go diff --git a/internal/config/load.go b/internal/config/load.go index e62e9efd..7d206747 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -35,7 +35,7 @@ func (err NotFoundError) Error() string { // Loads configs from the given directory with extensions. func Load(fs afero.Fs, repo *git.Repository) (*Config, error) { - global, err := readOne(fs, repo.RootPath, []string{".lefthook", "lefthook"}) + global, err := readOne(fs, repo.RootPath, []string{"lefthook", ".lefthook"}) if err != nil { return nil, err } @@ -98,7 +98,7 @@ func readOne(fs afero.Fs, path string, names []string) (*viper.Viper, error) { // mergeAll merges (.lefthook or lefthook) and (extended config) and (remote) // and (.lefthook-local or .lefthook-local) configs. func mergeAll(fs afero.Fs, repo *git.Repository) (*viper.Viper, error) { - extends, err := readOne(fs, repo.RootPath, []string{".lefthook", "lefthook"}) + extends, err := readOne(fs, repo.RootPath, []string{"lefthook", ".lefthook"}) if err != nil { return nil, err } @@ -111,7 +111,7 @@ func mergeAll(fs afero.Fs, repo *git.Repository) (*viper.Viper, error) { return nil, err } - if err := mergeOne([]string{".lefthook-local", "lefthook-local"}, "", extends); err == nil { + if err := mergeOne([]string{"lefthook-local", ".lefthook-local"}, "", extends); err == nil { if err = extend(extends, repo.RootPath); err != nil { return nil, err } diff --git a/internal/config/load_test.go b/internal/config/load_test.go index aead0dfc..3c37fff6 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -79,7 +79,7 @@ pre-commit: }, }, { - name: "with global, dot has priority", + name: "with global, nodot has priority", otherFiles: map[string]string{ ".lefthook.yml": ` pre-commit: @@ -103,7 +103,7 @@ pre-commit: Parallel: false, Commands: map[string]*Command{ "tests": { - Run: "yarn test1", + Run: "yarn test2", }, }, }, @@ -297,7 +297,7 @@ pre-push: }, }, { - name: "with overrides, dot has priority", + name: "with overrides, nodot has priority", otherFiles: map[string]string{ "lefthook.yml": ` pre-push: @@ -329,7 +329,7 @@ pre-push: Runner: "bash", }, "local-extend": { - Runner: "bash1", + Runner: "bash2", }, }, }, @@ -614,12 +614,16 @@ pre-push: } t.Run(fmt.Sprintf("%d: %s", i, tt.name), func(t *testing.T) { - if err := fs.WriteFile(filepath.Join(root, "lefthook.yml"), []byte(tt.global), 0o644); err != nil { - t.Errorf("unexpected error: %s", err) + if tt.global != "" { + if err := fs.WriteFile(filepath.Join(root, "lefthook.yml"), []byte(tt.global), 0o644); err != nil { + t.Errorf("unexpected error: %s", err) + } } - if err := fs.WriteFile(filepath.Join(root, "lefthook-local.yml"), []byte(tt.local), 0o644); err != nil { - t.Errorf("unexpected error: %s", err) + if tt.local != "" { + if err := fs.WriteFile(filepath.Join(root, "lefthook-local.yml"), []byte(tt.local), 0o644); err != nil { + t.Errorf("unexpected error: %s", err) + } } if len(tt.remoteConfigPath) > 0 { diff --git a/internal/lefthook/dump_test.go b/internal/lefthook/dump_test.go new file mode 100644 index 00000000..70af9cb3 --- /dev/null +++ b/internal/lefthook/dump_test.go @@ -0,0 +1,7 @@ +package lefthook + +import "testing" + +func TestLefthookDump(t *testing.T) { + +} From a641a597eb79ed0e1898b07b31b61b3adaa58c57 Mon Sep 17 00:00:00 2001 From: Edwin Kofler Date: Fri, 14 Jul 2023 15:19:12 -0700 Subject: [PATCH 5/6] fix: Revert back to creating `lefthook.yml` --- internal/config/load.go | 1 - internal/lefthook/install.go | 2 +- internal/lefthook/install_test.go | 3 +-- testdata/install.txt | 2 +- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/config/load.go b/internal/config/load.go index 7d206747..f05f3e17 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -15,7 +15,6 @@ import ( ) const ( - DefaultDotConfigName = ".lefthook.yml" DefaultConfigName = "lefthook.yml" DefaultSourceDir = ".lefthook" DefaultSourceDirLocal = ".lefthook-local" diff --git a/internal/lefthook/install.go b/internal/lefthook/install.go index a4685d1b..c2a0896a 100644 --- a/internal/lefthook/install.go +++ b/internal/lefthook/install.go @@ -99,7 +99,7 @@ func (l *Lefthook) configExists(path string) bool { } func (l *Lefthook) createConfig(path string) error { - file := filepath.Join(path, config.DefaultDotConfigName) + file := filepath.Join(path, config.DefaultConfigName) err := afero.WriteFile(l.Fs, file, templates.Config(), configFileMode) if err != nil { diff --git a/internal/lefthook/install_test.go b/internal/lefthook/install_test.go index 3fa9d37b..6fafcba6 100644 --- a/internal/lefthook/install_test.go +++ b/internal/lefthook/install_test.go @@ -18,7 +18,6 @@ func TestLefthookInstall(t *testing.T) { t.Errorf("unexpected error: %s", err) } - dotConfigPath := filepath.Join(root, ".lefthook.yml") configPath := filepath.Join(root, "lefthook.yml") hookPath := func(hook string) string { @@ -44,7 +43,7 @@ func TestLefthookInstall(t *testing.T) { }{ { name: "without a config file", - wantExist: []string{dotConfigPath}, + wantExist: []string{configPath}, }, { name: "simple default config", diff --git a/testdata/install.txt b/testdata/install.txt index 754cd265..250ac54b 100644 --- a/testdata/install.txt +++ b/testdata/install.txt @@ -1,4 +1,4 @@ exec git init exec lefthook install -exists .lefthook.yml +exists lefthook.yml ! stderr . From 101a50af20a0cbebccc53d2faa885175e321a865 Mon Sep 17 00:00:00 2001 From: Edwin Kofler Date: Fri, 14 Jul 2023 15:25:06 -0700 Subject: [PATCH 6/6] Remove spurious file --- internal/lefthook/dump_test.go | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 internal/lefthook/dump_test.go diff --git a/internal/lefthook/dump_test.go b/internal/lefthook/dump_test.go deleted file mode 100644 index 70af9cb3..00000000 --- a/internal/lefthook/dump_test.go +++ /dev/null @@ -1,7 +0,0 @@ -package lefthook - -import "testing" - -func TestLefthookDump(t *testing.T) { - -}