Migrate deprecated AllBranchesLogCmd to AllBranchesLogCmds

Author: Chris McDonnell

Diff for: docs/Config.md

@@ -335,9 +335,9 @@ git:
   # Command used when displaying the current branch git log in the main window
   branchLogCmd: git log --graph --color=always --abbrev-commit --decorate --date=relative --pretty=medium {{branchName}} --
 
-  # Command used to display git log of all branches in the main window.
-  # Deprecated: Use `allBranchesLogCmds` instead.
-  allBranchesLogCmd: git log --graph --all --color=always --abbrev-commit --decorate --date=relative  --pretty=medium
+  # Commands used to display git log of all branches in the main window, they will be cycled in order of appearance
+  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 for: 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 for: 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 elemnt 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 for: 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 for: pkg/config/user_config.go

@@ -249,9 +249,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"`
 	// Commands used to display git log of all branches in the main window, they will be cycled in order of appearance
 	AllBranchesLogCmds []string `yaml:"allBranchesLogCmds"`
 	// If true, do not spawn a separate process when using GPG
@@ -801,7 +798,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 for: 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 for: 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,15 +167,15 @@ 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
 	}
 
 	// 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 for: schema/config.json

@@ -319,17 +319,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"
+          "description": "Commands used to display git log of all branches in the main window, they will be cycled in order of appearance",
+          "default": [
+            "git log --graph --all --color=always --abbrev-commit --decorate --date=relative  --pretty=medium"
+          ]
         },
         "overrideGpg": {
           "type": "boolean",