mirror of
https://github.com/jesseduffield/lazygit.git
synced 2025-08-10 22:42:00 +02:00
Don't rewrite config file unnecessarily when it contains commitPrefixes (#4311)
- **PR Description** The issue statement https://github.com/jesseduffield/lazygit/issues/4310 is exactly right, that the `commitPrefixes` element improperly claims that it has modified the yaml whenever it exists, even if it does not need to do changes. Now, we initialize it to false, only set it to true inside our modification section of the for loop. Tests updated to add one that would have failed prior to this change. The syntax change to use named struct fields instead of positional fields felt nice since I wanted to just define on a single one of them the `assertAsString` field. The reason that field is required at all is the 2nd complaint on the linked issue about the formatting change, is I don't believe is something that is trivial to fix. I observed on existing migrations before I wrote this one. But if it is easy to wrap up into this, let me know! Also, how do we normally backport things into previous releases? We'll probably want this to make it into a `0.47.2`. - **Please check if the PR fulfills these requirements** * [ ] 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)) * [ ] Docs have been updated if necessary * [X]You've read through your own file changes for silly mistakes etc
This commit is contained in:
@@ -306,6 +306,7 @@ func changeElementToSequence(changedContent []byte, path []string) ([]byte, erro
|
||||
|
||||
func changeCommitPrefixesMap(changedContent []byte) ([]byte, error) {
|
||||
return yaml_utils.TransformNode(changedContent, []string{"git", "commitPrefixes"}, func(prefixesNode *yaml.Node) (bool, error) {
|
||||
changedAnyNodes := false
|
||||
if prefixesNode.Kind == yaml.MappingNode {
|
||||
for _, contentNode := range prefixesNode.Content {
|
||||
if contentNode.Kind == yaml.MappingNode {
|
||||
@@ -317,12 +318,11 @@ func changeCommitPrefixesMap(changedContent []byte) ([]byte, error) {
|
||||
Kind: yaml.MappingNode,
|
||||
Content: nodeContentCopy,
|
||||
}}
|
||||
|
||||
changedAnyNodes = true
|
||||
}
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
return false, nil
|
||||
return changedAnyNodes, nil
|
||||
})
|
||||
}
|
||||
|
||||
|
@@ -4,7 +4,6 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"gopkg.in/yaml.v3"
|
||||
)
|
||||
|
||||
func TestCommitPrefixMigrations(t *testing.T) {
|
||||
@@ -14,65 +13,77 @@ func TestCommitPrefixMigrations(t *testing.T) {
|
||||
expected string
|
||||
}{
|
||||
{
|
||||
"Empty String",
|
||||
"",
|
||||
"",
|
||||
name: "Empty String",
|
||||
input: "",
|
||||
expected: "",
|
||||
}, {
|
||||
"Single CommitPrefix Rename",
|
||||
`
|
||||
git:
|
||||
name: "Single CommitPrefix Rename",
|
||||
input: `git:
|
||||
commitPrefix:
|
||||
pattern: "^\\w+-\\w+.*"
|
||||
replace: '[JIRA $0] '`,
|
||||
`
|
||||
git:
|
||||
replace: '[JIRA $0] '
|
||||
`,
|
||||
expected: `git:
|
||||
commitPrefix:
|
||||
- pattern: "^\\w+-\\w+.*"
|
||||
replace: '[JIRA $0] '`,
|
||||
replace: '[JIRA $0] '
|
||||
`,
|
||||
}, {
|
||||
"Complicated CommitPrefixes Rename",
|
||||
`
|
||||
git:
|
||||
name: "Complicated CommitPrefixes Rename",
|
||||
input: `git:
|
||||
commitPrefixes:
|
||||
foo:
|
||||
pattern: "^\\w+-\\w+.*"
|
||||
replace: '[OTHER $0] '
|
||||
CrazyName!@#$^*&)_-)[[}{f{[]:
|
||||
pattern: "^foo.bar*"
|
||||
replace: '[FUN $0] '`,
|
||||
`
|
||||
git:
|
||||
replace: '[FUN $0] '
|
||||
`,
|
||||
expected: `git:
|
||||
commitPrefixes:
|
||||
foo:
|
||||
- pattern: "^\\w+-\\w+.*"
|
||||
replace: '[OTHER $0] '
|
||||
CrazyName!@#$^*&)_-)[[}{f{[]:
|
||||
- pattern: "^foo.bar*"
|
||||
replace: '[FUN $0] '`,
|
||||
foo:
|
||||
- pattern: "^\\w+-\\w+.*"
|
||||
replace: '[OTHER $0] '
|
||||
CrazyName!@#$^*&)_-)[[}{f{[]:
|
||||
- pattern: "^foo.bar*"
|
||||
replace: '[FUN $0] '
|
||||
`,
|
||||
}, {
|
||||
"Incomplete Configuration",
|
||||
"git:",
|
||||
"git:",
|
||||
name: "Incomplete Configuration",
|
||||
input: "git:",
|
||||
expected: "git:",
|
||||
}, {
|
||||
// This test intentionally uses non-standard indentation to test that the migration
|
||||
// does not change the input.
|
||||
name: "No changes made when already migrated",
|
||||
input: `
|
||||
git:
|
||||
commitPrefix:
|
||||
- pattern: "Hello World"
|
||||
replace: "Goodbye"
|
||||
commitPrefixes:
|
||||
foo:
|
||||
- pattern: "^\\w+-\\w+.*"
|
||||
replace: '[JIRA $0] '`,
|
||||
expected: `
|
||||
git:
|
||||
commitPrefix:
|
||||
- pattern: "Hello World"
|
||||
replace: "Goodbye"
|
||||
commitPrefixes:
|
||||
foo:
|
||||
- pattern: "^\\w+-\\w+.*"
|
||||
replace: '[JIRA $0] '`,
|
||||
},
|
||||
}
|
||||
|
||||
for _, s := range scenarios {
|
||||
t.Run(s.name, func(t *testing.T) {
|
||||
expectedConfig := GetDefaultConfig()
|
||||
err := yaml.Unmarshal([]byte(s.expected), expectedConfig)
|
||||
if err != nil {
|
||||
t.Error(err)
|
||||
}
|
||||
actual, err := computeMigratedConfig("path doesn't matter", []byte(s.input))
|
||||
if err != nil {
|
||||
t.Error(err)
|
||||
}
|
||||
actualConfig := GetDefaultConfig()
|
||||
err = yaml.Unmarshal(actual, actualConfig)
|
||||
if err != nil {
|
||||
t.Error(err)
|
||||
}
|
||||
assert.Equal(t, expectedConfig, actualConfig)
|
||||
assert.Equal(t, s.expected, string(actual))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user