Merge pull request profclems#813 from profclems/config-fix

internal/config: migrate local config to `.git/glab-cli`

.gitignore

@@ -47,3 +47,4 @@ test/testdata-*
 coverage*
 vendor
 log.txt.glab-cli
+.git

README.md

@@ -292,7 +292,7 @@ Get a GitLab access token at <https://gitlab.com/-/profile/personal_access_token
 
 ## Configuration
 
-`glab` follows the XDG Base Directory [Spec](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html): global configuration file is saved at `~/.config/glab-cli`. Local configuration file is saved at the root of the working git directory and automatically added to `.gitignore`.
+`glab` follows the XDG Base Directory [Spec](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html): global configuration file is saved at `~/.config/glab-cli`. Local configuration file is saved at `.git/glab-cli` in the current working git directory.
 
 **To set configuration globally**
 

internal/config/config_migration.go

@@ -38,6 +38,27 @@ func MigrateOldConfig() error {
 	if err := migrateUserConfigs(".glab-cli/config", cfg, false); err != nil {
 		return err
 	}
+
+	// move local config from {currentDir}/.glab-cli/config to {currentDir}/.git/glab-cli
+	oldLocalCfgFile := OldLocalConfigFile()
+	if CheckFileExists(oldLocalCfgFile) {
+		log.Println("- Migrating local config dir from", oldLocalCfgFile, "to", LocalConfigFile())
+		newLocalPath := LocalConfigDir()
+		if !CheckPathExists(filepath.Join(newLocalPath...)) {
+			if err := os.MkdirAll(filepath.Join(newLocalPath...), os.ModePerm); err != nil {
+				return fmt.Errorf("failed to create new local config dir: %v", err)
+			}
+		}
+		err = copy(oldLocalCfgFile, LocalConfigFile())
+		if err != nil {
+			return err
+		}
+		// backup old local config file
+		err = BackupConfigFile(oldLocalCfgFile)
+		if err != nil {
+			return err
+		}
+	}
 	return nil
 }
 
@@ -215,3 +236,13 @@ func writeConfig(cfg Config, key, value string, isGlobal bool) (nCfg Config, err
 	}
 	return
 }
+
+func copy(src string, dst string) error {
+	// Read all content of src to data
+	data, err := ioutil.ReadFile(src)
+	if err != nil {
+		return err
+	}
+	// Write data to dst
+	return WriteFile(dst, data, 0600)
+}

internal/config/config_type.go

@@ -45,7 +45,7 @@ type HostConfig struct {
 	Host string
 }
 
