Skip to content

Commit f0c591e

Browse files
author
Chris McDonnell
committed
Migrate deprecated AllBranchesLogCmd to AllBranchesLogCmds
1 parent 3916549 commit f0c591e

File tree

8 files changed

+149
-23
lines changed

8 files changed

+149
-23
lines changed

Diff for: ‎docs/Config.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,8 @@ git:
350350
branchLogCmd: git log --graph --color=always --abbrev-commit --decorate --date=relative --pretty=medium {{branchName}} --
351351

352352
# Commands used to display git log of all branches in the main window, they will be cycled in order of appearance (array of strings)
353-
allBranchesLogCmds: []
353+
allBranchesLogCmds:
354+
- git log --graph --all --color=always --abbrev-commit --decorate --date=relative --pretty=medium
354355

355356
# If true, do not spawn a separate process when using GPG
356357
overrideGpg: false

Diff for: ‎pkg/commands/git_commands/branch.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -248,11 +248,7 @@ func (self *BranchCommands) Merge(branchName string, opts MergeOpts) error {
248248

249249
func (self *BranchCommands) AllBranchesLogCmdObj() oscommands.ICmdObj {
250250
// Only choose between non-empty, non-identical commands
251-
candidates := lo.Uniq(lo.WithoutEmpty(append([]string{
252-
self.UserConfig().Git.AllBranchesLogCmd,
253-
},
254-
self.UserConfig().Git.AllBranchesLogCmds...,
255-
)))
251+
candidates := lo.Uniq(lo.WithoutEmpty(self.UserConfig().Git.AllBranchesLogCmds))
256252

257253
n := len(candidates)
258254

Diff for: ‎pkg/config/app_config.go

+43
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,11 @@ func computeMigratedConfig(path string, content []byte) ([]byte, error) {
281281
return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
282282
}
283283

284+
err = migrateAllBranchesLogCmd(&rootNode)
285+
if err != nil {
286+
return nil, fmt.Errorf("Couldn't migrate config file at `%s`: %s", path, err)
287+
}
288+
284289
// Add more migrations here...
285290

286291
if !reflect.DeepEqual(rootNode, originalCopy) {
@@ -341,6 +346,44 @@ func changeCommitPrefixesMap(rootNode *yaml.Node) error {
341346
})
342347
}
343348

349+
// This migration is special because users have already defined
350+
// a single element at `allBranchesLogCmd` and the sequence at `allBranchesLogCmds`.
351+
// Some users have explicitly set `allBranchesLogCmd` to be an empty string in order
352+
// to remove it, so in that case we just delete the element, and add nothing to the list
353+
func migrateAllBranchesLogCmd(rootNode *yaml.Node) error {
354+
return yaml_utils.TransformNode(rootNode, []string{"git"}, func(gitNode *yaml.Node) error {
355+
cmdKeyNode, cmdValueNode := yaml_utils.LookupKey(gitNode, "allBranchesLogCmd")
356+
// Nothing to do if they do not have the deprecated item
357+
if cmdKeyNode == nil {
358+
return nil
359+
}
360+
361+
cmdsKeyNode, cmdsValueNode := yaml_utils.LookupKey(gitNode, "allBranchesLogCmds")
362+
if cmdsKeyNode == nil {
363+
// Create dummy node and attach it onto the root git node
364+
cmdsKeyNode = &yaml.Node{Kind: yaml.ScalarNode, Value: "allBranchesLogCmds"}
365+
cmdsValueNode = &yaml.Node{Kind: yaml.SequenceNode, Content: []*yaml.Node{}}
366+
gitNode.Content = append(gitNode.Content,
367+
cmdsKeyNode,
368+
cmdsValueNode,
369+
)
370+
}
371+
372+
if cmdValueNode.Value != "" {
373+
if cmdsValueNode.Kind != yaml.SequenceNode {
374+
return fmt.Errorf("You should have an allBranchesLogCmds defined as a sequence!")
375+
}
376+
// Prepending the individual element to make it show up first in the list, which was prior behavior
377+
cmdsValueNode.Content = append([]*yaml.Node{{Kind: yaml.ScalarNode, Value: cmdValueNode.Value}}, cmdsValueNode.Content...)
378+
}
379+
380+
// Clear out the existing allBranchesLogCmd, now that we have migrated it into the list
381+
_, _ = yaml_utils.RemoveKey(gitNode, "allBranchesLogCmd")
382+
383+
return nil
384+
})
385+
}
386+
344387
func (c *AppConfig) GetDebug() bool {
345388
return c.debug
346389
}

Diff for: ‎pkg/config/app_config_test.go

+79
Original file line numberDiff line numberDiff line change
@@ -695,3 +695,82 @@ func BenchmarkMigrationOnLargeConfiguration(b *testing.B) {
695695
_, _ = computeMigratedConfig("path doesn't matter", largeConfiguration)
696696
}
697697
}
698+
699+
func TestAllBranchesLogCmdMigrations(t *testing.T) {
700+
scenarios := []struct {
701+
name string
702+
input string
703+
expected string
704+
}{
705+
{
706+
name: "Incomplete Configuration Passes uneventfully",
707+
input: "git:",
708+
expected: "git:",
709+
}, {
710+
name: "Single Cmd with no Cmds",
711+
input: `git:
712+
allBranchesLogCmd: git log --graph --oneline
713+
`,
714+
expected: `git:
715+
allBranchesLogCmds:
716+
- git log --graph --oneline
717+
`,
718+
}, {
719+
name: "Cmd with one existing Cmds",
720+
input: `git:
721+
allBranchesLogCmd: git log --graph --oneline
722+
allBranchesLogCmds:
723+
- git log --graph --oneline --pretty
724+
`,
725+
expected: `git:
726+
allBranchesLogCmds:
727+
- git log --graph --oneline
728+
- git log --graph --oneline --pretty
729+
`,
730+
}, {
731+
name: "Only Cmds set have no changes",
732+
input: `git:
733+
allBranchesLogCmds:
734+
- git log
735+
`,
736+
expected: `git:
737+
allBranchesLogCmds:
738+
- git log
739+
`,
740+
}, {
741+
name: "Removes Empty Cmd When at end of yaml",
742+
input: `git:
743+
allBranchesLogCmds:
744+
- git log --graph --oneline
745+
allBranchesLogCmd:
746+
`,
747+
expected: `git:
748+
allBranchesLogCmds:
749+
- git log --graph --oneline
750+
`,
751+
}, {
752+
name: "Removes Empty Cmd With Keys Afterwards",
753+
input: `git:
754+
allBranchesLogCmds:
755+
- git log --graph --oneline
756+
allBranchesLogCmd:
757+
foo: bar
758+
`,
759+
expected: `git:
760+
allBranchesLogCmds:
761+
- git log --graph --oneline
762+
foo: bar
763+
`,
764+
},
765+
}
766+
767+
for _, s := range scenarios {
768+
t.Run(s.name, func(t *testing.T) {
769+
actual, err := computeMigratedConfig("path doesn't matter", []byte(s.input))
770+
if err != nil {
771+
t.Error(err)
772+
}
773+
assert.Equal(t, s.expected, string(actual))
774+
})
775+
}
776+
}

