Merge pull request #588 from wakatime/feature/ini

Revert to use gopkg.in/ini.v1 when saving config file

go.mod

@@ -17,7 +17,7 @@ require (
 	github.com/stretchr/testify v1.7.0
 	github.com/yookoala/realpath v1.0.0
 	go.etcd.io/bbolt v1.3.5
-	gopkg.in/ini.v1 v1.62.0
+	gopkg.in/ini.v1 v1.65.0
 )
 
 require (

go.sum

@@ -680,6 +680,8 @@ gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
 gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
 gopkg.in/ini.v1 v1.62.0 h1:duBzk771uxoUuOlyRLkHsygud9+5lrlGjdFBb4mSKDU=
 gopkg.in/ini.v1 v1.62.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
+gopkg.in/ini.v1 v1.65.0 h1:B2//IEITFk89S+Nl2tozBeqUvFEpUAY6daarSlrx8jU=
+gopkg.in/ini.v1 v1.65.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
 gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo=
 gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74=
 gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=

pkg/backoff/backoff.go

@@ -7,9 +7,8 @@ import (
 	"time"
 
 	"github.com/wakatime/wakatime-cli/pkg/api"
-	"github.com/wakatime/wakatime-cli/pkg/config"
+	ini "github.com/wakatime/wakatime-cli/pkg/config"
 	"github.com/wakatime/wakatime-cli/pkg/heartbeat"
-	"github.com/wakatime/wakatime-cli/pkg/ini"
 	"github.com/wakatime/wakatime-cli/pkg/log"
 
 	"github.com/spf13/viper"
@@ -35,12 +34,12 @@ type Config struct {
 // WithBackoff initializes and returns a heartbeat handle option, which
 // can be used in a heartbeat processing pipeline to prevent trying to send
 // a heartbeat when the api is unresponsive.
-func WithBackoff(c Config) heartbeat.HandleOption {
+func WithBackoff(config Config) heartbeat.HandleOption {
 	return func(next heartbeat.Handle) heartbeat.Handle {
 		return func(hh []heartbeat.Heartbeat) ([]heartbeat.Result, error) {
 			log.Debugln("execute heartbeat backoff algorithm")
 
-			if shouldBackoff(c.Retries, c.At) {
+			if shouldBackoff(config.Retries, config.At) {
 				return nil, api.Err("won't send heartbeat due to backoff")
 			}
 
@@ -49,16 +48,16 @@ func WithBackoff(c Config) heartbeat.HandleOption {
 				log.Debugf("incrementing backoff due to error")
 
 				// error response, increment backoff
-				if updateErr := updateBackoffSettings(c.V, c.Retries+1, time.Now()); updateErr != nil {
+				if updateErr := updateBackoffSettings(config.V, config.Retries+1, time.Now()); updateErr != nil {
 					log.Warnf("failed to update backoff settings: %s", updateErr)
 				}
 
 				return nil, err
 			}
 
-			if !c.At.IsZero() {
+			if !config.At.IsZero() {
 				// success response, reset backoff
-				if resetErr := updateBackoffSettings(c.V, 0, time.Time{}); resetErr != nil {
+				if resetErr := updateBackoffSettings(config.V, 0, time.Time{}); resetErr != nil {
 					log.Warnf("failed to reset backoff settings: %s", resetErr)
 				}
 			}
@@ -87,28 +86,25 @@ func shouldBackoff(retries int, at time.Time) bool {
 }
 
 func updateBackoffSettings(v *viper.Viper, retries int, at time.Time) error {
-	keys := map[ini.Key]string{
-		{
-			Section: "internal",
-			Name:    "backoff_retries",
-		}: strconv.Itoa(retries),
-		{
-			Section: "internal",
-			Name:    "backoff_at",
-		}: "",
+	w, err := ini.NewIniWriter(v, ini.FilePath)
+	if err != nil {
+		return fmt.Errorf("failed to parse config file: %s", err)
+	}
+
+	keyValue := map[string]string{
+		"backoff_retries": strconv.Itoa(retries),
+		"backoff_at":      "",
 	}
 
 	if !at.IsZero() {
-		keys[ini.Key{
-			Section: "internal",
-			Name:    "backoff_at",
-		}] = at.Format(config.DateFormat)
+		keyValue["backoff_at"] = at.Format(ini.DateFormat)
+	} else {
+		keyValue["backoff_at"] = ""
 	}
 
-	iniFile, err := config.InternalFilePath(v)
-	if err != nil {
-		return fmt.Errorf("error getting filepath: %s", err)
+	if err := w.Write("internal", keyValue); err != nil {
+		return fmt.Errorf("failed to write to internal config file: %s", err)
 	}
 
-	return ini.SetKeys(iniFile, keys)
+	return nil
 }

pkg/backoff/backoff_internal_test.go

@@ -2,12 +2,10 @@ package backoff
 
 import (
 	"os"
-	"strings"
 	"testing"
 	"time"
 
 	"github.com/wakatime/wakatime-cli/pkg/config"
-	"github.com/wakatime/wakatime-cli/pkg/ini"
 
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
@@ -52,16 +50,16 @@ func TestUpdateBackoffSettings(t *testing.T) {
 	err = updateBackoffSettings(v, 2, at)
 	require.NoError(t, err)
 
-	writer, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) {
+	ini, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) {
 		assert.Equal(t, v, vp)
 		return tmpFile.Name(), nil
 	})
 	require.NoError(t, err)
 
-	backoffAt := writer.File.Section("internal").Key("backoff_at").MustTimeFormat(config.DateFormat)
+	backoffAt := ini.File.Section("internal").Key("backoff_at").MustTimeFormat(config.DateFormat)
 
 	assert.WithinDuration(t, time.Now(), backoffAt, 15*time.Second)
-	assert.Equal(t, "2", writer.File.Section("internal").Key("backoff_retries").String())
+	assert.Equal(t, "2", ini.File.Section("internal").Key("backoff_retries").String())
 }
 
 func TestUpdateBackoffSettings_NotInBackoff(t *testing.T) {
@@ -78,58 +76,12 @@ func TestUpdateBackoffSettings_NotInBackoff(t *testing.T) {
 	err = updateBackoffSettings(v, 0, time.Time{})
 	require.NoError(t, err)
 
-	writer, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) {
+	ini, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) {
 		assert.Equal(t, v, vp)
 		return tmpFile.Name(), nil
 	})
 	require.NoError(t, err)
 
-	assert.Empty(t, writer.File.Section("internal").Key("backoff_at").String())
-	assert.Equal(t, "0", writer.File.Section("internal").Key("backoff_retries").String())
-}
-
-func TestUpdateBackoffSettings_NoMultilineSideEffects(t *testing.T) {
-	v := viper.New()
-
-	tmpFile, err := os.CreateTemp(os.TempDir(), "wakatime")
-	require.NoError(t, err)
-
-	defer os.Remove(tmpFile.Name())
-
-	v.Set("config", tmpFile.Name())
-	v.Set("internal-config", tmpFile.Name())
-
-	copyFile(t, "testdata/multiline.cfg", tmpFile.Name())
-
-	err = updateBackoffSettings(v, 0, time.Time{})
-	require.NoError(t, err)
-
-	writer, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) {
-		assert.Equal(t, v, vp)
-		return tmpFile.Name(), nil
-	})
-	require.NoError(t, err)
-
-	assert.Equal(t, "\none\ntwo", writer.File.Section("settings").Key("ignore").String())
-	assert.Empty(t, writer.File.Section("internal").Key("backoff_at").String())
-	assert.Equal(t, "0", writer.File.Section("internal").Key("backoff_retries").String())
-
-	value := ini.GetKey(tmpFile.Name(), ini.Key{Section: "settings", Name: "ignore"})
-	assert.Equal(t, "\n one\n two", value)
-
-	actual, err := os.ReadFile(tmpFile.Name())
-	require.NoError(t, err)
-
-	expected, err := os.ReadFile("testdata/multiline_expected.cfg")
-	require.NoError(t, err)
-
-	assert.Equal(t, strings.ReplaceAll(string(expected), "\r", ""), string(actual))
-}
-
-func copyFile(t *testing.T, source, destination string) {
-	input, err := os.ReadFile(source)
-	require.NoError(t, err)
-
-	err = os.WriteFile(destination, input, 0600)
-	require.NoError(t, err)
+	assert.Empty(t, ini.File.Section("internal").Key("backoff_at").String())
+	assert.Equal(t, "0", ini.File.Section("internal").Key("backoff_retries").String())
 }

pkg/config/config_test.go

@@ -39,10 +39,10 @@ func TestReadInConfig_Multiline(t *testing.T) {
 	require.NoError(t, err)
 
 	ignoreConfig := strings.ReplaceAll(vipertools.GetString(v, "settings.ignore"), "\r", "")
-	assert.Equal(t, "\nCOMMIT_EDITMSG$\nPULLREQ_EDITMSG$\nMERGE_MSG$\nTAG_EDITMSG$", ignoreConfig)
+	assert.Equal(t, "\n  COMMIT_EDITMSG$\n  PULLREQ_EDITMSG$\n  MERGE_MSG$\n  TAG_EDITMSG$", ignoreConfig)
 
 	gitConfig := strings.ReplaceAll(vipertools.GetString(v, "git.submodules_disabled"), "\r", "")
-	assert.Equal(t, "\n.*secret.*\nfix.*", gitConfig)
+	assert.Equal(t, "\n  .*secret.*\n  fix.*", gitConfig)
 }
 
 func TestReadInConfig_Multiple(t *testing.T) {
@@ -238,6 +238,39 @@ func TestWrite(t *testing.T) {
 	}
 }
 
+func TestWrite_NoMultilineSideEffects(t *testing.T) {
+	multilineOption := viper.IniLoadOptions(ini.LoadOptions{AllowPythonMultilineValues: true})
+	v := viper.NewWithOptions(multilineOption)
+
+	tmpFile, err := os.CreateTemp(os.TempDir(), "wakatime")
+	require.NoError(t, err)
+
+	defer os.Remove(tmpFile.Name())
+
+	v.Set("config", tmpFile.Name())
+
+	copyFile(t, "testdata/wakatime-multiline.cfg", tmpFile.Name())
+
+	w, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) {
+		assert.Equal(t, v, vp)
+		return tmpFile.Name(), nil
+	})
+	require.NoError(t, err)
+
+	err = w.Write("settings", map[string]string{"debug": "true"})
+	require.NoError(t, err)
+
+	actual, err := os.ReadFile(tmpFile.Name())
+	require.NoError(t, err)
+
+	expected, err := os.ReadFile("testdata/wakatime-multiline-expected.cfg")
+	require.NoError(t, err)
+
+	assert.Equal(t,
+		strings.ReplaceAll(string(expected), "\r", ""),
+		strings.ReplaceAll(string(actual), "\r", ""))
+}
+
 func TestWriteErr(t *testing.T) {
 	w := config.IniWriter{}
 
@@ -246,3 +279,11 @@ func TestWriteErr(t *testing.T) {
 
 	assert.Equal(t, "got undefined wakatime config file instance", err.Error())
 }
+
+func copyFile(t *testing.T, source, destination string) {
+	input, err := os.ReadFile(source)
+	require.NoError(t, err)
+
+	err = os.WriteFile(destination, input, 0600)
+	require.NoError(t, err)
+}

pkg/config/testdata/wakatime-multiline-expected.cfg

@@ -0,0 +1,13 @@
+[settings]
+ignore = """
+  COMMIT_EDITMSG$
+  PULLREQ_EDITMSG$
+  MERGE_MSG$
+  TAG_EDITMSG$"""
+debug  = true
+
+[git]
+submodules_disabled = """
+  .*secret.*
+  fix.*"""
+