-// This type implements a low-level get/set config that is backed by an in-memory tree of Yaml
+// ConfigMap type implements a low-level get/set config that is backed by an in-memory tree of Yaml
 // nodes. It allows us to interact with a yaml-based config programmatically, preserving any
 // comments that were present when the yaml was parsed.
 type ConfigMap struct {

internal/config/file.go

@@ -1,7 +1,6 @@
 package config
 
 import (
-	"bufio"
 	"os"
 )
 
@@ -22,42 +21,6 @@ func CheckFileExists(filename string) bool {
 	return !info.IsDir()
 }
 
-// CheckFileHasLine : returns true if line exists in file, otherwise false (also for non-existant file).
-func CheckFileHasLine(filePath, line string) bool {
-	f, err := os.Open(filePath)
-	if err != nil {
-		return false
-	}
-	defer f.Close()
-
-	fs := bufio.NewScanner(f)
-	fs.Split(bufio.ScanLines)
-
-	for fs.Scan() {
-		if fs.Text() == line {
-			return true
-		}
-	}
-
-	return false
-}
-
-// ReadAndAppend : appends string to file
-func ReadAndAppend(file, text string) error {
-	// If the file doesn't exist, create it, or append to the file
-	f, err := os.OpenFile(file, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
-	if err != nil {
-		return err
-	}
-	if _, err := f.Write([]byte(text)); err != nil {
-		return err
-	}
-	if err := f.Close(); err != nil {
-		return err
-	}
-	return nil
-}
-
 // BackupConfigFile creates a backup of the provided config file
 var BackupConfigFile = func(filename string) error {
 	return os.Rename(filename, filename+".bak")

internal/config/file_test.go

@@ -3,7 +3,6 @@ package config
 import (
 	"io/ioutil"
 	"os"
-	"path/filepath"
 	"testing"
 
 	"github.com/alecthomas/assert"
@@ -67,104 +66,3 @@ func Test_BackupConfigFile(t *testing.T) {
 		assert.EqualError(t, err, "rename /Path/Not/Exist /Path/Not/Exist.bak: no such file or directory")
 	})
 }
-
-func Test_CheckFileHasLine(t *testing.T) {
-	t.Run("success", func(t *testing.T) {
-		file, err := ioutil.TempFile("", "")
-		if err != nil {
-			t.Skipf("Unexpected error creeating temporary file for testing = %s", err)
-		}
-		fPath := file.Name()
-		defer os.Remove(fPath)
-
-		_, _ = file.WriteString("profclems/glab")
-
-		got := CheckFileHasLine(fPath, "profclems/glab")
-		assert.True(t, got)
-	})
-	t.Run("failed", func(t *testing.T) {
-		t.Run("no-line-present", func(t *testing.T) {
-			file, err := ioutil.TempFile("", "")
-			if err != nil {
-				t.Skipf("Unexpected error creeating temporary file for testing = %s", err)
-			}
-			fPath := file.Name()
-			defer os.Remove(fPath)
-
-			_, _ = file.WriteString("profclems/glab")
-
-			got := CheckFileHasLine(fPath, "maxice8/glab")
-			assert.False(t, got)
-		})
-		t.Run("no-file-present", func(t *testing.T) {
-			got := CheckFileHasLine("/Path/Not/Exist", "profclems/glab")
-			assert.False(t, got)
-		})
-	})
-}
-
-func Test_ReadAndAppend(t *testing.T) {
-	t.Run("success", func(t *testing.T) {
-		t.Run("write", func(t *testing.T) {
-			file, err := ioutil.TempFile("", "")
-			if err != nil {
-				t.Skipf("Unexpected error creating temporary file for testing = %s", err)
-			}
-			fPath := file.Name()
-			defer os.Remove(fPath)
-
-			err = ReadAndAppend(fPath, "profclems/glab")
-			assert.NoError(t, err)
-			got := CheckFileHasLine(fPath, "profclems/glab")
-			assert.True(t, got)
-		})
-		t.Run("create-and-write", func(t *testing.T) {
-			dir, err := ioutil.TempDir("", "")
-			if err != nil {
-				t.Skipf("Unexpected error creating temporary directory for testing = %s", err)
-			}
-			defer os.RemoveAll(dir)
-
-			fPath := filepath.Join(dir, "file")
-
-			err = ReadAndAppend(fPath, "profclems/glab")
-			assert.NoError(t, err)
-			got := CheckFileHasLine(fPath, "profclems/glab")
-			assert.True(t, got)
-		})
-		t.Run("create-and-write-and-append", func(t *testing.T) {
-			dir, err := ioutil.TempDir("", "")
-			if err != nil {
-				t.Skipf("Unexpected error creating temporary directory for testing = %s", err)
-			}
-			defer os.RemoveAll(dir)
-
-			fPath := filepath.Join(dir, "file")
-
-			err = ReadAndAppend(fPath, "profclems/glab")
-			assert.NoError(t, err)
-			err = ReadAndAppend(fPath, "maxice8/glab")
-			assert.NoError(t, err)
-		})
-		t.Run("write-and-append", func(t *testing.T) {
-			file, err := ioutil.TempFile("", "")
-			if err != nil {
-				t.Skipf("Unexpected error creating temporary file for testing = %s", err)
-			}
-			fPath := file.Name()
-			defer os.Remove(fPath)
-
-			err = ReadAndAppend(fPath, "profclems/glab")
-			assert.NoError(t, err)
-
-			err = ReadAndAppend(fPath, "maxice8/glab")
-			assert.NoError(t, err)
-		})
-	})
-	t.Run("failed", func(t *testing.T) {
-		t.Run("no-permissions", func(t *testing.T) {
-			err := ReadAndAppend("/no-perm", "profclems/glab")
-			assert.EqualError(t, err, "open /no-perm: permission denied")
-		})
-	})
-}

internal/config/local_config.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"path"
+	"path/filepath"
 
 	"gopkg.in/yaml.v3"
 )
@@ -13,10 +14,12 @@ type LocalConfig struct {
 	Parent Config
 }
 
+const oldLocalConfigFile = ".glab-cli/config/config.yml"
+
 // LocalConfigDir returns the local config path in map
 // which must be joined for complete path
 var LocalConfigDir = func() []string {
-	return []string{".glab-cli", "config"}
+	return []string{".git", "glab-cli"}
 }
 
 // LocalConfigFile returns the config file name with full path
@@ -25,6 +28,11 @@ var LocalConfigFile = func() string {
 	return path.Join(configFile...)
 }
 
+// OldLocalConfigFile returns the path to the old local config path.
+func OldLocalConfigFile() string {
+	return filepath.Clean(oldLocalConfigFile)
+}
+
 func (a *LocalConfig) Get(key string) (string, bool) {
 	key = ConfigKeyEquivalence(key)
 	if a.Empty() {
@@ -72,14 +80,6 @@ func (a *LocalConfig) Write() error {
 		return fmt.Errorf("failed to write config: %w", err)
 	}
 
-	// Append local config dir if not already ignored in the .gitignore file
-	if !CheckFileHasLine(".gitignore", LocalConfigDir()[0]) {
-		err := ReadAndAppend(".gitignore", LocalConfigDir()[0]+"\n")
-		if err != nil {
-			return fmt.Errorf("failed to write file to .gitignore: %w", err)
-		}
-	}
-
 	return nil
 }
 

internal/config/local_config_test.go

@@ -9,15 +9,22 @@ import (
 
 func Test_LocalConfigDir(t *testing.T) {
 	got := LocalConfigDir()
-	assert.ElementsMatch(t, []string{".glab-cli", "config"}, got)
+	assert.ElementsMatch(t, []string{".git", "glab-cli"}, got)
 }
 
 func Test_LocalConfigFile(t *testing.T) {
 	t.Run("default", func(t *testing.T) {
-		expectedPath := filepath.Join(".glab-cli", "config", "config.yml")
+		expectedPath := filepath.Join(".git", "glab-cli", "config.yml")
 		got := LocalConfigFile()
 		assert.Equal(t, expectedPath, got)
 	})
+
+	t.Run("old config file", func(t *testing.T) {
+		expectedPath := filepath.Join(".glab-cli", "config", "config.yml")
+		got := OldLocalConfigFile()
+		assert.Equal(t, expectedPath, got)
+	})
+
 	t.Run("modified-LocalConfigDir()", func(t *testing.T) {
 		expectedPath := filepath.Join(".config", "glab-cli", "config.yml")