Migrate deprecated AllBranchesLogCmd to AllBranchesLogCmds (#4345)

- **PR Description**

Fixes #3961

Their issue where the default `allBranchesLogCmd` default remains
present is because we just do a `lo.Uniq(lo.WithoutEmpty())` on the
combined list of `allBranchesLogCmd` and `allBranchesLogCmds`.

At the point of this code, it is not possible to tell whether the value
present in `allBranchesLogCmd` is user-provided or not. We have already
merged the config with the default config, so the user not setting
anything, and the user explicitly setting "Yes, I want the default", are
indistinguishable.

Based on that bug report, I'm assuming that users that have not set
anything for `allBranchesLogCmd`, but _have_ set something for
`allBranchesLogCmds`, just want the list they have specified in the
plural version. Some users have likely figured out they can explicitly
set `allBranchesLogCmd: ""` to get this behavior, but most would not.

To achieve this desired behavior, I figure it is easiest to just migrate
all user config to `allBranchesLogCmds`. If they have explicitly set a
non-empty value in `allBranchesLogCmd`, it will be pulled over. If they
set an empty string, it will be excluded.

- **Please check if the PR fulfills these requirements**

* [X] Cheatsheets are up-to-date (run `go generate ./...`)
* [X] Code has been formatted (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#code-formatting))
* [X] Tests have been added/updated (see
[here](https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md)
for the integration test guide)
* [ ] Text is internationalised (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation))
* [ ] If a new UserConfig entry was added, make sure it can be
hot-reloaded (see
[here](https://github.com/jesseduffield/lazygit/blob/master/docs/dev/Codebase_Guide.md#using-userconfig))
* [X] Docs have been updated if necessary
* [X] You've read through your own file changes for silly mistakes etc

Author: stefanhaller

docs/Config.md

@@ -354,7 +354,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

pkg/commands/git_commands/branch.go

@@ -257,11 +257,7 @@ func (self *BranchCommands) Merge(branchName string, opts MergeOpts) error {
 
 func (self *BranchCommands) AllBranchesLogCmdObj() *oscommands.CmdObj {
 	// 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)
 

pkg/config/app_config.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"errors"
 	"fmt"
 	"log"
 	"os"
@@ -10,6 +11,7 @@ import (
 	"time"
 
 	"github.com/adrg/xdg"
+	"github.com/jesseduffield/lazygit/pkg/utils"
 	"github.com/jesseduffield/lazygit/pkg/utils/yaml_utils"
 	"github.com/samber/lo"
 	"gopkg.in/yaml.v3"
@@ -286,6 +288,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) {
@@ -386,6 +393,44 @@ func changeCustomCommandStreamAndOutputToOutputEnum(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 empty sequence node and attach it onto the root git node
+			// We will later populate it with the individual allBranchesLogCmd record
+			cmdsKeyNode = &yaml.Node{Kind: yaml.ScalarNode, Value: "allBranchesLogCmds"}
+			cmdsValueNode = &yaml.Node{Kind: yaml.SequenceNode, Content: []*yaml.Node{}}
+			gitNode.Content = append(gitNode.Content,
+				cmdsKeyNode,
+				cmdsValueNode,
+			)
+		} else if cmdsValueNode.Kind != yaml.SequenceNode {
+			return errors.New("You should have an allBranchesLogCmds defined as a sequence!")
+		}
+
+		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})
+		}
+
+		// 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
 }

pkg/config/app_config_test.go

@@ -781,3 +781,89 @@ 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: "Migrates when sequence defined inline",
+			input: `git:
+  allBranchesLogCmds: [foo, bar]
+  allBranchesLogCmd: baz
+`,
+			expected: `git:
+  allBranchesLogCmds: [baz, foo, bar]
+`,
+		}, {
+			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))
+			assert.NoError(t, err)
+			assert.Equal(t, s.expected, string(actual))
+		})
+	}
+}

pkg/config/user_config.go

@@ -256,9 +256,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:                 "",

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) {

pkg/utils/yaml_utils/yaml_utils.go

@@ -9,7 +9,7 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-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]
@@ -55,7 +55,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
 	}
@@ -86,15 +86,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)
 		}
 

schema/config.json

@@ -349,17 +349,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",