Skip to content

Print migration changes to the console when migrating config file #4548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ require (
github.com/go-errors/errors v1.5.1
github.com/gookit/color v1.4.2
github.com/integrii/flaggy v1.4.0
github.com/jesseduffield/generics v0.0.0-20250406224309-4f541cb84918
github.com/jesseduffield/generics v0.0.0-20250517122708-b0b4a53a6f5c
github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd
github.com/jesseduffield/gocui v0.3.1-0.20250421160159-82c9aaeba2b9
github.com/jesseduffield/kill v0.0.0-20250101124109-e216ddbe133a
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ github.com/invopop/jsonschema v0.10.0 h1:c1ktzNLBun3LyQQhyty5WE3lulbOdIIyOVlkmDL
github.com/invopop/jsonschema v0.10.0/go.mod h1:ffZ5Km5SWWRAIN6wbDXItl95euhFz2uON45H2qjYt+0=
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A=
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo=
github.com/jesseduffield/generics v0.0.0-20250406224309-4f541cb84918 h1:meoUDZGF6jZAbhW5IBwj92mTqGmrOn+Cuu0jM7/aUcs=
github.com/jesseduffield/generics v0.0.0-20250406224309-4f541cb84918/go.mod h1:+LLj9/WUPAP8LqCchs7P+7X0R98HiFujVFANdNaxhGk=
github.com/jesseduffield/generics v0.0.0-20250517122708-b0b4a53a6f5c h1:tC2PaiisXAC5sOjDPfMArSnbswDObtCssx+xn28edX4=
github.com/jesseduffield/generics v0.0.0-20250517122708-b0b4a53a6f5c/go.mod h1:F2fEBk0ddf6ixrBrJjY7phfQ3hL9rXG0uSjvwYe50bE=
github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd h1:ViKj6qth8FgcIWizn9KiACWwPemWSymx62OPN0tHT+Q=
github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd/go.mod h1:lRhCiBr6XjQrvcQVa+UYsy/99d3wMXn/a0nSQlhnhlA=
github.com/jesseduffield/gocui v0.3.1-0.20250421160159-82c9aaeba2b9 h1:k23sCKHCNpAvwJP8Yr16CBUItuarmUHBGH7FaAm2glc=
Expand Down
144 changes: 96 additions & 48 deletions pkg/config/app_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -96,7 +97,7 @@ func NewAppConfig(
configFiles = []*ConfigFile{configFile}
}

userConfig, err := loadUserConfigWithDefaults(configFiles)
userConfig, err := loadUserConfigWithDefaults(configFiles, false)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -145,11 +146,11 @@ func findOrCreateConfigDir() (string, error) {
return folder, os.MkdirAll(folder, 0o755)
}

func loadUserConfigWithDefaults(configFiles []*ConfigFile) (*UserConfig, error) {
return loadUserConfig(configFiles, GetDefaultConfig())
func loadUserConfigWithDefaults(configFiles []*ConfigFile, isGuiInitialized bool) (*UserConfig, error) {
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)
Expand Down Expand Up @@ -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
}
Expand All @@ -215,41 +216,64 @@ func loadUserConfig(configFiles []*ConfigFile, base *UserConfig) (*UserConfig, e
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) ([]byte, error) {
changedContent, err := computeMigratedConfig(path, content)
func migrateUserConfig(path string, content []byte, isGuiInitialized bool) ([]byte, error) {
changes := NewChangesSet()

changedContent, didChange, err := computeMigratedConfig(path, content, changes)
if err != nil {
return nil, err
}

// Write config back if changed
if string(changedContent) != string(content) {
fmt.Println("Provided user config is deprecated but auto-fixable. Attempting to write fixed version back to file...")
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
// Nothing to do if config didn't change
if !didChange {
return content, nil
}

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.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 *ChangesSet) ([]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 {
Expand All @@ -262,60 +286,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.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, 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) {
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
if reflect.DeepEqual(rootNode, originalCopy) {
return nil, false, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are going to switch to returning nil in all cases when we don't make changes, could we just use a check against changedContent == nill instead?

No strong preference for that, I just thought of the idea

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, that's what I initially did. It has the theoretical problem that if we ever write a migrator that only removes a key without replacement, then you can't distinguish the case that nothing was done from the case that the config file only contained that key, and we removed it. (You might argue that the Yaml library will probably return an empty bytes slice instead of nil in that case, so we could still distinguish that, but I'm actually not sure if that's really the case, and I don't think we should rely on it.)

The extra bool is a bit clearer and safer.

}

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 *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 = "<disabled>"
node.Tag = "!!str"
changes.Add(fmt.Sprintf("Changed 'null' to '<disabled>' 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
Expand All @@ -327,13 +355,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 {
Expand All @@ -346,14 +376,15 @@ func changeCommitPrefixesMap(rootNode *yaml.Node) error {
Kind: yaml.MappingNode,
Content: nodeContentCopy,
}}
changes.Add("Changed 'git.commitPrefixes' elements to arrays of strings")
}
}
}
return nil
})
}

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
Expand All @@ -364,16 +395,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 != "" {
Expand All @@ -397,7 +437,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
Expand All @@ -406,6 +446,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
Expand All @@ -415,17 +456,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
})
Expand Down Expand Up @@ -471,7 +519,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
}
Expand All @@ -496,7 +544,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
}
Expand Down
Loading