Log a list of migration changes to the console

This might be useful to see in general (users will normally only see it after
they quit lazygit again, but still). But it is especially useful when writing
back the config file fails for some reason, because users can then make these
changes manually if they want.

We do this only at startup, when the GUI hasn't started yet. This is probably
good enough, because it is much less likely that writing back a migrated
repo-local config fails because it is not writeable.
This commit is contained in:
Stefan Haller 2025-05-05 10:58:21 +02:00
parent bf95068b11
commit 7a2d5fcec9
3 changed files with 137 additions and 25 deletions

2
go.mod
View file

@ -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

View file

@ -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"
)
@ -220,7 +221,9 @@ func loadUserConfig(configFiles []*ConfigFile, base *UserConfig, isGuiInitialize
// 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 := orderedmap.New[string, bool]()
changedContent, didChange, err := computeMigratedConfig(path, content, changes)
if err != nil {
return nil, err
}
@ -230,21 +233,27 @@ func migrateUserConfig(path string, content []byte, isGuiInitialized bool) ([]by
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.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 *orderedmap.OrderedMap[string, bool]) ([]byte, bool, error) {
var err error
var rootNode yaml.Node
err = yaml.Unmarshal(content, &rootNode)
@ -267,33 +276,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.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, 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 +323,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 *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
@ -332,13 +345,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 {
@ -351,6 +366,7 @@ func changeCommitPrefixesMap(rootNode *yaml.Node) error {
Kind: yaml.MappingNode,
Content: nodeContentCopy,
}}
changes.Set("Changed 'git.commitPrefixes' elements to arrays of strings", true)
}
}
}
@ -358,7 +374,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
@ -369,16 +385,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 != "" {
@ -402,7 +427,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
@ -411,6 +436,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 +446,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.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
})

View file

@ -4,19 +4,30 @@ 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",
@ -24,6 +35,7 @@ func TestMigrationOfRenamedKeys(t *testing.T) {
bar: 5
`,
expectedDidChange: false,
expectedChanges: []string{},
},
{
name: "Rename one",
@ -34,6 +46,7 @@ func TestMigrationOfRenamedKeys(t *testing.T) {
skipDiscardChangeWarning: true
`,
expectedDidChange: true,
expectedChanges: []string{"Renamed 'gui.skipUnstageLineWarning' to 'skipDiscardChangeWarning'"},
},
{
name: "Rename several",
@ -52,17 +65,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 := 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))
})
}
}
@ -73,11 +93,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 +108,7 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) {
quit: q
`,
expectedDidChange: false,
expectedChanges: []string{},
},
{
name: "Change one",
@ -98,6 +121,7 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) {
quit: <disabled>
`,
expectedDidChange: true,
expectedChanges: []string{"Changed 'null' to '<disabled>' for keybinding 'keybinding.universal.quit'"},
},
{
name: "Change several",
@ -114,17 +138,23 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) {
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) {
actual, didChange, 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.expectedDidChange, didChange)
if didChange {
assert.Equal(t, s.expected, string(actual))
}
assert.Equal(t, s.expectedChanges, orderedMapToSlice(changes))
})
}
}
@ -135,11 +165,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 +186,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 +209,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 +229,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 := 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))
})
}
}
@ -215,11 +253,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 +272,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 +285,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 +298,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 +313,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 +331,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 +348,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 := 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))
})
}
}
@ -915,7 +974,8 @@ 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)
}
}
@ -925,11 +985,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 +1003,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 +1021,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 +1032,8 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
allBranchesLogCmds:
- git log
`,
expected: "",
expected: "",
expectedChanges: []string{},
},
{
name: "Removes Empty Cmd When at end of yaml",
@ -976,6 +1047,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 +1059,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 +1078,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 := 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))
})
}
}