Refactor migrations to only marshall yaml twice

pkg/config/app_config.go

@@ -5,6 +5,7 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"time"
 
@@ -237,7 +238,17 @@ func migrateUserConfig(path string, content []byte) ([]byte, error) {
 
 // A pure function helper for testing purposes
 func computeMigratedConfig(path string, content []byte) ([]byte, error) {
-	changedContent := content
+	var err error
+	var rootNode yaml.Node
+	err = yaml.Unmarshal(content, &rootNode)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse YAML: %w", err)
+	}
+	var originalCopy yaml.Node
+	err = yaml.Unmarshal(content, &originalCopy)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse YAML, but only the second time!?!? How did that happen: %w", err)
+	}
 
 	pathsToReplace := []struct {
 		oldPath []string
@@ -248,46 +259,52 @@ func computeMigratedConfig(path string, content []byte) ([]byte, error) {
 		{[]string{"gui", "windowSize"}, "screenMode"},
 	}
 
-	var err error
 	for _, pathToReplace := range pathsToReplace {
-		changedContent, err = yaml_utils.RenameYamlKey(changedContent, pathToReplace.oldPath, pathToReplace.newName)
+		err := yaml_utils.RenameYamlKey(&rootNode, pathToReplace.oldPath, pathToReplace.newName)
 		if err != nil {
 			return nil, fmt.Errorf("Couldn't migrate config file at `%s` for key %s: %s", path, strings.Join(pathToReplace.oldPath, "."), err)
 		}
 	}
 
-	changedContent, err = changeNullKeybindingsToDisabled(changedContent)
+	err = changeNullKeybindingsToDisabled(&rootNode)
 	if err != nil {
 		return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
 	}
 
-	changedContent, err = changeElementToSequence(changedContent, []string{"git", "commitPrefix"})
+	err = changeElementToSequence(&rootNode, []string{"git", "commitPrefix"})
 	if err != nil {
 		return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
 	}
 
-	changedContent, err = changeCommitPrefixesMap(changedContent)
+	err = changeCommitPrefixesMap(&rootNode)
 	if err != nil {
 		return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
 	}
+
 	// Add more migrations here...
 
-	return changedContent, nil
+	if !reflect.DeepEqual(rootNode, originalCopy) {
+		newContent, err := yaml_utils.YamlMarshal(&rootNode)
+		if err != nil {
+			return nil, fmt.Errorf("Failed to remarsal!\n %w", err)
+		}
+		return newContent, nil
+	} else {
+		return content, nil
+	}
 }
 
-func changeNullKeybindingsToDisabled(changedContent []byte) ([]byte, error) {
-	return yaml_utils.Walk(changedContent, func(node *yaml.Node, path string) bool {
+func changeNullKeybindingsToDisabled(rootNode *yaml.Node) error {
+	return yaml_utils.Walk(rootNode, func(node *yaml.Node, path string) {
 		if strings.HasPrefix(path, "keybinding.") && node.Kind == yaml.ScalarNode && node.Tag == "!!null" {
 			node.Value = "<disabled>"
 			node.Tag = "!!str"
-			return true
 		}
-		return false
 	})
 }
 
-func changeElementToSequence(changedContent []byte, path []string) ([]byte, error) {
-	return yaml_utils.TransformNode(changedContent, path, func(node *yaml.Node) (bool, error) {
+func changeElementToSequence(rootNode *yaml.Node, path []string) error {
+	return yaml_utils.TransformNode(rootNode, path, func(node *yaml.Node) error {
 		if node.Kind == yaml.MappingNode {
 			nodeContentCopy := node.Content
 			node.Kind = yaml.SequenceNode
@@ -298,15 +315,14 @@ func changeElementToSequence(changedContent []byte, path []string) ([]byte, erro
 				Content: nodeContentCopy,
 			}}
 
-			return true, nil
+			return nil
 		}
-		return false, nil
+		return nil
 	})
 }
 
-func changeCommitPrefixesMap(changedContent []byte) ([]byte, error) {
-	return yaml_utils.TransformNode(changedContent, []string{"git", "commitPrefixes"}, func(prefixesNode *yaml.Node) (bool, error) {
-		changedAnyNodes := false
+func changeCommitPrefixesMap(rootNode *yaml.Node) error {
+	return yaml_utils.TransformNode(rootNode, []string{"git", "commitPrefixes"}, func(prefixesNode *yaml.Node) error {
 		if prefixesNode.Kind == yaml.MappingNode {
 			for _, contentNode := range prefixesNode.Content {
 				if contentNode.Kind == yaml.MappingNode {
@@ -318,11 +334,10 @@ func changeCommitPrefixesMap(changedContent []byte) ([]byte, error) {
 						Kind:    yaml.MappingNode,
 						Content: nodeContentCopy,
 					}}
-					changedAnyNodes = true
 				}
 			}
 		}
-		return changedAnyNodes, nil
+		return nil
 	})
 }