Repository: evilmartians/lefthook (language: python)

Issue:
support new config file format
It will be really cool to add `.lefthook.yml` (with dot in the beginning) config file format support. Just to align with other libs:
- [lint-staged](https://github.com/okonet/lint-staged#configuration)
- [syncpack](https://github.com/JamieMason/syncpack#-configuration-file)
- [eslint](https://eslint.org/docs/latest/user-guide/configuring/configuration-files)


Discussion / hints:
Sounds cool! But I have concerns about how it would play with *overriding feature*. I mean, we support `lefthook.yml` as global config and `lefthook-local.yml` as local config which overrides values from global config. 

It would be confusing if we have both `.lefthook.yml` and `lefthook.yml`. Main questions is : how should we merge them? What config would be the "main"? Wouldn't it be confusing?

Good point! I think for this case we shouldn’t support overriding feature. We just need to define priority order and use only one config file. Same as [eslint](https://eslint.org/docs/latest/user-guide/configuring/configuration-files) currently does:

> If there are multiple configuration files in the same directory, ESLint will only use one. The priority order is as follows:

For global it could be something like this:
1. `lefthook.yml` (potentially this format can be deprecated and removed in v2)
2. `.lefthook.yml`
3. `.lefthook.json` (maybe it's a good idea to add `json` format support)

For local:
1. `lefthook-local.yml`
2. `.lefthook.local.yml` (see [CRA](https://create-react-app.dev/docs/adding-custom-environment-variables/#what-other-env-files-can-be-used) for `.local` reference)
3. `.lefthook.local.json`
It seems like it would be best to treat any config __file__ without `-local` as a global config.

You could get the core config file path via this regex [[regex101](https://regex101.com/r/EULlOW/1)]:

```golang
^\.?lefthook\.(yml|yaml|json)
```

And any local configs via this regex [[regex101](https://regex101.com/r/fSW9UG/1)]:

```golang
^\.?lefthook-local\.(yml|yaml|json)
```

## Valid example

All of these scenarios would be supported:

### current
```
.
├── lefthook-local.yaml
└── lefthook.yaml
```

### new dot configs
```
.
├── .lefthook-local.yaml
└── .lefthook.yaml
```

### mixed
```
.
├── lefthook-local.yaml
└── .lefthook.yaml
```

For config files conventions are pretty common these days.

----

## Invalid example

Multiple global or local configs would throw an error.

```
.
├── lefthook.yaml
└── .lefthook.yaml

# Error: invalid configuration, too many files.
```
Wow, it makes sense! What do you think about default config file? I guess we could change it to be `.lefthook.yml` in v2 but still support `lefthook.yml` for existing projects.

Why do you think we should support JSON format? What is the potential use case?

Do you have a will to prepare a draft PR for this change?
> Good point! I think for this case we shouldn’t support overriding feature. We just need to define priority order and use only one config file. 

Something I would add to this is to display a warning when multiple global config files are found, and say which is being used. This could be a pretty frustrating situation when you have both, trying to iterate on the file, and not see your changes taking an effect until you eventually discover that a different file is loading.

Fix patch:
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 ae9ea4aa..f05f3e17 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, &notFoundErr); ok {
-			return nil, NotFoundError{err.Error()}
-		}
-
 		return nil, err
 	}
 
@@ -81,9 +76,28 @@ func read(fs afero.Fs, path string, name string) (*viper.Viper, error) {
 	return v, nil
 }
 
-// mergeAll merges remotes and extends from .lefthook and .lefthook-local.
+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, &notFoundErr); 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 (.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 := read(fs, repo.RootPath, "lefthook")
+	extends, err := readOne(fs, repo.RootPath, []string{"lefthook", ".lefthook"})
 	if err != nil {
 		return nil, err
 	}
@@ -96,7 +110,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
 		}
@@ -173,6 +187,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, &notFoundErr); !ok {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 func unmarshalConfigs(base, extra *viper.Viper, c *Config) error {
 	c.Hooks = make(map[string]*Hook)
 
diff --git a/internal/lefthook/install.go b/internal/lefthook/install.go
index 32504ab2..c2a0896a 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")
 )
 
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))


Test patch:
diff --git a/internal/config/load_test.go b/internal/config/load_test.go
index c65ec338..3c37fff6 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, nodot 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 test2",
+							},
+						},
+					},
+				},
+			},
+		},
 		{
 			name: "simple",
 			global: `
@@ -144,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, nodot 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: "bash2",
+							},
+						},
+					},
+				},
+			},
+		},
 		{
 			name: "with extra hooks",
 			global: `
@@ -356,7 +548,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:
@@ -422,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 {
@@ -440,7 +636,7 @@ pre-push:
 				}
 			}
 
-			for name, content := range tt.extends {
+			for name, content := range tt.otherFiles {
 				path := filepath.Join(
 					root,
 					filepath.Join(strings.Split(name, "/")...),