diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index 010db02ef..560031cc1 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -11,6 +11,7 @@ import ( "time" "github.com/adrg/xdg" + "github.com/jesseduffield/generics/orderedset" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/jesseduffield/lazygit/pkg/utils/yaml_utils" "github.com/samber/lo" @@ -215,12 +216,20 @@ func loadUserConfig(configFiles []*ConfigFile, base *UserConfig, isGuiInitialize return base, nil } +type ChangesSet = orderedset.OrderedSet[string] + +func NewChangesSet() *ChangesSet { + return orderedset.New[string]() +} + // Do any backward-compatibility migrations of things that have changed in the // 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, isGuiInitialized bool) ([]byte, error) { - changedContent, didChange, err := computeMigratedConfig(path, content) + changes := NewChangesSet() + + changedContent, didChange, err := computeMigratedConfig(path, content, changes) if err != nil { return nil, err } @@ -230,21 +239,27 @@ func migrateUserConfig(path string, content []byte, isGuiInitialized bool) ([]by return content, nil } + changesText := "The following changes were made:\n\n" + changesText += strings.Join(lo.Map(changes.ToSliceFromOldest(), func(change string, _ int) string { + return fmt.Sprintf("- %s\n", change) + }), "") + // Write config back if !isGuiInitialized { - fmt.Println("Provided user config is deprecated but auto-fixable. Attempting to write fixed version back to file...") + 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 { return nil, fmt.Errorf("While attempting to write back fixed user config to %s, an error occurred: %s", path, err) } if !isGuiInitialized { - fmt.Printf("Success. New config written to %s\n", path) + 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, bool, error) { +func computeMigratedConfig(path string, content []byte, changes *ChangesSet) ([]byte, bool, error) { var err error var rootNode yaml.Node err = yaml.Unmarshal(content, &rootNode) @@ -267,33 +282,36 @@ func computeMigratedConfig(path string, content []byte) ([]byte, bool, 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, false, fmt.Errorf("Couldn't migrate config file at `%s` for key %s: %s", path, strings.Join(pathToReplace.oldPath, "."), err) } + if didReplace { + changes.Add(fmt.Sprintf("Renamed '%s' to '%s'", strings.Join(pathToReplace.oldPath, "."), pathToReplace.newName)) + } } - err = changeNullKeybindingsToDisabled(&rootNode) + err = changeNullKeybindingsToDisabled(&rootNode, changes) if err != nil { 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, 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, 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, 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, false, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err) } @@ -311,16 +329,17 @@ func computeMigratedConfig(path string, content []byte) ([]byte, bool, error) { return newContent, true, nil } -func changeNullKeybindingsToDisabled(rootNode *yaml.Node) error { +func changeNullKeybindingsToDisabled(rootNode *yaml.Node, changes *ChangesSet) 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 = "" node.Tag = "!!str" + changes.Add(fmt.Sprintf("Changed 'null' to '' for keybinding '%s'", path)) } }) } -func changeElementToSequence(rootNode *yaml.Node, path []string) error { +func changeElementToSequence(rootNode *yaml.Node, path []string, changes *ChangesSet) error { return yaml_utils.TransformNode(rootNode, path, func(node *yaml.Node) error { if node.Kind == yaml.MappingNode { nodeContentCopy := node.Content @@ -332,13 +351,15 @@ func changeElementToSequence(rootNode *yaml.Node, path []string) error { Content: nodeContentCopy, }} + changes.Add(fmt.Sprintf("Changed '%s' to an array of strings", strings.Join(path, "."))) + return nil } return nil }) } -func changeCommitPrefixesMap(rootNode *yaml.Node) error { +func changeCommitPrefixesMap(rootNode *yaml.Node, changes *ChangesSet) error { return yaml_utils.TransformNode(rootNode, []string{"git", "commitPrefixes"}, func(prefixesNode *yaml.Node) error { if prefixesNode.Kind == yaml.MappingNode { for _, contentNode := range prefixesNode.Content { @@ -351,6 +372,7 @@ func changeCommitPrefixesMap(rootNode *yaml.Node) error { Kind: yaml.MappingNode, Content: nodeContentCopy, }} + changes.Add("Changed 'git.commitPrefixes' elements to arrays of strings") } } } @@ -358,7 +380,7 @@ func changeCommitPrefixesMap(rootNode *yaml.Node) error { }) } -func changeCustomCommandStreamAndOutputToOutputEnum(rootNode *yaml.Node) error { +func changeCustomCommandStreamAndOutputToOutputEnum(rootNode *yaml.Node, changes *ChangesSet) 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 @@ -369,16 +391,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.Add("Changed 'subprocess: true' to 'output: terminal' in custom command") + } else { + changes.Add("Deleted redundant 'subprocess: false' in custom command") } } if streamKey, streamValue := yaml_utils.RemoveKey(node, "stream"); streamKey != nil { if streamValue.Kind == yaml.ScalarNode && streamValue.Value == "true" && output == "" { output = "log" + changes.Add("Changed 'stream: true' to 'output: log' in custom command") + } else { + changes.Add(fmt.Sprintf("Deleted redundant 'stream: %v' property in custom command", streamValue.Value)) } } if streamKey, streamValue := yaml_utils.RemoveKey(node, "showOutput"); streamKey != nil { if streamValue.Kind == yaml.ScalarNode && streamValue.Value == "true" && output == "" { + changes.Add("Changed 'showOutput: true' to 'output: popup' in custom command") output = "popup" + } else { + changes.Add(fmt.Sprintf("Deleted redundant 'showOutput: %v' property in custom command", streamValue.Value)) } } if output != "" { @@ -402,7 +433,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 *ChangesSet) 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 @@ -411,6 +442,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 @@ -420,17 +452,24 @@ func migrateAllBranchesLogCmd(rootNode *yaml.Node) error { cmdsKeyNode, cmdsValueNode, ) - } else if cmdsValueNode.Kind != yaml.SequenceNode { - return errors.New("You should have an allBranchesLogCmds defined as a sequence!") + change = "Created git.allBranchesLogCmds array containing value of git.allBranchesLogCmd" + } 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.Add(change) } // Clear out the existing allBranchesLogCmd, now that we have migrated it into the list _, _ = yaml_utils.RemoveKey(gitNode, "allBranchesLogCmd") + changes.Add("Removed obsolete git.allBranchesLogCmd") return nil }) diff --git a/pkg/config/app_config_test.go b/pkg/config/app_config_test.go index 6f15b6d4a..90c13ce6c 100644 --- a/pkg/config/app_config_test.go +++ b/pkg/config/app_config_test.go @@ -12,11 +12,13 @@ func TestMigrationOfRenamedKeys(t *testing.T) { input string expected string expectedDidChange bool + expectedChanges []string }{ { name: "Empty String", input: "", expectedDidChange: false, + expectedChanges: []string{}, }, { name: "No rename needed", @@ -24,6 +26,7 @@ func TestMigrationOfRenamedKeys(t *testing.T) { bar: 5 `, expectedDidChange: false, + expectedChanges: []string{}, }, { name: "Rename one", @@ -34,6 +37,7 @@ func TestMigrationOfRenamedKeys(t *testing.T) { skipDiscardChangeWarning: true `, expectedDidChange: true, + expectedChanges: []string{"Renamed 'gui.skipUnstageLineWarning' to 'skipDiscardChangeWarning'"}, }, { name: "Rename several", @@ -52,17 +56,24 @@ keybinding: 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) { - actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input)) + changes := NewChangesSet() + 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, changes.ToSliceFromOldest()) }) } } @@ -73,11 +84,13 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) { input string expected string expectedDidChange bool + expectedChanges []string }{ { name: "Empty String", input: "", expectedDidChange: false, + expectedChanges: []string{}, }, { name: "No change needed", @@ -86,6 +99,7 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) { quit: q `, expectedDidChange: false, + expectedChanges: []string{}, }, { name: "Change one", @@ -98,6 +112,7 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) { quit: `, expectedDidChange: true, + expectedChanges: []string{"Changed 'null' to '' for keybinding 'keybinding.universal.quit'"}, }, { name: "Change several", @@ -114,17 +129,23 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) { new: `, expectedDidChange: true, + expectedChanges: []string{ + "Changed 'null' to '' for keybinding 'keybinding.universal.quit'", + "Changed 'null' to '' for keybinding 'keybinding.universal.new'", + }, }, } for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input)) + changes := NewChangesSet() + 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, changes.ToSliceFromOldest()) }) } } @@ -135,11 +156,13 @@ func TestCommitPrefixMigrations(t *testing.T) { input string expected string expectedDidChange bool + expectedChanges []string }{ { name: "Empty String", input: "", expectedDidChange: false, + expectedChanges: []string{}, }, { name: "Single CommitPrefix Rename", @@ -154,6 +177,7 @@ func TestCommitPrefixMigrations(t *testing.T) { replace: '[JIRA $0] ' `, expectedDidChange: true, + expectedChanges: []string{"Changed 'git.commitPrefix' to an array of strings"}, }, { name: "Complicated CommitPrefixes Rename", @@ -176,11 +200,13 @@ func TestCommitPrefixMigrations(t *testing.T) { replace: '[FUN $0] ' `, expectedDidChange: true, + expectedChanges: []string{"Changed 'git.commitPrefixes' elements to arrays of strings"}, }, { name: "Incomplete Configuration", input: "git:", expectedDidChange: false, + expectedChanges: []string{}, }, { name: "No changes made when already migrated", @@ -194,17 +220,20 @@ git: - pattern: "^\\w+-\\w+.*" replace: '[JIRA $0] '`, expectedDidChange: false, + expectedChanges: []string{}, }, } for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input)) + changes := NewChangesSet() + 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, changes.ToSliceFromOldest()) }) } } @@ -215,11 +244,13 @@ func TestCustomCommandsOutputMigration(t *testing.T) { input string expected string expectedDidChange bool + expectedChanges []string }{ { name: "Empty String", input: "", expectedDidChange: false, + expectedChanges: []string{}, }, { name: "Convert subprocess to output=terminal", @@ -232,6 +263,7 @@ func TestCustomCommandsOutputMigration(t *testing.T) { output: terminal `, expectedDidChange: true, + expectedChanges: []string{"Changed 'subprocess: true' to 'output: terminal' in custom command"}, }, { name: "Convert stream to output=log", @@ -244,6 +276,7 @@ func TestCustomCommandsOutputMigration(t *testing.T) { output: log `, expectedDidChange: true, + expectedChanges: []string{"Changed 'stream: true' to 'output: log' in custom command"}, }, { name: "Convert showOutput to output=popup", @@ -256,6 +289,7 @@ func TestCustomCommandsOutputMigration(t *testing.T) { output: popup `, expectedDidChange: true, + expectedChanges: []string{"Changed 'showOutput: true' to 'output: popup' in custom command"}, }, { name: "Subprocess wins over the other two", @@ -270,6 +304,11 @@ func TestCustomCommandsOutputMigration(t *testing.T) { 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", @@ -283,6 +322,10 @@ func TestCustomCommandsOutputMigration(t *testing.T) { 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", @@ -296,17 +339,24 @@ func TestCustomCommandsOutputMigration(t *testing.T) { - 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, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input)) + changes := NewChangesSet() + 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, changes.ToSliceFromOldest()) }) } } @@ -915,7 +965,8 @@ keybinding: func BenchmarkMigrationOnLargeConfiguration(b *testing.B) { for b.Loop() { - _, _, _ = computeMigratedConfig("path doesn't matter", largeConfiguration) + changes := NewChangesSet() + _, _, _ = computeMigratedConfig("path doesn't matter", largeConfiguration, changes) } } @@ -925,11 +976,13 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) { input string expected string expectedDidChange bool + expectedChanges []string }{ { name: "Incomplete Configuration Passes uneventfully", input: "git:", expectedDidChange: false, + expectedChanges: []string{}, }, { name: "Single Cmd with no Cmds", @@ -941,6 +994,10 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) { - 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", @@ -955,6 +1012,10 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) { - 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", @@ -962,7 +1023,8 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) { allBranchesLogCmds: - git log `, - expected: "", + expected: "", + expectedChanges: []string{}, }, { name: "Removes Empty Cmd When at end of yaml", @@ -976,6 +1038,7 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) { - git log --graph --oneline `, expectedDidChange: true, + expectedChanges: []string{"Removed obsolete git.allBranchesLogCmd"}, }, { name: "Migrates when sequence defined inline", @@ -987,6 +1050,10 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) { 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", @@ -1002,17 +1069,20 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) { foo: bar `, expectedDidChange: true, + expectedChanges: []string{"Removed obsolete git.allBranchesLogCmd"}, }, } for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input)) + changes := NewChangesSet() + 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, changes.ToSliceFromOldest()) }) } }