From 0138b9efbd5cc445be5b340ce120cb5ce1de809a Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 10 May 2025 07:39:44 +0200 Subject: [PATCH] fixup! Log a list of migration changes to the console --- pkg/config/app_config.go | 52 +++++++++++++++++++---------------- pkg/config/app_config_test.go | 31 ++++++++------------- 2 files changed, 40 insertions(+), 43 deletions(-) diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index eb3315383..e3bc4cd40 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -11,10 +11,10 @@ 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" - orderedmap "github.com/wk8/go-ordered-map/v2" "gopkg.in/yaml.v3" ) @@ -216,12 +216,18 @@ 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) { - changes := orderedmap.New[string, bool]() + changes := NewChangesSet() changedContent, didChange, err := computeMigratedConfig(path, content, changes) if err != nil { @@ -234,9 +240,9 @@ func migrateUserConfig(path string, content []byte, isGuiInitialized bool) ([]by } changesText := "The following changes were made:\n\n" - for pair := changes.Oldest(); pair != nil; pair = pair.Next() { - changesText += fmt.Sprintf("- %s\n", pair.Key) - } + changesText += strings.Join(lo.Map(changes.ToSliceFromOldest(), func(change string, _ int) string { + return fmt.Sprintf("- %s\n", change) + }), "") // Write config back if !isGuiInitialized { @@ -257,7 +263,7 @@ func migrateUserConfig(path string, content []byte, isGuiInitialized bool) ([]by } // A pure function helper for testing purposes -func computeMigratedConfig(path string, content []byte, changes *orderedmap.OrderedMap[string, bool]) ([]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) @@ -285,7 +291,7 @@ func computeMigratedConfig(path string, content []byte, changes *orderedmap.Orde 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) + changes.Add(fmt.Sprintf("Renamed '%s' to '%s'", strings.Join(pathToReplace.oldPath, "."), pathToReplace.newName)) } } @@ -327,17 +333,17 @@ func computeMigratedConfig(path string, content []byte, changes *orderedmap.Orde return newContent, true, nil } -func changeNullKeybindingsToDisabled(rootNode *yaml.Node, changes *orderedmap.OrderedMap[string, bool]) 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.Set(fmt.Sprintf("Changed 'null' to '' for keybinding '%s'", path), true) + changes.Add(fmt.Sprintf("Changed 'null' to '' for keybinding '%s'", path)) } }) } -func changeElementToSequence(rootNode *yaml.Node, path []string, changes *orderedmap.OrderedMap[string, bool]) 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 @@ -349,7 +355,7 @@ func changeElementToSequence(rootNode *yaml.Node, path []string, changes *ordere Content: nodeContentCopy, }} - changes.Set(fmt.Sprintf("Changed '%s' to an array of strings", strings.Join(path, ".")), true) + changes.Add(fmt.Sprintf("Changed '%s' to an array of strings", strings.Join(path, "."))) return nil } @@ -357,7 +363,7 @@ func changeElementToSequence(rootNode *yaml.Node, path []string, changes *ordere }) } -func changeCommitPrefixesMap(rootNode *yaml.Node, changes *orderedmap.OrderedMap[string, bool]) 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 { @@ -370,7 +376,7 @@ func changeCommitPrefixesMap(rootNode *yaml.Node, changes *orderedmap.OrderedMap Kind: yaml.MappingNode, Content: nodeContentCopy, }} - changes.Set("Changed 'git.commitPrefixes' elements to arrays of strings", true) + changes.Add("Changed 'git.commitPrefixes' elements to arrays of strings") } } } @@ -378,7 +384,7 @@ func changeCommitPrefixesMap(rootNode *yaml.Node, changes *orderedmap.OrderedMap }) } -func changeCustomCommandStreamAndOutputToOutputEnum(rootNode *yaml.Node, changes *orderedmap.OrderedMap[string, bool]) 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 @@ -389,25 +395,25 @@ func changeCustomCommandStreamAndOutputToOutputEnum(rootNode *yaml.Node, changes 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) + changes.Add("Changed 'subprocess: true' to 'output: terminal' in custom command") } else { - changes.Set("Deleted redundant 'subprocess: false' in custom command", true) + 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.Set("Changed 'stream: true' to 'output: log' in custom command", true) + changes.Add("Changed 'stream: true' to 'output: log' in custom command") } else { - changes.Set(fmt.Sprintf("Deleted redundant 'stream: %v' property in custom command", streamValue.Value), true) + 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.Set("Changed 'showOutput: true' to 'output: popup' in custom command", true) + changes.Add("Changed 'showOutput: true' to 'output: popup' in custom command") output = "popup" } else { - changes.Set(fmt.Sprintf("Deleted redundant 'showOutput: %v' property in custom command", streamValue.Value), true) + changes.Add(fmt.Sprintf("Deleted redundant 'showOutput: %v' property in custom command", streamValue.Value)) } } if output != "" { @@ -431,7 +437,7 @@ func changeCustomCommandStreamAndOutputToOutputEnum(rootNode *yaml.Node, changes // 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, changes *orderedmap.OrderedMap[string, bool]) 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 @@ -462,12 +468,12 @@ func migrateAllBranchesLogCmd(rootNode *yaml.Node, changes *orderedmap.OrderedMa 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) + changes.Add(change) } // 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) + 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 0a2843741..90c13ce6c 100644 --- a/pkg/config/app_config_test.go +++ b/pkg/config/app_config_test.go @@ -4,17 +4,8 @@ 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 @@ -75,14 +66,14 @@ keybinding: for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - changes := orderedmap.New[string, bool]() + 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, orderedMapToSlice(changes)) + assert.Equal(t, s.expectedChanges, changes.ToSliceFromOldest()) }) } } @@ -147,14 +138,14 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) { for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - changes := orderedmap.New[string, bool]() + 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, orderedMapToSlice(changes)) + assert.Equal(t, s.expectedChanges, changes.ToSliceFromOldest()) }) } } @@ -235,14 +226,14 @@ git: for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - changes := orderedmap.New[string, bool]() + 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, orderedMapToSlice(changes)) + assert.Equal(t, s.expectedChanges, changes.ToSliceFromOldest()) }) } } @@ -358,14 +349,14 @@ func TestCustomCommandsOutputMigration(t *testing.T) { for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - changes := orderedmap.New[string, bool]() + 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, orderedMapToSlice(changes)) + assert.Equal(t, s.expectedChanges, changes.ToSliceFromOldest()) }) } } @@ -974,7 +965,7 @@ keybinding: func BenchmarkMigrationOnLargeConfiguration(b *testing.B) { for b.Loop() { - changes := orderedmap.New[string, bool]() + changes := NewChangesSet() _, _, _ = computeMigratedConfig("path doesn't matter", largeConfiguration, changes) } } @@ -1084,14 +1075,14 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) { for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - changes := orderedmap.New[string, bool]() + 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, orderedMapToSlice(changes)) + assert.Equal(t, s.expectedChanges, changes.ToSliceFromOldest()) }) } }