Diff for: ‎pkg/config/user_config.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -253,9 +253,6 @@ type GitConfig struct {
253253
AutoStageResolvedConflicts bool `yaml:"autoStageResolvedConflicts"`
254254
// Command used when displaying the current branch git log in the main window
255255
BranchLogCmd string `yaml:"branchLogCmd"`
256-
// Command used to display git log of all branches in the main window.
257-
// Deprecated: Use `allBranchesLogCmds` instead.
258-
AllBranchesLogCmd string `yaml:"allBranchesLogCmd" jsonschema:"deprecated"`
259256
// Commands used to display git log of all branches in the main window, they will be cycled in order of appearance (array of strings)
260257
AllBranchesLogCmds []string `yaml:"allBranchesLogCmds"`
261258
// If true, do not spawn a separate process when using GPG
@@ -823,7 +820,7 @@ func GetDefaultConfig() *UserConfig {
823820
FetchAll: true,
824821
AutoStageResolvedConflicts: true,
825822
BranchLogCmd: "git log --graph --color=always --abbrev-commit --decorate --date=relative --pretty=medium {{branchName}} --",
826-
AllBranchesLogCmd: "git log --graph --all --color=always --abbrev-commit --decorate --date=relative --pretty=medium",
823+
AllBranchesLogCmds: []string{"git log --graph --all --color=always --abbrev-commit --decorate --date=relative --pretty=medium"},
827824
DisableForcePushing: false,
828825
CommitPrefixes: map[string][]CommitPrefixConfig(nil),
829826
BranchPrefix: "",

Diff for: ‎pkg/integration/tests/status/log_cmd.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ var LogCmd = NewIntegrationTest(NewIntegrationTestArgs{
1010
ExtraCmdArgs: []string{},
1111
Skip: false,
1212
SetupConfig: func(config *config.AppConfig) {
13-
config.GetUserConfig().Git.AllBranchesLogCmd = `echo "view1"`
14-
config.GetUserConfig().Git.AllBranchesLogCmds = []string{`echo "view2"`}
13+
config.GetUserConfig().Git.AllBranchesLogCmds = []string{`echo "view1"`, `echo "view2"`}
1514
},
1615
SetupRepo: func(shell *Shell) {},
1716
Run: func(t *TestDriver, keys config.KeybindingConfig) {

Diff for: ‎pkg/utils/yaml_utils/yaml_utils.go

+18-5
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func updateYamlNode(node *yaml.Node, path []string, value string) (bool, error)
6161
}
6262

6363
key := path[0]
64-
if _, valueNode := lookupKey(node, key); valueNode != nil {
64+
if _, valueNode := LookupKey(node, key); valueNode != nil {
6565
return updateYamlNode(valueNode, path[1:], value)
6666
}
6767

@@ -90,7 +90,7 @@ func updateYamlNode(node *yaml.Node, path []string, value string) (bool, error)
9090
return updateYamlNode(newNode, path[1:], value)
9191
}
9292

93-
func lookupKey(node *yaml.Node, key string) (*yaml.Node, *yaml.Node) {
93+
func LookupKey(node *yaml.Node, key string) (*yaml.Node, *yaml.Node) {
9494
for i := 0; i < len(node.Content)-1; i += 2 {
9595
if node.Content[i].Value == key {
9696
return node.Content[i], node.Content[i+1]
@@ -100,6 +100,19 @@ func lookupKey(node *yaml.Node, key string) (*yaml.Node, *yaml.Node) {
100100
return nil, nil
101101
}
102102

103+
// Returns the key and value if they were present
104+
func RemoveKey(node *yaml.Node, key string) (*yaml.Node, *yaml.Node) {
105+
for i := 0; i < len(node.Content)-1; i += 2 {
106+
if node.Content[i].Value == key {
107+
key, value := node.Content[i], node.Content[i+1]
108+
node.Content = append(node.Content[:i], node.Content[i+2:]...)
109+
return key, value
110+
}
111+
}
112+
113+
return nil, nil
114+
}
115+
103116
// Walks a yaml document from the root node to the specified path, and then applies the transformation to that node.
104117
// If the requested path is not defined in the document, no changes are made to the document.
105118
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
123136
return transform(node)
124137
}
125138

126-
keyNode, valueNode := lookupKey(node, path[0])
139+
keyNode, valueNode := LookupKey(node, path[0])
127140
if keyNode == nil {
128141
return nil
129142
}
@@ -154,15 +167,15 @@ func renameYamlKey(node *yaml.Node, path []string, newKey string) error {
154167
return errors.New("yaml node in path is not a dictionary")
155168
}
156169

157-
keyNode, valueNode := lookupKey(node, path[0])
170+
keyNode, valueNode := LookupKey(node, path[0])
158171
if keyNode == nil {
159172
return nil
160173
}
161174

162175
// end of path reached: rename key
163176
if len(path) == 1 {
164177
// Check that new key doesn't exist yet
165-
if newKeyNode, _ := lookupKey(node, newKey); newKeyNode != nil {
178+
if newKeyNode, _ := LookupKey(node, newKey); newKeyNode != nil {
166179
return fmt.Errorf("new key `%s' already exists", newKey)
167180
}
168181

Diff for: ‎schema/config.json

+4-6
Original file line numberDiff line numberDiff line change
@@ -340,17 +340,15 @@
340340
"description": "Command used when displaying the current branch git log in the main window",
341341
"default": "git log --graph --color=always --abbrev-commit --decorate --date=relative --pretty=medium {{branchName}} --"
342342
},
343-
"allBranchesLogCmd": {
344-
"type": "string",
345-
"description": "Command used to display git log of all branches in the main window.\nDeprecated: Use `allBranchesLogCmds` instead.",
346-
"default": "git log --graph --all --color=always --abbrev-commit --decorate --date=relative --pretty=medium"
347-
},
348343
"allBranchesLogCmds": {
349344
"items": {
350345
"type": "string"
351346
},
352347
"type": "array",
353-
"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)"
348+
"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)",
349+
"default": [
350+
"git log --graph --all --color=always --abbrev-commit --decorate --date=relative --pretty=medium"
351+
]
354352
},
355353
"overrideGpg": {
356354
"type": "boolean",

0 commit comments

Comments
 (0)