Merge 2e4ba67b12 into ef1da6f704

go.mod

@@ -35,6 +35,7 @@ require (
 	github.com/spkg/bom v0.0.0-20160624110644-59b7046e48ad
 	github.com/stefanhaller/git-todo-parser v0.0.7-0.20250429125209-dcf39e4641f5
 	github.com/stretchr/testify v1.10.0
+	github.com/wk8/go-ordered-map/v2 v2.1.8
 	github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778
 	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
 	golang.org/x/sync v0.13.0
@@ -74,7 +75,6 @@ require (
 	github.com/rivo/uniseg v0.4.7 // indirect
 	github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 // indirect
 	github.com/skeema/knownhosts v1.3.1 // indirect
-	github.com/wk8/go-ordered-map/v2 v2.1.8 // indirect
 	github.com/xanzy/ssh-agent v0.3.3 // indirect
 	golang.org/x/crypto v0.37.0 // indirect
 	golang.org/x/net v0.39.0 // indirect

pkg/config/app_config.go

@@ -14,6 +14,7 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/utils"
 	"github.com/jesseduffield/lazygit/pkg/utils/yaml_utils"
 	"github.com/samber/lo"
+	orderedmap "github.com/wk8/go-ordered-map/v2"
 	"gopkg.in/yaml.v3"
 )
 
@@ -96,7 +97,7 @@ func NewAppConfig(
 		configFiles = []*ConfigFile{configFile}
 	}
 
-	userConfig, err := loadUserConfigWithDefaults(configFiles)
+	userConfig, err := loadUserConfigWithDefaults(configFiles, false)
 	if err != nil {
 		return nil, err
 	}
@@ -145,11 +146,11 @@ func findOrCreateConfigDir() (string, error) {
 	return folder, os.MkdirAll(folder, 0o755)
 }
 
-func loadUserConfigWithDefaults(configFiles []*ConfigFile) (*UserConfig, error) {
+func loadUserConfigWithDefaults(configFiles []*ConfigFile, isGuiInitialized bool) (*UserConfig, error) {
-	return loadUserConfig(configFiles, GetDefaultConfig())
+	return loadUserConfig(configFiles, GetDefaultConfig(), isGuiInitialized)
 }
 
-func loadUserConfig(configFiles []*ConfigFile, base *UserConfig) (*UserConfig, error) {
+func loadUserConfig(configFiles []*ConfigFile, base *UserConfig, isGuiInitialized bool) (*UserConfig, error) {
 	for _, configFile := range configFiles {
 		path := configFile.Path
 		statInfo, err := os.Stat(path)
@@ -194,7 +195,7 @@ func loadUserConfig(configFiles []*ConfigFile, base *UserConfig) (*UserConfig, e
 			return nil, err
 		}
 
-		content, err = migrateUserConfig(path, content)
+		content, err = migrateUserConfig(path, content, isGuiInitialized)
 		if err != nil {
 			return nil, err
 		}
@@ -219,37 +220,54 @@ func loadUserConfig(configFiles []*ConfigFile, base *UserConfig) (*UserConfig, e
 // config over time; examples are renaming a key to a better name, moving a key
 // from one container to another, or changing the type of a key (e.g. from bool
 // to an enum).
-func migrateUserConfig(path string, content []byte) ([]byte, error) {
+func migrateUserConfig(path string, content []byte, isGuiInitialized bool) ([]byte, error) {
-	changedContent, err := computeMigratedConfig(path, content)
+	changes := orderedmap.New[string, bool]()
+
+	changedContent, didChange, err := computeMigratedConfig(path, content, changes)
 	if err != nil {
 		return nil, err
 	}
 
-	// Write config back if changed
+	// Nothing to do if config didn't change
-	if string(changedContent) != string(content) {
+	if !didChange {
-		fmt.Println("Provided user config is deprecated but auto-fixable. Attempting to write fixed version back to file...")
+		return content, nil
-		if err := os.WriteFile(path, changedContent, 0o644); err != nil {
-			return nil, fmt.Errorf("While attempting to write back fixed user config to %s, an error occurred: %s", path, err)
-		}
-		fmt.Printf("Success. New config written to %s\n", path)
-		return changedContent, nil
 	}
 
-	return content, nil
+	changesText := "The following changes were made:\n\n"
+	for pair := changes.Oldest(); pair != nil; pair = pair.Next() {
+		changesText += fmt.Sprintf("- %s\n", pair.Key)
+	}
+
+	// Write config back
+	if !isGuiInitialized {
+		fmt.Printf("The user config file %s must be migrated. Attempting to do this automatically.\n", path)
+		fmt.Println(changesText)
+	}
+	if err := os.WriteFile(path, changedContent, 0o644); err != nil {
+		errorMsg := fmt.Sprintf("While attempting to write back migrated user config to %s, an error occurred: %s", path, err)
+		if isGuiInitialized {
+			errorMsg += "\n\n" + changesText
+		}
+		return nil, errors.New(errorMsg)
+	}
+	if !isGuiInitialized {
+		fmt.Printf("Config file saved successfully to %s\n", path)
+	}
+	return changedContent, nil
 }
 
 // A pure function helper for testing purposes
-func computeMigratedConfig(path string, content []byte) ([]byte, error) {
+func computeMigratedConfig(path string, content []byte, changes *orderedmap.OrderedMap[string, bool]) ([]byte, bool, error) {
 	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)
+		return nil, false, 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)
+		return nil, false, fmt.Errorf("failed to parse YAML, but only the second time!?!? How did that happen: %w", err)
 	}
 
 	pathsToReplace := []struct {
@@ -262,60 +280,64 @@ func computeMigratedConfig(path string, content []byte) ([]byte, error) {
 	}
 
 	for _, pathToReplace := range pathsToReplace {
-		err := yaml_utils.RenameYamlKey(&rootNode, pathToReplace.oldPath, pathToReplace.newName)
+		err, didReplace := 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)
+			return nil, false, fmt.Errorf("Couldn't migrate config file at `%s` for key %s: %s", path, strings.Join(pathToReplace.oldPath, "."), err)
+		}
+		if didReplace {
+			changes.Set(fmt.Sprintf("Renamed '%s' to '%s'", strings.Join(pathToReplace.oldPath, "."), pathToReplace.newName), true)
 		}
 	}
 
-	err = changeNullKeybindingsToDisabled(&rootNode)
+	err = changeNullKeybindingsToDisabled(&rootNode, changes)
 	if err != nil {
-		return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
+		return nil, false, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
 	}
 
-	err = changeElementToSequence(&rootNode, []string{"git", "commitPrefix"})
+	err = changeElementToSequence(&rootNode, []string{"git", "commitPrefix"}, changes)
 	if err != nil {
-		return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
+		return nil, false, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
 	}
 
-	err = changeCommitPrefixesMap(&rootNode)
+	err = changeCommitPrefixesMap(&rootNode, changes)
 	if err != nil {
-		return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
+		return nil, false, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
 	}
 
-	err = changeCustomCommandStreamAndOutputToOutputEnum(&rootNode)
+	err = changeCustomCommandStreamAndOutputToOutputEnum(&rootNode, changes)
 	if err != nil {
-		return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
+		return nil, false, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
 	}
 
-	err = migrateAllBranchesLogCmd(&rootNode)
+	err = migrateAllBranchesLogCmd(&rootNode, changes)
 	if err != nil {
-		return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
+		return nil, false, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
 	}
 
 	// Add more migrations here...
 
-	if !reflect.DeepEqual(rootNode, originalCopy) {
+	if reflect.DeepEqual(rootNode, originalCopy) {
-		newContent, err := yaml_utils.YamlMarshal(&rootNode)
+		return nil, false, nil
-		if err != nil {
-			return nil, fmt.Errorf("Failed to remarsal!\n %w", err)
-		}
-		return newContent, nil
-	} else {
-		return content, nil
 	}
+
+	newContent, err := yaml_utils.YamlMarshal(&rootNode)
+	if err != nil {
+		return nil, false, fmt.Errorf("Failed to remarsal!\n %w", err)
+	}
+	return newContent, true, nil
 }
 
-func changeNullKeybindingsToDisabled(rootNode *yaml.Node) error {
+func changeNullKeybindingsToDisabled(rootNode *yaml.Node, changes *orderedmap.OrderedMap[string, bool]) 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"
+			changes.Set(fmt.Sprintf("Changed 'null' to '<disabled>' for keybinding '%s'", path), true)
 		}
 	})
 }
 
-func changeElementToSequence(rootNode *yaml.Node, path []string) error {
+func changeElementToSequence(rootNode *yaml.Node, path []string, changes *orderedmap.OrderedMap[string, bool]) error {
 	return yaml_utils.TransformNode(rootNode, path, func(node *yaml.Node) error {
 		if node.Kind == yaml.MappingNode {
 			nodeContentCopy := node.Content
@@ -327,13 +349,15 @@ func changeElementToSequence(rootNode *yaml.Node, path []string) error {
 				Content: nodeContentCopy,
 			}}
 
+			changes.Set(fmt.Sprintf("Changed '%s' to an array of strings", strings.Join(path, ".")), true)
+
 			return nil
 		}
 		return nil
 	})
 }
 
-func changeCommitPrefixesMap(rootNode *yaml.Node) error {
+func changeCommitPrefixesMap(rootNode *yaml.Node, changes *orderedmap.OrderedMap[string, bool]) error {
 	return yaml_utils.TransformNode(rootNode, []string{"git", "commitPrefixes"}, func(prefixesNode *yaml.Node) error {
 		if prefixesNode.Kind == yaml.MappingNode {
 			for _, contentNode := range prefixesNode.Content {
@@ -346,6 +370,7 @@ func changeCommitPrefixesMap(rootNode *yaml.Node) error {
 						Kind:    yaml.MappingNode,
 						Content: nodeContentCopy,
 					}}
+					changes.Set("Changed 'git.commitPrefixes' elements to arrays of strings", true)
 				}
 			}
 		}
@@ -353,7 +378,7 @@ func changeCommitPrefixesMap(rootNode *yaml.Node) error {
 	})
 }
 
-func changeCustomCommandStreamAndOutputToOutputEnum(rootNode *yaml.Node) error {
+func changeCustomCommandStreamAndOutputToOutputEnum(rootNode *yaml.Node, changes *orderedmap.OrderedMap[string, bool]) error {
 	return yaml_utils.Walk(rootNode, func(node *yaml.Node, path string) {
 		// We are being lazy here and rely on the fact that the only mapping
 		// nodes in the tree under customCommands are actual custom commands. If
@@ -364,16 +389,25 @@ func changeCustomCommandStreamAndOutputToOutputEnum(rootNode *yaml.Node) error {
 			if streamKey, streamValue := yaml_utils.RemoveKey(node, "subprocess"); streamKey != nil {
 				if streamValue.Kind == yaml.ScalarNode && streamValue.Value == "true" {
 					output = "terminal"
+					changes.Set("Changed 'subprocess: true' to 'output: terminal' in custom command", true)
+				} else {
+					changes.Set("Deleted redundant 'subprocess: false' in custom command", true)
 				}
 			}
 			if streamKey, streamValue := yaml_utils.RemoveKey(node, "stream"); streamKey != nil {
 				if streamValue.Kind == yaml.ScalarNode && streamValue.Value == "true" && output == "" {
 					output = "log"
+					changes.Set("Changed 'stream: true' to 'output: log' in custom command", true)
+				} else {
+					changes.Set(fmt.Sprintf("Deleted redundant 'stream: %v' property in custom command", streamValue.Value), true)
 				}
 			}
 			if streamKey, streamValue := yaml_utils.RemoveKey(node, "showOutput"); streamKey != nil {
 				if streamValue.Kind == yaml.ScalarNode && streamValue.Value == "true" && output == "" {
+					changes.Set("Changed 'showOutput: true' to 'output: popup' in custom command", true)
 					output = "popup"
+				} else {
+					changes.Set(fmt.Sprintf("Deleted redundant 'showOutput: %v' property in custom command", streamValue.Value), true)
 				}
 			}
 			if output != "" {
@@ -397,7 +431,7 @@ func changeCustomCommandStreamAndOutputToOutputEnum(rootNode *yaml.Node) error {
 // a single element at `allBranchesLogCmd` and the sequence at `allBranchesLogCmds`.
 // Some users have explicitly set `allBranchesLogCmd` to be an empty string in order
 // to remove it, so in that case we just delete the element, and add nothing to the list
-func migrateAllBranchesLogCmd(rootNode *yaml.Node) error {
+func migrateAllBranchesLogCmd(rootNode *yaml.Node, changes *orderedmap.OrderedMap[string, bool]) error {
 	return yaml_utils.TransformNode(rootNode, []string{"git"}, func(gitNode *yaml.Node) error {
 		cmdKeyNode, cmdValueNode := yaml_utils.LookupKey(gitNode, "allBranchesLogCmd")
 		// Nothing to do if they do not have the deprecated item
@@ -406,6 +440,7 @@ func migrateAllBranchesLogCmd(rootNode *yaml.Node) error {
 		}
 
 		cmdsKeyNode, cmdsValueNode := yaml_utils.LookupKey(gitNode, "allBranchesLogCmds")
+		var change string
 		if cmdsKeyNode == nil {
 			// Create empty sequence node and attach it onto the root git node
 			// We will later populate it with the individual allBranchesLogCmd record
@@ -415,17 +450,24 @@ func migrateAllBranchesLogCmd(rootNode *yaml.Node) error {
 				cmdsKeyNode,
 				cmdsValueNode,
 			)
-		} else if cmdsValueNode.Kind != yaml.SequenceNode {
+			change = "Created git.allBranchesLogCmds array containing value of git.allBranchesLogCmd"
-			return errors.New("You should have an allBranchesLogCmds defined as a sequence!")
+		} else {
+			if cmdsValueNode.Kind != yaml.SequenceNode {
+				return errors.New("You should have an allBranchesLogCmds defined as a sequence!")
+			}
+
+			change = "Prepended git.allBranchesLogCmd value to git.allBranchesLogCmds array"
 		}
 
 		if cmdValueNode.Value != "" {
 			// Prepending the individual element to make it show up first in the list, which was prior behavior
 			cmdsValueNode.Content = utils.Prepend(cmdsValueNode.Content, &yaml.Node{Kind: yaml.ScalarNode, Value: cmdValueNode.Value})
+			changes.Set(change, true)
 		}
 
 		// Clear out the existing allBranchesLogCmd, now that we have migrated it into the list
 		_, _ = yaml_utils.RemoveKey(gitNode, "allBranchesLogCmd")
+		changes.Set("Removed obsolete git.allBranchesLogCmd", true)
 
 		return nil
 	})
@@ -471,7 +513,7 @@ func (c *AppConfig) GetUserConfigDir() string {
 
 func (c *AppConfig) ReloadUserConfigForRepo(repoConfigFiles []*ConfigFile) error {
 	configFiles := append(c.globalUserConfigFiles, repoConfigFiles...)
-	userConfig, err := loadUserConfigWithDefaults(configFiles)
+	userConfig, err := loadUserConfigWithDefaults(configFiles, true)
 	if err != nil {
 		return err
 	}
@@ -496,7 +538,7 @@ func (c *AppConfig) ReloadChangedUserConfigFiles() (error, bool) {
 		return nil, false
 	}
 
-	userConfig, err := loadUserConfigWithDefaults(c.userConfigFiles)
+	userConfig, err := loadUserConfigWithDefaults(c.userConfigFiles, true)
 	if err != nil {
 		return err, false
 	}

pkg/config/app_config_test.go

@@ -4,19 +4,176 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	orderedmap "github.com/wk8/go-ordered-map/v2"
 )
 
+func orderedMapToSlice(m *orderedmap.OrderedMap[string, bool]) []string {
+	result := make([]string, 0, m.Len())
+	for i := m.Oldest(); i != nil; i = i.Next() {
+		result = append(result, i.Key)
+	}
+	return result
+}
+
+func TestMigrationOfRenamedKeys(t *testing.T) {
+	scenarios := []struct {
+		name              string
+		input             string
+		expected          string
+		expectedDidChange bool
+		expectedChanges   []string
+	}{
+		{
+			name:              "Empty String",
+			input:             "",
+			expectedDidChange: false,
+			expectedChanges:   []string{},
+		},
+		{
+			name: "No rename needed",
+			input: `foo:
+  bar: 5
+`,
+			expectedDidChange: false,
+			expectedChanges:   []string{},
+		},
+		{
+			name: "Rename one",
+			input: `gui:
+  skipUnstageLineWarning: true
+`,
+			expected: `gui:
+  skipDiscardChangeWarning: true
+`,
+			expectedDidChange: true,
+			expectedChanges:   []string{"Renamed 'gui.skipUnstageLineWarning' to 'skipDiscardChangeWarning'"},
+		},
+		{
+			name: "Rename several",
+			input: `gui:
+  windowSize: half
+  skipUnstageLineWarning: true
+keybinding:
+  universal:
+    executeCustomCommand: a
+`,
+			expected: `gui:
+  screenMode: half
+  skipDiscardChangeWarning: true
+keybinding:
+  universal:
+    executeShellCommand: a
+`,
+			expectedDidChange: true,
+			expectedChanges: []string{
+				"Renamed 'gui.skipUnstageLineWarning' to 'skipDiscardChangeWarning'",
+				"Renamed 'keybinding.universal.executeCustomCommand' to 'executeShellCommand'",
+				"Renamed 'gui.windowSize' to 'screenMode'",
+			},
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.name, func(t *testing.T) {
+			changes := orderedmap.New[string, bool]()
+			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input), changes)
+			assert.NoError(t, err)
+			assert.Equal(t, s.expectedDidChange, didChange)
+			if didChange {
+				assert.Equal(t, s.expected, string(actual))
+			}
+			assert.Equal(t, s.expectedChanges, orderedMapToSlice(changes))
+		})
+	}
+}
+
+func TestMigrateNullKeybindingsToDisabled(t *testing.T) {
+	scenarios := []struct {
+		name              string
+		input             string
+		expected          string
+		expectedDidChange bool
+		expectedChanges   []string
+	}{
+		{
+			name:              "Empty String",
+			input:             "",
+			expectedDidChange: false,
+			expectedChanges:   []string{},
+		},
+		{
+			name: "No change needed",
+			input: `keybinding:
+  universal:
+    quit: q
+`,
+			expectedDidChange: false,
+			expectedChanges:   []string{},
+		},
+		{
+			name: "Change one",
+			input: `keybinding:
+  universal:
+    quit: null
+`,
+			expected: `keybinding:
+  universal:
+    quit: <disabled>
+`,
+			expectedDidChange: true,
+			expectedChanges:   []string{"Changed 'null' to '<disabled>' for keybinding 'keybinding.universal.quit'"},
+		},
+		{
+			name: "Change several",
+			input: `keybinding:
+  universal:
+    quit: null
+    return: <esc>
+    new: null
+`,
+			expected: `keybinding:
+  universal:
+    quit: <disabled>
+    return: <esc>
+    new: <disabled>
+`,
+			expectedDidChange: true,
+			expectedChanges: []string{
+				"Changed 'null' to '<disabled>' for keybinding 'keybinding.universal.quit'",
+				"Changed 'null' to '<disabled>' for keybinding 'keybinding.universal.new'",
+			},
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.name, func(t *testing.T) {
+			changes := orderedmap.New[string, bool]()
+			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input), changes)
+			assert.NoError(t, err)
+			assert.Equal(t, s.expectedDidChange, didChange)
+			if didChange {
+				assert.Equal(t, s.expected, string(actual))
+			}
+			assert.Equal(t, s.expectedChanges, orderedMapToSlice(changes))
+		})
+	}
+}
+
 func TestCommitPrefixMigrations(t *testing.T) {
 	scenarios := []struct {
-		name     string
+		name              string
-		input    string
+		input             string
-		expected string
+		expected          string
+		expectedDidChange bool
+		expectedChanges   []string
 	}{
 		{
-			name:     "Empty String",
+			name:              "Empty String",
-			input:    "",
+			input:             "",
-			expected: "",
+			expectedDidChange: false,
-		}, {
+			expectedChanges:   []string{},
+		},
+		{
 			name: "Single CommitPrefix Rename",
 			input: `git:
   commitPrefix:
@@ -28,7 +185,10 @@ func TestCommitPrefixMigrations(t *testing.T) {
     - pattern: "^\\w+-\\w+.*"
       replace: '[JIRA $0] '
 `,
-		}, {
+			expectedDidChange: true,
+			expectedChanges:   []string{"Changed 'git.commitPrefix' to an array of strings"},
+		},
+		{
 			name: "Complicated CommitPrefixes Rename",
 			input: `git:
   commitPrefixes:
@@ -48,13 +208,16 @@ func TestCommitPrefixMigrations(t *testing.T) {
       - pattern: "^foo.bar*"
         replace: '[FUN $0] '
 `,
-		}, {
+			expectedDidChange: true,
-			name:     "Incomplete Configuration",
+			expectedChanges:   []string{"Changed 'git.commitPrefixes' elements to arrays of strings"},
-			input:    "git:",
+		},
-			expected: "git:",
+		{
-		}, {
+			name:              "Incomplete Configuration",
-			// This test intentionally uses non-standard indentation to test that the migration
+			input:             "git:",
-			// does not change the input.
+			expectedDidChange: false,
+			expectedChanges:   []string{},
+		},
+		{
 			name: "No changes made when already migrated",
 			input: `
 git:
@@ -65,40 +228,40 @@ git:
     foo:
       - pattern: "^\\w+-\\w+.*"
         replace: '[JIRA $0] '`,
-			expected: `
+			expectedDidChange: false,
-git:
+			expectedChanges:   []string{},
-   commitPrefix:
-    - pattern: "Hello World"
-      replace: "Goodbye"
-   commitPrefixes:
-    foo:
-      - pattern: "^\\w+-\\w+.*"
-        replace: '[JIRA $0] '`,
 		},
 	}
 
 	for _, s := range scenarios {
 		t.Run(s.name, func(t *testing.T) {
-			actual, err := computeMigratedConfig("path doesn't matter", []byte(s.input))
+			changes := orderedmap.New[string, bool]()
-			if err != nil {
+			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input), changes)
-				t.Error(err)
+			assert.NoError(t, err)
+			assert.Equal(t, s.expectedDidChange, didChange)
+			if didChange {
+				assert.Equal(t, s.expected, string(actual))
 			}
-			assert.Equal(t, s.expected, string(actual))
+			assert.Equal(t, s.expectedChanges, orderedMapToSlice(changes))
 		})
 	}
 }
 
 func TestCustomCommandsOutputMigration(t *testing.T) {
 	scenarios := []struct {
-		name     string
+		name              string
-		input    string
+		input             string
-		expected string
+		expected          string
+		expectedDidChange bool
+		expectedChanges   []string
 	}{
 		{
-			name:     "Empty String",
+			name:              "Empty String",
-			input:    "",
+			input:             "",
-			expected: "",
+			expectedDidChange: false,
-		}, {
+			expectedChanges:   []string{},
+		},
+		{
 			name: "Convert subprocess to output=terminal",
 			input: `customCommands:
   - command: echo 'hello'
@@ -108,7 +271,10 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
   - command: echo 'hello'
     output: terminal
 `,
-		}, {
+			expectedDidChange: true,
+			expectedChanges:   []string{"Changed 'subprocess: true' to 'output: terminal' in custom command"},
+		},
+		{
 			name: "Convert stream to output=log",
 			input: `customCommands:
   - command: echo 'hello'
@@ -118,7 +284,10 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
   - command: echo 'hello'
     output: log
 `,
-		}, {
+			expectedDidChange: true,
+			expectedChanges:   []string{"Changed 'stream: true' to 'output: log' in custom command"},
+		},
+		{
 			name: "Convert showOutput to output=popup",
 			input: `customCommands:
   - command: echo 'hello'
@@ -128,7 +297,10 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
   - command: echo 'hello'
     output: popup
 `,
-		}, {
+			expectedDidChange: true,
+			expectedChanges:   []string{"Changed 'showOutput: true' to 'output: popup' in custom command"},
+		},
+		{
 			name: "Subprocess wins over the other two",
 			input: `customCommands:
   - command: echo 'hello'
@@ -140,7 +312,14 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
   - command: echo 'hello'
     output: terminal
 `,
-		}, {
+			expectedDidChange: true,
+			expectedChanges: []string{
+				"Changed 'subprocess: true' to 'output: terminal' in custom command",
+				"Deleted redundant 'stream: true' property in custom command",
+				"Deleted redundant 'showOutput: true' property in custom command",
+			},
+		},
+		{
 			name: "Stream wins over showOutput",
 			input: `customCommands:
   - command: echo 'hello'
@@ -151,7 +330,13 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
   - command: echo 'hello'
     output: log
 `,
-		}, {
+			expectedDidChange: true,
+			expectedChanges: []string{
+				"Changed 'stream: true' to 'output: log' in custom command",
+				"Deleted redundant 'showOutput: true' property in custom command",
+			},
+		},
+		{
 			name: "Explicitly setting to false doesn't create an output=none key",
 			input: `customCommands:
   - command: echo 'hello'
@@ -162,14 +347,25 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
 			expected: `customCommands:
   - command: echo 'hello'
 `,
+			expectedDidChange: true,
+			expectedChanges: []string{
+				"Deleted redundant 'subprocess: false' in custom command",
+				"Deleted redundant 'stream: false' property in custom command",
+				"Deleted redundant 'showOutput: false' property in custom command",
+			},
 		},
 	}
 
 	for _, s := range scenarios {
 		t.Run(s.name, func(t *testing.T) {
-			actual, err := computeMigratedConfig("path doesn't matter", []byte(s.input))
+			changes := orderedmap.New[string, bool]()
+			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input), changes)
 			assert.NoError(t, err)
-			assert.Equal(t, s.expected, string(actual))
+			assert.Equal(t, s.expectedDidChange, didChange)
+			if didChange {
+				assert.Equal(t, s.expected, string(actual))
+			}
+			assert.Equal(t, s.expectedChanges, orderedMapToSlice(changes))
 		})
 	}
 }
@@ -778,21 +974,26 @@ keybinding:
 
 func BenchmarkMigrationOnLargeConfiguration(b *testing.B) {
 	for b.Loop() {
-		_, _ = computeMigratedConfig("path doesn't matter", largeConfiguration)
+		changes := orderedmap.New[string, bool]()
+		_, _, _ = computeMigratedConfig("path doesn't matter", largeConfiguration, changes)
 	}
 }
 
 func TestAllBranchesLogCmdMigrations(t *testing.T) {
 	scenarios := []struct {
-		name     string
+		name              string
-		input    string
+		input             string
-		expected string
+		expected          string
+		expectedDidChange bool
+		expectedChanges   []string
 	}{
 		{
-			name:     "Incomplete Configuration Passes uneventfully",
+			name:              "Incomplete Configuration Passes uneventfully",
-			input:    "git:",
+			input:             "git:",
-			expected: "git:",
+			expectedDidChange: false,
-		}, {
+			expectedChanges:   []string{},
+		},
+		{
 			name: "Single Cmd with no Cmds",
 			input: `git:
   allBranchesLogCmd: git log --graph --oneline
@@ -801,7 +1002,13 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
   allBranchesLogCmds:
     - git log --graph --oneline
 `,
-		}, {
+			expectedDidChange: true,
+			expectedChanges: []string{
+				"Created git.allBranchesLogCmds array containing value of git.allBranchesLogCmd",
+				"Removed obsolete git.allBranchesLogCmd",
+			},
+		},
+		{
 			name: "Cmd with one existing Cmds",
 			input: `git:
   allBranchesLogCmd: git log --graph --oneline
@@ -813,17 +1020,22 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
     - git log --graph --oneline
     - git log --graph --oneline --pretty
 `,
-		}, {
+			expectedDidChange: true,
+			expectedChanges: []string{
+				"Prepended git.allBranchesLogCmd value to git.allBranchesLogCmds array",
+				"Removed obsolete git.allBranchesLogCmd",
+			},
+		},
+		{
 			name: "Only Cmds set have no changes",
 			input: `git:
   allBranchesLogCmds:
     - git log
 `,
-			expected: `git:
+			expected:        "",
-  allBranchesLogCmds:
+			expectedChanges: []string{},
-    - git log
+		},
-`,
+		{
-		}, {
 			name: "Removes Empty Cmd When at end of yaml",
 			input: `git:
   allBranchesLogCmds:
@@ -834,7 +1046,10 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
   allBranchesLogCmds:
     - git log --graph --oneline
 `,
-		}, {
+			expectedDidChange: true,
+			expectedChanges:   []string{"Removed obsolete git.allBranchesLogCmd"},
+		},
+		{
 			name: "Migrates when sequence defined inline",
 			input: `git:
   allBranchesLogCmds: [foo, bar]
@@ -843,7 +1058,13 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
 			expected: `git:
   allBranchesLogCmds: [baz, foo, bar]
 `,
-		}, {
+			expectedDidChange: true,
+			expectedChanges: []string{
+				"Prepended git.allBranchesLogCmd value to git.allBranchesLogCmds array",
+				"Removed obsolete git.allBranchesLogCmd",
+			},
+		},
+		{
 			name: "Removes Empty Cmd With Keys Afterwards",
 			input: `git:
   allBranchesLogCmds:
@@ -856,14 +1077,21 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
     - git log --graph --oneline
   foo: bar
 `,
+			expectedDidChange: true,
+			expectedChanges:   []string{"Removed obsolete git.allBranchesLogCmd"},
 		},
 	}
 
 	for _, s := range scenarios {
 		t.Run(s.name, func(t *testing.T) {
-			actual, err := computeMigratedConfig("path doesn't matter", []byte(s.input))
+			changes := orderedmap.New[string, bool]()
+			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input), changes)
 			assert.NoError(t, err)
-			assert.Equal(t, s.expected, string(actual))
+			assert.Equal(t, s.expectedDidChange, didChange)
+			if didChange {
+				assert.Equal(t, s.expected, string(actual))
+			}
+			assert.Equal(t, s.expectedChanges, orderedMapToSlice(changes))
 		})
 	}
 }

pkg/utils/yaml_utils/yaml_utils.go

@@ -65,41 +65,37 @@ func transformNode(node *yaml.Node, path []string, transform func(node *yaml.Nod
 
 // Takes the root node of a yaml document, a path to a key, and a new name for the key.
 // Will rename the key to the new name if it exists, and do nothing otherwise.
-func RenameYamlKey(rootNode *yaml.Node, path []string, newKey string) error {
+func RenameYamlKey(rootNode *yaml.Node, path []string, newKey string) (error, bool) {
 	// Empty document: nothing to do.
 	if len(rootNode.Content) == 0 {
-		return nil
+		return nil, false
 	}
 
 	body := rootNode.Content[0]
 
-	if err := renameYamlKey(body, path, newKey); err != nil {
+	return renameYamlKey(body, path, newKey)
-		return err
-	}
-
-	return nil
 }
 
 // Recursive function to rename the YAML key.
-func renameYamlKey(node *yaml.Node, path []string, newKey string) error {
+func renameYamlKey(node *yaml.Node, path []string, newKey string) (error, bool) {
 	if node.Kind != yaml.MappingNode {
-		return errors.New("yaml node in path is not a dictionary")
+		return errors.New("yaml node in path is not a dictionary"), false
 	}
 
 	keyNode, valueNode := LookupKey(node, path[0])
 	if keyNode == nil {
-		return nil
+		return nil, false
 	}
 
 	// end of path reached: rename key
 	if len(path) == 1 {
 		// Check that new key doesn't exist yet
 		if newKeyNode, _ := LookupKey(node, newKey); newKeyNode != nil {
-			return fmt.Errorf("new key `%s' already exists", newKey)
+			return fmt.Errorf("new key `%s' already exists", newKey), false
 		}
 
 		keyNode.Value = newKey
-		return nil
+		return nil, true
 	}
 
 	return renameYamlKey(valueNode, path[1:], newKey)

pkg/utils/yaml_utils/yaml_utils_test.go

@@ -10,77 +10,85 @@ import (
 
 func TestRenameYamlKey(t *testing.T) {
 	tests := []struct {
-		name        string
+		name              string
-		in          string
+		in                string
-		path        []string
+		path              []string
-		newKey      string
+		newKey            string
-		expectedOut string
+		expectedOut       string
-		expectedErr string
+		expectedDidRename bool
+		expectedErr       string
 	}{
 		{
-			name:        "rename key",
+			name:              "rename key",
-			in:          "foo: 5\n",
+			in:                "foo: 5\n",
-			path:        []string{"foo"},
+			path:              []string{"foo"},
-			newKey:      "bar",
+			newKey:            "bar",
-			expectedOut: "bar: 5\n",
+			expectedOut:       "bar: 5\n",
-			expectedErr: "",
+			expectedDidRename: true,
+			expectedErr:       "",
 		},
 		{
-			name:        "rename key, nested",
+			name:              "rename key, nested",
-			in:          "foo:\n  bar: 5\n",
+			in:                "foo:\n  bar: 5\n",
-			path:        []string{"foo", "bar"},
+			path:              []string{"foo", "bar"},
-			newKey:      "baz",
+			newKey:            "baz",
-			expectedOut: "foo:\n  baz: 5\n",
+			expectedOut:       "foo:\n  baz: 5\n",
-			expectedErr: "",
+			expectedDidRename: true,
+			expectedErr:       "",
 		},
 		{
-			name:        "rename non-scalar key",
+			name:              "rename non-scalar key",
-			in:          "foo:\n  bar: 5\n",
+			in:                "foo:\n  bar: 5\n",
-			path:        []string{"foo"},
+			path:              []string{"foo"},
-			newKey:      "qux",
+			newKey:            "qux",
-			expectedOut: "qux:\n  bar: 5\n",
+			expectedOut:       "qux:\n  bar: 5\n",
-			expectedErr: "",
+			expectedDidRename: true,
+			expectedErr:       "",
 		},
 		{
-			name:        "don't rewrite file if value didn't change",
+			name:              "don't rewrite file if value didn't change",
-			in:          "foo:\n  bar: 5\n",
+			in:                "foo:\n  bar: 5\n",
-			path:        []string{"nonExistingKey"},
+			path:              []string{"nonExistingKey"},
-			newKey:      "qux",
+			newKey:            "qux",
-			expectedOut: "foo:\n  bar: 5\n",
+			expectedOut:       "foo:\n  bar: 5\n",
-			expectedErr: "",
+			expectedDidRename: false,
+			expectedErr:       "",
 		},
 
 		// Error cases
 		{
-			name:        "existing document is not a dictionary",
+			name:              "existing document is not a dictionary",
-			in:          "42\n",
+			in:                "42\n",
-			path:        []string{"foo"},
+			path:              []string{"foo"},
-			newKey:      "bar",
+			newKey:            "bar",
-			expectedOut: "42\n",
+			expectedOut:       "42\n",
-			expectedErr: "yaml node in path is not a dictionary",
+			expectedDidRename: false,
+			expectedErr:       "yaml node in path is not a dictionary",
 		},
 		{
-			name:        "not all path elements are dictionaries",
+			name:              "not all path elements are dictionaries",
-			in:          "foo:\n  bar: [1, 2, 3]\n",
+			in:                "foo:\n  bar: [1, 2, 3]\n",
-			path:        []string{"foo", "bar", "baz"},
+			path:              []string{"foo", "bar", "baz"},
-			newKey:      "qux",
+			newKey:            "qux",
-			expectedOut: "foo:\n  bar: [1, 2, 3]\n",
+			expectedOut:       "foo:\n  bar: [1, 2, 3]\n",
-			expectedErr: "yaml node in path is not a dictionary",
+			expectedDidRename: false,
+			expectedErr:       "yaml node in path is not a dictionary",
 		},
 		{
-			name:        "new key exists",
+			name:              "new key exists",
-			in:          "foo: 5\nbar: 7\n",
+			in:                "foo: 5\nbar: 7\n",
-			path:        []string{"foo"},
+			path:              []string{"foo"},
-			newKey:      "bar",
+			newKey:            "bar",
-			expectedOut: "foo: 5\nbar: 7\n",
+			expectedOut:       "foo: 5\nbar: 7\n",
-			expectedErr: "new key `bar' already exists",
+			expectedDidRename: false,
+			expectedErr:       "new key `bar' already exists",
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			node := unmarshalForTest(t, test.in)
-			actualErr := RenameYamlKey(&node, test.path, test.newKey)
+			actualErr, didRename := RenameYamlKey(&node, test.path, test.newKey)
 			if test.expectedErr == "" {
 				assert.NoError(t, actualErr)
 			} else {
@@ -89,6 +97,8 @@ func TestRenameYamlKey(t *testing.T) {
 			out := marshalForTest(t, &node)
 
 			assert.Equal(t, test.expectedOut, out)
+
+			assert.Equal(t, test.expectedDidRename, didRename)
 		})
 	}
 }