From f0c591ee583f648240a215e336efe71ad847a20d Mon Sep 17 00:00:00 2001 From: Chris McDonnell Date: Tue, 25 Feb 2025 23:44:37 -0500 Subject: [PATCH] Migrate deprecated AllBranchesLogCmd to AllBranchesLogCmds --- docs/Config.md | 3 +- pkg/commands/git_commands/branch.go | 6 +- pkg/config/app_config.go | 43 ++++++++++++++ pkg/config/app_config_test.go | 79 +++++++++++++++++++++++++ pkg/config/user_config.go | 5 +- pkg/integration/tests/status/log_cmd.go | 3 +- pkg/utils/yaml_utils/yaml_utils.go | 23 +++++-- schema/config.json | 10 ++-- 8 files changed, 149 insertions(+), 23 deletions(-) diff --git a/docs/Config.md b/docs/Config.md index e9a8dab2a0d..4b5b702daf4 100644 --- a/docs/Config.md +++ b/docs/Config.md @@ -350,7 +350,8 @@ git: branchLogCmd: git log --graph --color=always --abbrev-commit --decorate --date=relative --pretty=medium {{branchName}} -- # Commands used to display git log of all branches in the main window, they will be cycled in order of appearance (array of strings) - allBranchesLogCmds: [] + allBranchesLogCmds: + - git log --graph --all --color=always --abbrev-commit --decorate --date=relative --pretty=medium # If true, do not spawn a separate process when using GPG overrideGpg: false diff --git a/pkg/commands/git_commands/branch.go b/pkg/commands/git_commands/branch.go index 85408f1d90c..553f518c5ef 100644 --- a/pkg/commands/git_commands/branch.go +++ b/pkg/commands/git_commands/branch.go @@ -248,11 +248,7 @@ func (self *BranchCommands) Merge(branchName string, opts MergeOpts) error { func (self *BranchCommands) AllBranchesLogCmdObj() oscommands.ICmdObj { // Only choose between non-empty, non-identical commands - candidates := lo.Uniq(lo.WithoutEmpty(append([]string{ - self.UserConfig().Git.AllBranchesLogCmd, - }, - self.UserConfig().Git.AllBranchesLogCmds..., - ))) + candidates := lo.Uniq(lo.WithoutEmpty(self.UserConfig().Git.AllBranchesLogCmds)) n := len(candidates) diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index 12b377f96ff..1bc0a8cf81a 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -281,6 +281,11 @@ func computeMigratedConfig(path string, content []byte) ([]byte, error) { return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err) } + err = migrateAllBranchesLogCmd(&rootNode) + if err != nil { + return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err) + } + // Add more migrations here... if !reflect.DeepEqual(rootNode, originalCopy) { @@ -341,6 +346,44 @@ func changeCommitPrefixesMap(rootNode *yaml.Node) error { }) } +// This migration is special because users have already defined +// 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 { + 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 + if cmdKeyNode == nil { + return nil + } + + cmdsKeyNode, cmdsValueNode := yaml_utils.LookupKey(gitNode, "allBranchesLogCmds") + if cmdsKeyNode == nil { + // Create dummy node and attach it onto the root git node + cmdsKeyNode = &yaml.Node{Kind: yaml.ScalarNode, Value: "allBranchesLogCmds"} + cmdsValueNode = &yaml.Node{Kind: yaml.SequenceNode, Content: []*yaml.Node{}} + gitNode.Content = append(gitNode.Content, + cmdsKeyNode, + cmdsValueNode, + ) + } + + if cmdValueNode.Value != "" { + if cmdsValueNode.Kind != yaml.SequenceNode { + return fmt.Errorf("You should have an allBranchesLogCmds defined as a sequence!") + } + // Prepending the individual element to make it show up first in the list, which was prior behavior + cmdsValueNode.Content = append([]*yaml.Node{{Kind: yaml.ScalarNode, Value: cmdValueNode.Value}}, cmdsValueNode.Content...) + } + + // Clear out the existing allBranchesLogCmd, now that we have migrated it into the list + _, _ = yaml_utils.RemoveKey(gitNode, "allBranchesLogCmd") + + return nil + }) +} + func (c *AppConfig) GetDebug() bool { return c.debug } diff --git a/pkg/config/app_config_test.go b/pkg/config/app_config_test.go index 9f9abc38a94..780b87da590 100644 --- a/pkg/config/app_config_test.go +++ b/pkg/config/app_config_test.go @@ -695,3 +695,82 @@ func BenchmarkMigrationOnLargeConfiguration(b *testing.B) { _, _ = computeMigratedConfig("path doesn't matter", largeConfiguration) } } + +func TestAllBranchesLogCmdMigrations(t *testing.T) { + scenarios := []struct { + name string + input string + expected string + }{ + { + name: "Incomplete Configuration Passes uneventfully", + input: "git:", + expected: "git:", + }, { + name: "Single Cmd with no Cmds", + input: `git: + allBranchesLogCmd: git log --graph --oneline +`, + expected: `git: + allBranchesLogCmds: + - git log --graph --oneline +`, + }, { + name: "Cmd with one existing Cmds", + input: `git: + allBranchesLogCmd: git log --graph --oneline + allBranchesLogCmds: + - git log --graph --oneline --pretty +`, + expected: `git: + allBranchesLogCmds: + - git log --graph --oneline + - git log --graph --oneline --pretty +`, + }, { + name: "Only Cmds set have no changes", + input: `git: + allBranchesLogCmds: + - git log +`, + expected: `git: + allBranchesLogCmds: + - git log +`, + }, { + name: "Removes Empty Cmd When at end of yaml", + input: `git: + allBranchesLogCmds: + - git log --graph --oneline + allBranchesLogCmd: +`, + expected: `git: + allBranchesLogCmds: + - git log --graph --oneline +`, + }, { + name: "Removes Empty Cmd With Keys Afterwards", + input: `git: + allBranchesLogCmds: + - git log --graph --oneline + allBranchesLogCmd: + foo: bar +`, + expected: `git: + allBranchesLogCmds: + - git log --graph --oneline + foo: bar +`, + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + actual, err := computeMigratedConfig("path doesn't matter", []byte(s.input)) + if err != nil { + t.Error(err) + } + assert.Equal(t, s.expected, string(actual)) + }) + } +} diff --git a/pkg/config/user_config.go b/pkg/config/user_config.go index fd682a9cb5a..209f0dda0e9 100644 --- a/pkg/config/user_config.go +++ b/pkg/config/user_config.go @@ -253,9 +253,6 @@ type GitConfig struct { AutoStageResolvedConflicts bool `yaml:"autoStageResolvedConflicts"` // Command used when displaying the current branch git log in the main window BranchLogCmd string `yaml:"branchLogCmd"` - // Command used to display git log of all branches in the main window. - // Deprecated: Use `allBranchesLogCmds` instead. - AllBranchesLogCmd string `yaml:"allBranchesLogCmd" jsonschema:"deprecated"` // Commands used to display git log of all branches in the main window, they will be cycled in order of appearance (array of strings) AllBranchesLogCmds []string `yaml:"allBranchesLogCmds"` // If true, do not spawn a separate process when using GPG @@ -823,7 +820,7 @@ func GetDefaultConfig() *UserConfig { FetchAll: true, AutoStageResolvedConflicts: true, BranchLogCmd: "git log --graph --color=always --abbrev-commit --decorate --date=relative --pretty=medium {{branchName}} --", - AllBranchesLogCmd: "git log --graph --all --color=always --abbrev-commit --decorate --date=relative --pretty=medium", + AllBranchesLogCmds: []string{"git log --graph --all --color=always --abbrev-commit --decorate --date=relative --pretty=medium"}, DisableForcePushing: false, CommitPrefixes: map[string][]CommitPrefixConfig(nil), BranchPrefix: "", diff --git a/pkg/integration/tests/status/log_cmd.go b/pkg/integration/tests/status/log_cmd.go index eb421f6fe98..03dd1a405d7 100644 --- a/pkg/integration/tests/status/log_cmd.go +++ b/pkg/integration/tests/status/log_cmd.go @@ -10,8 +10,7 @@ var LogCmd = NewIntegrationTest(NewIntegrationTestArgs{ ExtraCmdArgs: []string{}, Skip: false, SetupConfig: func(config *config.AppConfig) { - config.GetUserConfig().Git.AllBranchesLogCmd = `echo "view1"` - config.GetUserConfig().Git.AllBranchesLogCmds = []string{`echo "view2"`} + config.GetUserConfig().Git.AllBranchesLogCmds = []string{`echo "view1"`, `echo "view2"`} }, SetupRepo: func(shell *Shell) {}, Run: func(t *TestDriver, keys config.KeybindingConfig) { diff --git a/pkg/utils/yaml_utils/yaml_utils.go b/pkg/utils/yaml_utils/yaml_utils.go index 956805691e7..33816c7751c 100644 --- a/pkg/utils/yaml_utils/yaml_utils.go +++ b/pkg/utils/yaml_utils/yaml_utils.go @@ -61,7 +61,7 @@ func updateYamlNode(node *yaml.Node, path []string, value string) (bool, error) } key := path[0] - if _, valueNode := lookupKey(node, key); valueNode != nil { + if _, valueNode := LookupKey(node, key); valueNode != nil { return updateYamlNode(valueNode, path[1:], value) } @@ -90,7 +90,7 @@ func updateYamlNode(node *yaml.Node, path []string, value string) (bool, error) return updateYamlNode(newNode, path[1:], value) } -func lookupKey(node *yaml.Node, key string) (*yaml.Node, *yaml.Node) { +func LookupKey(node *yaml.Node, key string) (*yaml.Node, *yaml.Node) { for i := 0; i < len(node.Content)-1; i += 2 { if node.Content[i].Value == key { return node.Content[i], node.Content[i+1] @@ -100,6 +100,19 @@ func lookupKey(node *yaml.Node, key string) (*yaml.Node, *yaml.Node) { return nil, nil } +// Returns the key and value if they were present +func RemoveKey(node *yaml.Node, key string) (*yaml.Node, *yaml.Node) { + for i := 0; i < len(node.Content)-1; i += 2 { + if node.Content[i].Value == key { + key, value := node.Content[i], node.Content[i+1] + node.Content = append(node.Content[:i], node.Content[i+2:]...) + return key, value + } + } + + return nil, nil +} + // Walks a yaml document from the root node to the specified path, and then applies the transformation to that node. // If the requested path is not defined in the document, no changes are made to the document. func TransformNode(rootNode *yaml.Node, path []string, transform func(node *yaml.Node) error) error { @@ -123,7 +136,7 @@ func transformNode(node *yaml.Node, path []string, transform func(node *yaml.Nod return transform(node) } - keyNode, valueNode := lookupKey(node, path[0]) + keyNode, valueNode := LookupKey(node, path[0]) if keyNode == nil { return nil } @@ -154,7 +167,7 @@ func renameYamlKey(node *yaml.Node, path []string, newKey string) error { return errors.New("yaml node in path is not a dictionary") } - keyNode, valueNode := lookupKey(node, path[0]) + keyNode, valueNode := LookupKey(node, path[0]) if keyNode == nil { return nil } @@ -162,7 +175,7 @@ func renameYamlKey(node *yaml.Node, path []string, newKey string) error { // 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 { + if newKeyNode, _ := LookupKey(node, newKey); newKeyNode != nil { return fmt.Errorf("new key `%s' already exists", newKey) } diff --git a/schema/config.json b/schema/config.json index 7c642af96e7..a5f4b2b09f8 100644 --- a/schema/config.json +++ b/schema/config.json @@ -340,17 +340,15 @@ "description": "Command used when displaying the current branch git log in the main window", "default": "git log --graph --color=always --abbrev-commit --decorate --date=relative --pretty=medium {{branchName}} --" }, - "allBranchesLogCmd": { - "type": "string", - "description": "Command used to display git log of all branches in the main window.\nDeprecated: Use `allBranchesLogCmds` instead.", - "default": "git log --graph --all --color=always --abbrev-commit --decorate --date=relative --pretty=medium" - }, "allBranchesLogCmds": { "items": { "type": "string" }, "type": "array", - "description": "Commands used to display git log of all branches in the main window, they will be cycled in order of appearance (array of strings)" + "description": "Commands used to display git log of all branches in the main window, they will be cycled in order of appearance (array of strings)", + "default": [ + "git log --graph --all --color=always --abbrev-commit --decorate --date=relative --pretty=medium" + ] }, "overrideGpg": { "type": "boolean",