Log a list of migration changes to the console

This might be useful to see in general (users will normally only see it after
they quit lazygit again, but still). But it is especially useful when writing
back the config file fails for some reason, because users can then make these
changes manually if they want.

We do this only at startup, when the GUI hasn't started yet. This is probably
good enough, because it is much less likely that writing back a migrated
repo-local config fails because it is not writeable.

pkg/config/app_config_test.go

@@ -12,11 +12,13 @@ func TestMigrationOfRenamedKeys(t *testing.T) {
 		input             string
 		expected          string
 		expectedDidChange bool
+		expectedChanges   []string
 	}{
 		{
 			name:              "Empty String",
 			input:             "",
 			expectedDidChange: false,
+			expectedChanges:   []string{},
 		},
 		{
 			name: "No rename needed",
@@ -24,6 +26,7 @@ func TestMigrationOfRenamedKeys(t *testing.T) {
   bar: 5
 `,
 			expectedDidChange: false,
+			expectedChanges:   []string{},
 		},
 		{
 			name: "Rename one",
@@ -34,6 +37,7 @@ func TestMigrationOfRenamedKeys(t *testing.T) {
   skipDiscardChangeWarning: true
 `,
 			expectedDidChange: true,
+			expectedChanges:   []string{"Renamed 'gui.skipUnstageLineWarning' to 'skipDiscardChangeWarning'"},
 		},
 		{
 			name: "Rename several",
@@ -52,17 +56,24 @@ keybinding:
     executeShellCommand: a
 `,
 			expectedDidChange: true,
+			expectedChanges: []string{
+				"Renamed 'gui.skipUnstageLineWarning' to 'skipDiscardChangeWarning'",
+				"Renamed 'keybinding.universal.executeCustomCommand' to 'executeShellCommand'",
+				"Renamed 'gui.windowSize' to 'screenMode'",
+			},
 		},
 	}
 
 	for _, s := range scenarios {
 		t.Run(s.name, func(t *testing.T) {
-			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input))
+			changes := NewChangesSet()
+			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input), changes)
 			assert.NoError(t, err)
 			assert.Equal(t, s.expectedDidChange, didChange)
 			if didChange {
 				assert.Equal(t, s.expected, string(actual))
 			}
+			assert.Equal(t, s.expectedChanges, changes.ToSliceFromOldest())
 		})
 	}
 }
@@ -73,11 +84,13 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) {
 		input             string
 		expected          string
 		expectedDidChange bool
+		expectedChanges   []string
 	}{
 		{
 			name:              "Empty String",
 			input:             "",
 			expectedDidChange: false,
+			expectedChanges:   []string{},
 		},
 		{
 			name: "No change needed",
@@ -86,6 +99,7 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) {
     quit: q
 `,
 			expectedDidChange: false,
+			expectedChanges:   []string{},
 		},
 		{
 			name: "Change one",
@@ -98,6 +112,7 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) {
     quit: <disabled>
 `,
 			expectedDidChange: true,
+			expectedChanges:   []string{"Changed 'null' to '<disabled>' for keybinding 'keybinding.universal.quit'"},
 		},
 		{
 			name: "Change several",
@@ -114,17 +129,23 @@ func TestMigrateNullKeybindingsToDisabled(t *testing.T) {
     new: <disabled>
 `,
 			expectedDidChange: true,
+			expectedChanges: []string{
+				"Changed 'null' to '<disabled>' for keybinding 'keybinding.universal.quit'",
+				"Changed 'null' to '<disabled>' for keybinding 'keybinding.universal.new'",
+			},
 		},
 	}
 
 	for _, s := range scenarios {
 		t.Run(s.name, func(t *testing.T) {
-			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input))
+			changes := NewChangesSet()
+			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input), changes)
 			assert.NoError(t, err)
 			assert.Equal(t, s.expectedDidChange, didChange)
 			if didChange {
 				assert.Equal(t, s.expected, string(actual))
 			}
+			assert.Equal(t, s.expectedChanges, changes.ToSliceFromOldest())
 		})
 	}
 }
@@ -135,11 +156,13 @@ func TestCommitPrefixMigrations(t *testing.T) {
 		input             string
 		expected          string
 		expectedDidChange bool
+		expectedChanges   []string
 	}{
 		{
 			name:              "Empty String",
 			input:             "",
 			expectedDidChange: false,
+			expectedChanges:   []string{},
 		},
 		{
 			name: "Single CommitPrefix Rename",
@@ -154,6 +177,7 @@ func TestCommitPrefixMigrations(t *testing.T) {
       replace: '[JIRA $0] '
 `,
 			expectedDidChange: true,
+			expectedChanges:   []string{"Changed 'git.commitPrefix' to an array of strings"},
 		},
 		{
 			name: "Complicated CommitPrefixes Rename",
@@ -176,11 +200,13 @@ func TestCommitPrefixMigrations(t *testing.T) {
         replace: '[FUN $0] '
 `,
 			expectedDidChange: true,
+			expectedChanges:   []string{"Changed 'git.commitPrefixes' elements to arrays of strings"},
 		},
 		{
 			name:              "Incomplete Configuration",
 			input:             "git:",
 			expectedDidChange: false,
+			expectedChanges:   []string{},
 		},
 		{
 			name: "No changes made when already migrated",
@@ -194,17 +220,20 @@ git:
       - pattern: "^\\w+-\\w+.*"
         replace: '[JIRA $0] '`,
 			expectedDidChange: false,
+			expectedChanges:   []string{},
 		},
 	}
 
 	for _, s := range scenarios {
 		t.Run(s.name, func(t *testing.T) {
-			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input))
+			changes := NewChangesSet()
+			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input), changes)
 			assert.NoError(t, err)
 			assert.Equal(t, s.expectedDidChange, didChange)
 			if didChange {
 				assert.Equal(t, s.expected, string(actual))
 			}
+			assert.Equal(t, s.expectedChanges, changes.ToSliceFromOldest())
 		})
 	}
 }
@@ -215,11 +244,13 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
 		input             string
 		expected          string
 		expectedDidChange bool
+		expectedChanges   []string
 	}{
 		{
 			name:              "Empty String",
 			input:             "",
 			expectedDidChange: false,
+			expectedChanges:   []string{},
 		},
 		{
 			name: "Convert subprocess to output=terminal",
@@ -232,6 +263,7 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
     output: terminal
 `,
 			expectedDidChange: true,
+			expectedChanges:   []string{"Changed 'subprocess: true' to 'output: terminal' in custom command"},
 		},
 		{
 			name: "Convert stream to output=log",
@@ -244,6 +276,7 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
     output: log
 `,
 			expectedDidChange: true,
+			expectedChanges:   []string{"Changed 'stream: true' to 'output: log' in custom command"},
 		},
 		{
 			name: "Convert showOutput to output=popup",
@@ -256,6 +289,7 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
     output: popup
 `,
 			expectedDidChange: true,
+			expectedChanges:   []string{"Changed 'showOutput: true' to 'output: popup' in custom command"},
 		},
 		{
 			name: "Subprocess wins over the other two",
@@ -270,6 +304,11 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
     output: terminal
 `,
 			expectedDidChange: true,
+			expectedChanges: []string{
+				"Changed 'subprocess: true' to 'output: terminal' in custom command",
+				"Deleted redundant 'stream: true' property in custom command",
+				"Deleted redundant 'showOutput: true' property in custom command",
+			},
 		},
 		{
 			name: "Stream wins over showOutput",
@@ -283,6 +322,10 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
     output: log
 `,
 			expectedDidChange: true,
+			expectedChanges: []string{
+				"Changed 'stream: true' to 'output: log' in custom command",
+				"Deleted redundant 'showOutput: true' property in custom command",
+			},
 		},
 		{
 			name: "Explicitly setting to false doesn't create an output=none key",
@@ -296,17 +339,24 @@ func TestCustomCommandsOutputMigration(t *testing.T) {
   - command: echo 'hello'
 `,
 			expectedDidChange: true,
+			expectedChanges: []string{
+				"Deleted redundant 'subprocess: false' in custom command",
+				"Deleted redundant 'stream: false' property in custom command",
+				"Deleted redundant 'showOutput: false' property in custom command",
+			},
 		},
 	}
 
 	for _, s := range scenarios {
 		t.Run(s.name, func(t *testing.T) {
-			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input))
+			changes := NewChangesSet()
+			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input), changes)
 			assert.NoError(t, err)
 			assert.Equal(t, s.expectedDidChange, didChange)
 			if didChange {
 				assert.Equal(t, s.expected, string(actual))
 			}
+			assert.Equal(t, s.expectedChanges, changes.ToSliceFromOldest())
 		})
 	}
 }
@@ -915,7 +965,8 @@ keybinding:
 
 func BenchmarkMigrationOnLargeConfiguration(b *testing.B) {
 	for b.Loop() {
-		_, _, _ = computeMigratedConfig("path doesn't matter", largeConfiguration)
+		changes := NewChangesSet()
+		_, _, _ = computeMigratedConfig("path doesn't matter", largeConfiguration, changes)
 	}
 }
 
@@ -925,11 +976,13 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
 		input             string
 		expected          string
 		expectedDidChange bool
+		expectedChanges   []string
 	}{
 		{
 			name:              "Incomplete Configuration Passes uneventfully",
 			input:             "git:",
 			expectedDidChange: false,
+			expectedChanges:   []string{},
 		},
 		{
 			name: "Single Cmd with no Cmds",
@@ -941,6 +994,10 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
     - git log --graph --oneline
 `,
 			expectedDidChange: true,
+			expectedChanges: []string{
+				"Created git.allBranchesLogCmds array containing value of git.allBranchesLogCmd",
+				"Removed obsolete git.allBranchesLogCmd",
+			},
 		},
 		{
 			name: "Cmd with one existing Cmds",
@@ -955,6 +1012,10 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
     - git log --graph --oneline --pretty
 `,
 			expectedDidChange: true,
+			expectedChanges: []string{
+				"Prepended git.allBranchesLogCmd value to git.allBranchesLogCmds array",
+				"Removed obsolete git.allBranchesLogCmd",
+			},
 		},
 		{
 			name: "Only Cmds set have no changes",
@@ -962,7 +1023,8 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
   allBranchesLogCmds:
     - git log
 `,
-			expected: "",
+			expected:        "",
+			expectedChanges: []string{},
 		},
 		{
 			name: "Removes Empty Cmd When at end of yaml",
@@ -976,6 +1038,7 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
     - git log --graph --oneline
 `,
 			expectedDidChange: true,
+			expectedChanges:   []string{"Removed obsolete git.allBranchesLogCmd"},
 		},
 		{
 			name: "Migrates when sequence defined inline",
@@ -987,6 +1050,10 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
   allBranchesLogCmds: [baz, foo, bar]
 `,
 			expectedDidChange: true,
+			expectedChanges: []string{
+				"Prepended git.allBranchesLogCmd value to git.allBranchesLogCmds array",
+				"Removed obsolete git.allBranchesLogCmd",
+			},
 		},
 		{
 			name: "Removes Empty Cmd With Keys Afterwards",
@@ -1002,17 +1069,20 @@ func TestAllBranchesLogCmdMigrations(t *testing.T) {
   foo: bar
 `,
 			expectedDidChange: true,
+			expectedChanges:   []string{"Removed obsolete git.allBranchesLogCmd"},
 		},
 	}
 
 	for _, s := range scenarios {
 		t.Run(s.name, func(t *testing.T) {
-			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input))
+			changes := NewChangesSet()
+			actual, didChange, err := computeMigratedConfig("path doesn't matter", []byte(s.input), changes)
 			assert.NoError(t, err)
 			assert.Equal(t, s.expectedDidChange, didChange)
 			if didChange {
 				assert.Equal(t, s.expected, string(actual))
 			}
+			assert.Equal(t, s.expectedChanges, changes.ToSliceFromOldest())
 		})
 	}
 }