From 18c57804854692bd84ac50e88a10da7d81df363d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 28 Aug 2023 12:24:58 +0200 Subject: [PATCH 1/5] Add AppState to common.Common --- pkg/app/app.go | 2 ++ pkg/commands/git_commands/deps_test.go | 3 ++- pkg/common/common.go | 1 + pkg/utils/dummies.go | 3 ++- 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index f34611b34..f8e267ee9 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -62,6 +62,7 @@ func Run( func NewCommon(config config.AppConfigurer) (*common.Common, error) { userConfig := config.GetUserConfig() + appState := config.GetAppState() var err error log := newLogger(config) @@ -74,6 +75,7 @@ func NewCommon(config config.AppConfigurer) (*common.Common, error) { Log: log, Tr: tr, UserConfig: userConfig, + AppState: appState, Debug: config.GetDebug(), Fs: afero.NewOsFs(), }, nil diff --git a/pkg/commands/git_commands/deps_test.go b/pkg/commands/git_commands/deps_test.go index bb17ad0a7..eaf53306a 100644 --- a/pkg/commands/git_commands/deps_test.go +++ b/pkg/commands/git_commands/deps_test.go @@ -17,6 +17,7 @@ import ( type commonDeps struct { runner *oscommands.FakeCmdObjRunner userConfig *config.UserConfig + appState *config.AppState gitVersion *GitVersion gitConfig *git_config.FakeGitConfig getenv func(string) string @@ -32,7 +33,7 @@ func buildGitCommon(deps commonDeps) *GitCommon { gitCommon.Common = deps.common if gitCommon.Common == nil { - gitCommon.Common = utils.NewDummyCommonWithUserConfig(deps.userConfig) + gitCommon.Common = utils.NewDummyCommonWithUserConfigAndAppState(deps.userConfig, deps.appState) } if deps.fs != nil { diff --git a/pkg/common/common.go b/pkg/common/common.go index 35406118e..39de6e0d8 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -12,6 +12,7 @@ type Common struct { Log *logrus.Entry Tr *i18n.TranslationSet UserConfig *config.UserConfig + AppState *config.AppState Debug bool // for interacting with the filesystem. We use afero rather than the default // `os` package for the sake of mocking the filesystem in tests diff --git a/pkg/utils/dummies.go b/pkg/utils/dummies.go index 96c56b903..120c094f7 100644 --- a/pkg/utils/dummies.go +++ b/pkg/utils/dummies.go @@ -27,12 +27,13 @@ func NewDummyCommon() *common.Common { } } -func NewDummyCommonWithUserConfig(userConfig *config.UserConfig) *common.Common { +func NewDummyCommonWithUserConfigAndAppState(userConfig *config.UserConfig, appState *config.AppState) *common.Common { tr := i18n.EnglishTranslationSet() return &common.Common{ Log: NewDummyLog(), Tr: &tr, UserConfig: userConfig, + AppState: appState, // TODO: remove dependency on actual filesystem in tests and switch to using // in-memory for everything Fs: afero.NewOsFs(), From 1dac4158e9d8c8ab60203c76e9f86af409724823 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 28 Aug 2023 12:39:52 +0200 Subject: [PATCH 2/5] Don't pass ignoreWhitespace to git commands Now that AppState is available via common.Common, they can take it from there. --- pkg/commands/git.go | 4 +--- pkg/commands/git_commands/commit.go | 4 ++-- pkg/commands/git_commands/commit_test.go | 6 ++++-- pkg/commands/git_commands/stash.go | 4 ++-- pkg/commands/git_commands/stash_test.go | 6 ++++-- pkg/commands/git_commands/working_tree.go | 20 ++++++++----------- .../git_commands/working_tree_test.go | 12 +++++++---- .../controllers/commits_files_controller.go | 4 +--- pkg/gui/controllers/files_controller.go | 4 ++-- .../helpers/patch_building_helper.go | 4 +--- pkg/gui/controllers/helpers/staging_helper.go | 4 ++-- .../controllers/local_commits_controller.go | 2 +- .../controllers/reflog_commits_controller.go | 2 +- pkg/gui/controllers/stash_controller.go | 5 +---- pkg/gui/controllers/sub_commits_controller.go | 2 +- pkg/gui/controllers/submodules_controller.go | 2 +- 16 files changed, 40 insertions(+), 45 deletions(-) diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 92df3d8a2..f057f8d54 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -159,9 +159,7 @@ func NewGitCommandAux( stashCommands := git_commands.NewStashCommands(gitCommon, fileLoader, workingTreeCommands) patchBuilder := patch.NewPatchBuilder(cmn.Log, func(from string, to string, reverse bool, filename string, plain bool) (string, error) { - // TODO: make patch builder take Gui.IgnoreWhitespaceInDiffView into - // account. For now we just pass false. - return workingTreeCommands.ShowFileDiff(from, to, reverse, filename, plain, false) + return workingTreeCommands.ShowFileDiff(from, to, reverse, filename, plain) }) patchCommands := git_commands.NewPatchCommands(gitCommon, rebaseCommands, commitCommands, statusCommands, stashCommands, patchBuilder) bisectCommands := git_commands.NewBisectCommands(gitCommon) diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index c68cb0d9c..b56c87e74 100644 --- a/pkg/commands/git_commands/commit.go +++ b/pkg/commands/git_commands/commit.go @@ -196,7 +196,7 @@ func (self *CommitCommands) AmendHeadCmdObj() oscommands.ICmdObj { return self.cmd.New(cmdArgs) } -func (self *CommitCommands) ShowCmdObj(sha string, filterPath string, ignoreWhitespace bool) oscommands.ICmdObj { +func (self *CommitCommands) ShowCmdObj(sha string, filterPath string) oscommands.ICmdObj { contextSize := self.UserConfig.Git.DiffContextSize extDiffCmd := self.UserConfig.Git.Paging.ExternalDiffCommand @@ -210,7 +210,7 @@ func (self *CommitCommands) ShowCmdObj(sha string, filterPath string, ignoreWhit Arg("--decorate"). Arg("-p"). Arg(sha). - ArgIf(ignoreWhitespace, "--ignore-all-space"). + ArgIf(self.AppState.IgnoreWhitespaceInDiffView, "--ignore-all-space"). ArgIf(filterPath != "", "--", filterPath). ToArgv() diff --git a/pkg/commands/git_commands/commit_test.go b/pkg/commands/git_commands/commit_test.go index 48fc5edc7..ace0c97de 100644 --- a/pkg/commands/git_commands/commit_test.go +++ b/pkg/commands/git_commands/commit_test.go @@ -239,11 +239,13 @@ func TestCommitShowCmdObj(t *testing.T) { userConfig := config.GetDefaultConfig() userConfig.Git.DiffContextSize = s.contextSize userConfig.Git.Paging.ExternalDiffCommand = s.extDiffCmd + appState := &config.AppState{} + appState.IgnoreWhitespaceInDiffView = s.ignoreWhitespace runner := oscommands.NewFakeRunner(t).ExpectGitArgs(s.expected, "", nil) - instance := buildCommitCommands(commonDeps{userConfig: userConfig, runner: runner}) + instance := buildCommitCommands(commonDeps{userConfig: userConfig, appState: appState, runner: runner}) - assert.NoError(t, instance.ShowCmdObj("1234567890", s.filterPath, s.ignoreWhitespace).Run()) + assert.NoError(t, instance.ShowCmdObj("1234567890", s.filterPath).Run()) runner.CheckForMissingCalls() }) } diff --git a/pkg/commands/git_commands/stash.go b/pkg/commands/git_commands/stash.go index 562e7b97d..12907a6ae 100644 --- a/pkg/commands/git_commands/stash.go +++ b/pkg/commands/git_commands/stash.go @@ -80,13 +80,13 @@ func (self *StashCommands) Sha(index int) (string, error) { return strings.Trim(sha, "\r\n"), err } -func (self *StashCommands) ShowStashEntryCmdObj(index int, ignoreWhitespace bool) oscommands.ICmdObj { +func (self *StashCommands) ShowStashEntryCmdObj(index int) oscommands.ICmdObj { cmdArgs := NewGitCmd("stash").Arg("show"). Arg("-p"). Arg("--stat"). Arg(fmt.Sprintf("--color=%s", self.UserConfig.Git.Paging.ColorArg)). Arg(fmt.Sprintf("--unified=%d", self.UserConfig.Git.DiffContextSize)). - ArgIf(ignoreWhitespace, "--ignore-all-space"). + ArgIf(self.AppState.IgnoreWhitespaceInDiffView, "--ignore-all-space"). Arg(fmt.Sprintf("stash@{%d}", index)). ToArgv() diff --git a/pkg/commands/git_commands/stash_test.go b/pkg/commands/git_commands/stash_test.go index 1c8976c6d..651430ab8 100644 --- a/pkg/commands/git_commands/stash_test.go +++ b/pkg/commands/git_commands/stash_test.go @@ -135,9 +135,11 @@ func TestStashStashEntryCmdObj(t *testing.T) { t.Run(s.testName, func(t *testing.T) { userConfig := config.GetDefaultConfig() userConfig.Git.DiffContextSize = s.contextSize - instance := buildStashCommands(commonDeps{userConfig: userConfig}) + appState := &config.AppState{} + appState.IgnoreWhitespaceInDiffView = s.ignoreWhitespace + instance := buildStashCommands(commonDeps{userConfig: userConfig, appState: appState}) - cmdStr := instance.ShowStashEntryCmdObj(s.index, s.ignoreWhitespace).Args() + cmdStr := instance.ShowStashEntryCmdObj(s.index).Args() assert.Equal(t, s.expected, cmdStr) }) } diff --git a/pkg/commands/git_commands/working_tree.go b/pkg/commands/git_commands/working_tree.go index a9ebd3985..29e259f8f 100644 --- a/pkg/commands/git_commands/working_tree.go +++ b/pkg/commands/git_commands/working_tree.go @@ -228,13 +228,13 @@ func (self *WorkingTreeCommands) Exclude(filename string) error { } // WorktreeFileDiff returns the diff of a file -func (self *WorkingTreeCommands) WorktreeFileDiff(file *models.File, plain bool, cached bool, ignoreWhitespace bool) string { +func (self *WorkingTreeCommands) WorktreeFileDiff(file *models.File, plain bool, cached bool) string { // for now we assume an error means the file was deleted - s, _ := self.WorktreeFileDiffCmdObj(file, plain, cached, ignoreWhitespace).RunWithOutput() + s, _ := self.WorktreeFileDiffCmdObj(file, plain, cached).RunWithOutput() return s } -func (self *WorkingTreeCommands) WorktreeFileDiffCmdObj(node models.IFile, plain bool, cached bool, ignoreWhitespace bool) oscommands.ICmdObj { +func (self *WorkingTreeCommands) WorktreeFileDiffCmdObj(node models.IFile, plain bool, cached bool) oscommands.ICmdObj { colorArg := self.UserConfig.Git.Paging.ColorArg if plain { colorArg = "never" @@ -252,7 +252,7 @@ func (self *WorkingTreeCommands) WorktreeFileDiffCmdObj(node models.IFile, plain Arg("--submodule"). Arg(fmt.Sprintf("--unified=%d", contextSize)). Arg(fmt.Sprintf("--color=%s", colorArg)). - ArgIf(ignoreWhitespace, "--ignore-all-space"). + ArgIf(!plain && self.AppState.IgnoreWhitespaceInDiffView, "--ignore-all-space"). ArgIf(cached, "--cached"). ArgIf(noIndex, "--no-index"). Arg("--"). @@ -266,15 +266,11 @@ func (self *WorkingTreeCommands) WorktreeFileDiffCmdObj(node models.IFile, plain // ShowFileDiff get the diff of specified from and to. Typically this will be used for a single commit so it'll be 123abc^..123abc // but when we're in diff mode it could be any 'from' to any 'to'. The reverse flag is also here thanks to diff mode. -func (self *WorkingTreeCommands) ShowFileDiff(from string, to string, reverse bool, fileName string, plain bool, - ignoreWhitespace bool, -) (string, error) { - return self.ShowFileDiffCmdObj(from, to, reverse, fileName, plain, ignoreWhitespace).RunWithOutput() +func (self *WorkingTreeCommands) ShowFileDiff(from string, to string, reverse bool, fileName string, plain bool) (string, error) { + return self.ShowFileDiffCmdObj(from, to, reverse, fileName, plain).RunWithOutput() } -func (self *WorkingTreeCommands) ShowFileDiffCmdObj(from string, to string, reverse bool, fileName string, plain bool, - ignoreWhitespace bool, -) oscommands.ICmdObj { +func (self *WorkingTreeCommands) ShowFileDiffCmdObj(from string, to string, reverse bool, fileName string, plain bool) oscommands.ICmdObj { contextSize := self.UserConfig.Git.DiffContextSize colorArg := self.UserConfig.Git.Paging.ColorArg @@ -295,7 +291,7 @@ func (self *WorkingTreeCommands) ShowFileDiffCmdObj(from string, to string, reve Arg(from). Arg(to). ArgIf(reverse, "-R"). - ArgIf(ignoreWhitespace, "--ignore-all-space"). + ArgIf(!plain && self.AppState.IgnoreWhitespaceInDiffView, "--ignore-all-space"). Arg("--"). Arg(fileName). ToArgv() diff --git a/pkg/commands/git_commands/working_tree_test.go b/pkg/commands/git_commands/working_tree_test.go index a79631236..8137a96af 100644 --- a/pkg/commands/git_commands/working_tree_test.go +++ b/pkg/commands/git_commands/working_tree_test.go @@ -310,9 +310,11 @@ func TestWorkingTreeDiff(t *testing.T) { t.Run(s.testName, func(t *testing.T) { userConfig := config.GetDefaultConfig() userConfig.Git.DiffContextSize = s.contextSize + appState := &config.AppState{} + appState.IgnoreWhitespaceInDiffView = s.ignoreWhitespace - instance := buildWorkingTreeCommands(commonDeps{runner: s.runner, userConfig: userConfig}) - result := instance.WorktreeFileDiff(s.file, s.plain, s.cached, s.ignoreWhitespace) + instance := buildWorkingTreeCommands(commonDeps{runner: s.runner, userConfig: userConfig, appState: appState}) + result := instance.WorktreeFileDiff(s.file, s.plain, s.cached) assert.Equal(t, expectedResult, result) s.runner.CheckForMissingCalls() }) @@ -374,10 +376,12 @@ func TestWorkingTreeShowFileDiff(t *testing.T) { t.Run(s.testName, func(t *testing.T) { userConfig := config.GetDefaultConfig() userConfig.Git.DiffContextSize = s.contextSize + appState := &config.AppState{} + appState.IgnoreWhitespaceInDiffView = s.ignoreWhitespace - instance := buildWorkingTreeCommands(commonDeps{runner: s.runner, userConfig: userConfig}) + instance := buildWorkingTreeCommands(commonDeps{runner: s.runner, userConfig: userConfig, appState: appState}) - result, err := instance.ShowFileDiff(s.from, s.to, s.reverse, "test.txt", s.plain, s.ignoreWhitespace) + result, err := instance.ShowFileDiff(s.from, s.to, s.reverse, "test.txt", s.plain) assert.NoError(t, err) assert.Equal(t, expectedResult, result) s.runner.CheckForMissingCalls() diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go index c82a051f4..3730a0965 100644 --- a/pkg/gui/controllers/commits_files_controller.go +++ b/pkg/gui/controllers/commits_files_controller.go @@ -113,9 +113,7 @@ func (self *CommitFilesController) GetOnRenderToMain() func() error { to := ref.RefName() from, reverse := self.c.Modes().Diffing.GetFromAndReverseArgsForDiff(ref.ParentRefName()) - cmdObj := self.c.Git().WorkingTree.ShowFileDiffCmdObj( - from, to, reverse, node.GetPath(), false, self.c.GetAppState().IgnoreWhitespaceInDiffView, - ) + cmdObj := self.c.Git().WorkingTree.ShowFileDiffCmdObj(from, to, reverse, node.GetPath(), false) task := types.NewRunPtyTask(cmdObj.GetCmd()) pair := self.c.MainViewPairs().Normal diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index aae88fe79..2ecdc39e5 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -201,7 +201,7 @@ func (self *FilesController) GetOnRenderToMain() func() error { split := self.c.UserConfig.Gui.SplitDiff == "always" || (node.GetHasUnstagedChanges() && node.GetHasStagedChanges()) mainShowsStaged := !split && node.GetHasStagedChanges() - cmdObj := self.c.Git().WorkingTree.WorktreeFileDiffCmdObj(node, false, mainShowsStaged, self.c.GetAppState().IgnoreWhitespaceInDiffView) + cmdObj := self.c.Git().WorkingTree.WorktreeFileDiffCmdObj(node, false, mainShowsStaged) title := self.c.Tr.UnstagedChanges if mainShowsStaged { title = self.c.Tr.StagedChanges @@ -216,7 +216,7 @@ func (self *FilesController) GetOnRenderToMain() func() error { } if split { - cmdObj := self.c.Git().WorkingTree.WorktreeFileDiffCmdObj(node, false, true, self.c.GetAppState().IgnoreWhitespaceInDiffView) + cmdObj := self.c.Git().WorkingTree.WorktreeFileDiffCmdObj(node, false, true) title := self.c.Tr.StagedChanges if mainShowsStaged { diff --git a/pkg/gui/controllers/helpers/patch_building_helper.go b/pkg/gui/controllers/helpers/patch_building_helper.go index 7dd93e6a6..fd3c49718 100644 --- a/pkg/gui/controllers/helpers/patch_building_helper.go +++ b/pkg/gui/controllers/helpers/patch_building_helper.go @@ -73,9 +73,7 @@ func (self *PatchBuildingHelper) RefreshPatchBuildingPanel(opts types.OnFocusOpt ref := self.c.Contexts().CommitFiles.CommitFileTreeViewModel.GetRef() to := ref.RefName() from, reverse := self.c.Modes().Diffing.GetFromAndReverseArgsForDiff(ref.ParentRefName()) - // Passing false for ignoreWhitespace because the patch building panel - // doesn't work when whitespace is ignored - diff, err := self.c.Git().WorkingTree.ShowFileDiff(from, to, reverse, path, true, false) + diff, err := self.c.Git().WorkingTree.ShowFileDiff(from, to, reverse, path, true) if err != nil { return err } diff --git a/pkg/gui/controllers/helpers/staging_helper.go b/pkg/gui/controllers/helpers/staging_helper.go index 75280b985..bf4424907 100644 --- a/pkg/gui/controllers/helpers/staging_helper.go +++ b/pkg/gui/controllers/helpers/staging_helper.go @@ -52,8 +52,8 @@ func (self *StagingHelper) RefreshStagingPanel(focusOpts types.OnFocusOpts) erro return self.handleStagingEscape() } - mainDiff := self.c.Git().WorkingTree.WorktreeFileDiff(file, true, false, false) - secondaryDiff := self.c.Git().WorkingTree.WorktreeFileDiff(file, true, true, false) + mainDiff := self.c.Git().WorkingTree.WorktreeFileDiff(file, true, false) + secondaryDiff := self.c.Git().WorkingTree.WorktreeFileDiff(file, true, true) // grabbing locks here and releasing before we finish the function // because pushing say the secondary context could mean entering this function diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 397cdb808..a110310f7 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -177,7 +177,7 @@ func (self *LocalCommitsController) GetOnRenderToMain() func() error { "ref": commit.Name, })) } else { - cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Sha, self.c.Modes().Filtering.GetPath(), self.c.GetAppState().IgnoreWhitespaceInDiffView) + cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Sha, self.c.Modes().Filtering.GetPath()) task = types.NewRunPtyTask(cmdObj.GetCmd()) } diff --git a/pkg/gui/controllers/reflog_commits_controller.go b/pkg/gui/controllers/reflog_commits_controller.go index 512390a6b..9cd5dd050 100644 --- a/pkg/gui/controllers/reflog_commits_controller.go +++ b/pkg/gui/controllers/reflog_commits_controller.go @@ -37,7 +37,7 @@ func (self *ReflogCommitsController) GetOnRenderToMain() func() error { if commit == nil { task = types.NewRenderStringTask("No reflog history") } else { - cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Sha, self.c.Modes().Filtering.GetPath(), self.c.GetAppState().IgnoreWhitespaceInDiffView) + cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Sha, self.c.Modes().Filtering.GetPath()) task = types.NewRunPtyTask(cmdObj.GetCmd()) } diff --git a/pkg/gui/controllers/stash_controller.go b/pkg/gui/controllers/stash_controller.go index 1b8377f27..5d74e10af 100644 --- a/pkg/gui/controllers/stash_controller.go +++ b/pkg/gui/controllers/stash_controller.go @@ -64,10 +64,7 @@ func (self *StashController) GetOnRenderToMain() func() error { task = types.NewRenderStringTask(self.c.Tr.NoStashEntries) } else { task = types.NewRunPtyTask( - self.c.Git().Stash.ShowStashEntryCmdObj( - stashEntry.Index, - self.c.GetAppState().IgnoreWhitespaceInDiffView, - ).GetCmd(), + self.c.Git().Stash.ShowStashEntryCmdObj(stashEntry.Index).GetCmd(), ) } diff --git a/pkg/gui/controllers/sub_commits_controller.go b/pkg/gui/controllers/sub_commits_controller.go index c11bba089..46dc0df98 100644 --- a/pkg/gui/controllers/sub_commits_controller.go +++ b/pkg/gui/controllers/sub_commits_controller.go @@ -38,7 +38,7 @@ func (self *SubCommitsController) GetOnRenderToMain() func() error { if commit == nil { task = types.NewRenderStringTask("No commits") } else { - cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Sha, self.c.Modes().Filtering.GetPath(), self.c.GetAppState().IgnoreWhitespaceInDiffView) + cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Sha, self.c.Modes().Filtering.GetPath()) task = types.NewRunPtyTask(cmdObj.GetCmd()) } diff --git a/pkg/gui/controllers/submodules_controller.go b/pkg/gui/controllers/submodules_controller.go index 06a26af2e..d7ed12132 100644 --- a/pkg/gui/controllers/submodules_controller.go +++ b/pkg/gui/controllers/submodules_controller.go @@ -102,7 +102,7 @@ func (self *SubmodulesController) GetOnRenderToMain() func() error { if file == nil { task = types.NewRenderStringTask(prefix) } else { - cmdObj := self.c.Git().WorkingTree.WorktreeFileDiffCmdObj(file, false, !file.HasUnstagedChanges && file.HasStagedChanges, self.c.GetAppState().IgnoreWhitespaceInDiffView) + cmdObj := self.c.Git().WorkingTree.WorktreeFileDiffCmdObj(file, false, !file.HasUnstagedChanges && file.HasStagedChanges) task = types.NewRunCommandTaskWithPrefix(cmdObj.GetCmd(), prefix) } } From 1106981827f5730ae9e71ff38bde27751fc66213 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 28 Aug 2023 13:18:58 +0200 Subject: [PATCH 3/5] Extract a SaveAppStateAndLogError function It seems that most actions that change a state option and resave the state want to just log the error. --- pkg/gui/controllers/toggle_whitespace_action.go | 4 +--- pkg/gui/extras_panel.go | 4 +--- pkg/gui/gui_common.go | 6 ++++++ pkg/gui/types/common.go | 1 + 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/gui/controllers/toggle_whitespace_action.go b/pkg/gui/controllers/toggle_whitespace_action.go index 5e9c1bf1b..175738b59 100644 --- a/pkg/gui/controllers/toggle_whitespace_action.go +++ b/pkg/gui/controllers/toggle_whitespace_action.go @@ -24,9 +24,7 @@ func (self *ToggleWhitespaceAction) Call() error { } self.c.GetAppState().IgnoreWhitespaceInDiffView = !self.c.GetAppState().IgnoreWhitespaceInDiffView - if err := self.c.SaveAppState(); err != nil { - self.c.Log.Errorf("error when saving app state: %v", err) - } + self.c.SaveAppStateAndLogError() return self.c.CurrentSideContext().HandleFocus(types.OnFocusOpts{}) } diff --git a/pkg/gui/extras_panel.go b/pkg/gui/extras_panel.go index 5381823b9..1d42f3c92 100644 --- a/pkg/gui/extras_panel.go +++ b/pkg/gui/extras_panel.go @@ -24,9 +24,7 @@ func (gui *Gui) handleCreateExtrasMenuPanel() error { show := !gui.c.State().GetShowExtrasWindow() gui.c.State().SetShowExtrasWindow(show) gui.c.GetAppState().HideCommandLog = !show - if err := gui.c.SaveAppState(); err != nil { - gui.c.Log.Errorf("error when saving app state: %v", err) - } + gui.c.SaveAppStateAndLogError() return nil }, }, diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index e3e288c11..bad0957ec 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -92,6 +92,12 @@ func (self *guiCommon) SaveAppState() error { return self.gui.Config.SaveAppState() } +func (self *guiCommon) SaveAppStateAndLogError() { + if err := self.gui.Config.SaveAppState(); err != nil { + self.gui.Log.Errorf("error when saving app state: %v", err) + } +} + func (self *guiCommon) GetConfig() config.AppConfigurer { return self.gui.Config } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index df927e1b2..6eacd882b 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -73,6 +73,7 @@ type IGuiCommon interface { GetConfig() config.AppConfigurer GetAppState() *config.AppState SaveAppState() error + SaveAppStateAndLogError() // Runs the given function on the UI thread (this is for things like showing a popup asking a user for input). // Only necessary to call if you're not already on the UI thread i.e. you're inside a goroutine. From f7db17f7d05280d83d57f7266dbc4e9d3ba2c48a Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 28 Aug 2023 13:24:52 +0200 Subject: [PATCH 4/5] Load defaults for AppState before reading from yaml So far this hasn't been necessary because all defaults were zero values. We're about to add the first non-zero value though, and it's important that it is initialized correctly for users who have a state.yml that doesn't have it yet. --- pkg/config/app_config.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index a9872ea1e..ea26dd872 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -283,11 +283,13 @@ func (c *AppConfig) SaveAppState() error { // loadAppState loads recorded AppState from file func loadAppState() (*AppState, error) { + appState := getDefaultAppState() + filepath, err := configFilePath("state.yml") if err != nil { if os.IsPermission(err) { // apparently when people have read-only permissions they prefer us to fail silently - return getDefaultAppState(), nil + return appState, nil } return nil, err } @@ -298,10 +300,9 @@ func loadAppState() (*AppState, error) { } if len(appStateBytes) == 0 { - return getDefaultAppState(), nil + return appState, nil } - appState := &AppState{} err = yaml.Unmarshal(appStateBytes, appState) if err != nil { return nil, err From 7774fe0ab37bb07832ef7349a3f7c080f9d908e1 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 28 Aug 2023 13:09:55 +0200 Subject: [PATCH 5/5] Move diff context size from UserConfig to AppState --- docs/Config.md | 1 - pkg/commands/git_commands/commit.go | 2 +- pkg/commands/git_commands/commit_test.go | 2 +- pkg/commands/git_commands/stash.go | 2 +- pkg/commands/git_commands/stash_test.go | 2 +- pkg/commands/git_commands/working_tree.go | 4 ++-- pkg/commands/git_commands/working_tree_test.go | 4 ++-- pkg/config/app_config.go | 2 ++ pkg/config/user_config.go | 6 ++---- pkg/gui/controllers/context_lines_controller.go | 10 +++++++--- pkg/i18n/english.go | 2 ++ 11 files changed, 21 insertions(+), 16 deletions(-) diff --git a/docs/Config.md b/docs/Config.md index 6546572c9..f23e6ccf0 100644 --- a/docs/Config.md +++ b/docs/Config.md @@ -117,7 +117,6 @@ git: overrideGpg: false # prevents lazygit from spawning a separate process when using GPG disableForcePushing: false parseEmoji: false - diffContextSize: 3 # how many lines of context are shown around a change in diffs os: copyToClipboardCmd: '' # See 'Custom Command for Copying to Clipboard' section editPreset: '' # see 'Configuring File Editing' section diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index b56c87e74..2f8446b1a 100644 --- a/pkg/commands/git_commands/commit.go +++ b/pkg/commands/git_commands/commit.go @@ -197,7 +197,7 @@ func (self *CommitCommands) AmendHeadCmdObj() oscommands.ICmdObj { } func (self *CommitCommands) ShowCmdObj(sha string, filterPath string) oscommands.ICmdObj { - contextSize := self.UserConfig.Git.DiffContextSize + contextSize := self.AppState.DiffContextSize extDiffCmd := self.UserConfig.Git.Paging.ExternalDiffCommand cmdArgs := NewGitCmd("show"). diff --git a/pkg/commands/git_commands/commit_test.go b/pkg/commands/git_commands/commit_test.go index ace0c97de..a6f887a12 100644 --- a/pkg/commands/git_commands/commit_test.go +++ b/pkg/commands/git_commands/commit_test.go @@ -237,10 +237,10 @@ func TestCommitShowCmdObj(t *testing.T) { s := s t.Run(s.testName, func(t *testing.T) { userConfig := config.GetDefaultConfig() - userConfig.Git.DiffContextSize = s.contextSize userConfig.Git.Paging.ExternalDiffCommand = s.extDiffCmd appState := &config.AppState{} appState.IgnoreWhitespaceInDiffView = s.ignoreWhitespace + appState.DiffContextSize = s.contextSize runner := oscommands.NewFakeRunner(t).ExpectGitArgs(s.expected, "", nil) instance := buildCommitCommands(commonDeps{userConfig: userConfig, appState: appState, runner: runner}) diff --git a/pkg/commands/git_commands/stash.go b/pkg/commands/git_commands/stash.go index 12907a6ae..fad73a472 100644 --- a/pkg/commands/git_commands/stash.go +++ b/pkg/commands/git_commands/stash.go @@ -85,7 +85,7 @@ func (self *StashCommands) ShowStashEntryCmdObj(index int) oscommands.ICmdObj { Arg("-p"). Arg("--stat"). Arg(fmt.Sprintf("--color=%s", self.UserConfig.Git.Paging.ColorArg)). - Arg(fmt.Sprintf("--unified=%d", self.UserConfig.Git.DiffContextSize)). + Arg(fmt.Sprintf("--unified=%d", self.AppState.DiffContextSize)). ArgIf(self.AppState.IgnoreWhitespaceInDiffView, "--ignore-all-space"). Arg(fmt.Sprintf("stash@{%d}", index)). ToArgv() diff --git a/pkg/commands/git_commands/stash_test.go b/pkg/commands/git_commands/stash_test.go index 651430ab8..846c493ee 100644 --- a/pkg/commands/git_commands/stash_test.go +++ b/pkg/commands/git_commands/stash_test.go @@ -134,9 +134,9 @@ func TestStashStashEntryCmdObj(t *testing.T) { s := s t.Run(s.testName, func(t *testing.T) { userConfig := config.GetDefaultConfig() - userConfig.Git.DiffContextSize = s.contextSize appState := &config.AppState{} appState.IgnoreWhitespaceInDiffView = s.ignoreWhitespace + appState.DiffContextSize = s.contextSize instance := buildStashCommands(commonDeps{userConfig: userConfig, appState: appState}) cmdStr := instance.ShowStashEntryCmdObj(s.index).Args() diff --git a/pkg/commands/git_commands/working_tree.go b/pkg/commands/git_commands/working_tree.go index 29e259f8f..7921cd26f 100644 --- a/pkg/commands/git_commands/working_tree.go +++ b/pkg/commands/git_commands/working_tree.go @@ -240,7 +240,7 @@ func (self *WorkingTreeCommands) WorktreeFileDiffCmdObj(node models.IFile, plain colorArg = "never" } - contextSize := self.UserConfig.Git.DiffContextSize + contextSize := self.AppState.DiffContextSize prevPath := node.GetPreviousPath() noIndex := !node.GetIsTracked() && !node.GetHasStagedChanges() && !cached && node.GetIsFile() extDiffCmd := self.UserConfig.Git.Paging.ExternalDiffCommand @@ -271,7 +271,7 @@ func (self *WorkingTreeCommands) ShowFileDiff(from string, to string, reverse bo } func (self *WorkingTreeCommands) ShowFileDiffCmdObj(from string, to string, reverse bool, fileName string, plain bool) oscommands.ICmdObj { - contextSize := self.UserConfig.Git.DiffContextSize + contextSize := self.AppState.DiffContextSize colorArg := self.UserConfig.Git.Paging.ColorArg if plain { diff --git a/pkg/commands/git_commands/working_tree_test.go b/pkg/commands/git_commands/working_tree_test.go index 8137a96af..cc46d4994 100644 --- a/pkg/commands/git_commands/working_tree_test.go +++ b/pkg/commands/git_commands/working_tree_test.go @@ -309,9 +309,9 @@ func TestWorkingTreeDiff(t *testing.T) { s := s t.Run(s.testName, func(t *testing.T) { userConfig := config.GetDefaultConfig() - userConfig.Git.DiffContextSize = s.contextSize appState := &config.AppState{} appState.IgnoreWhitespaceInDiffView = s.ignoreWhitespace + appState.DiffContextSize = s.contextSize instance := buildWorkingTreeCommands(commonDeps{runner: s.runner, userConfig: userConfig, appState: appState}) result := instance.WorktreeFileDiff(s.file, s.plain, s.cached) @@ -375,9 +375,9 @@ func TestWorkingTreeShowFileDiff(t *testing.T) { s := s t.Run(s.testName, func(t *testing.T) { userConfig := config.GetDefaultConfig() - userConfig.Git.DiffContextSize = s.contextSize appState := &config.AppState{} appState.IgnoreWhitespaceInDiffView = s.ignoreWhitespace + appState.DiffContextSize = s.contextSize instance := buildWorkingTreeCommands(commonDeps{runner: s.runner, userConfig: userConfig, appState: appState}) diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index ea26dd872..54d41b695 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -322,6 +322,7 @@ type AppState struct { CustomCommandsHistory []string HideCommandLog bool IgnoreWhitespaceInDiffView bool + DiffContextSize int } func getDefaultAppState() *AppState { @@ -329,6 +330,7 @@ func getDefaultAppState() *AppState { LastUpdateCheck: 0, RecentRepos: []string{}, StartupPopupVersion: 0, + DiffContextSize: 3, } } diff --git a/pkg/config/user_config.go b/pkg/config/user_config.go index 0acfcfb9b..4df6b5676 100644 --- a/pkg/config/user_config.go +++ b/pkg/config/user_config.go @@ -95,9 +95,8 @@ type GitConfig struct { DisableForcePushing bool `yaml:"disableForcePushing"` CommitPrefixes map[string]CommitPrefixConfig `yaml:"commitPrefixes"` // this should really be under 'gui', not 'git' - ParseEmoji bool `yaml:"parseEmoji"` - Log LogConfig `yaml:"log"` - DiffContextSize int `yaml:"diffContextSize"` + ParseEmoji bool `yaml:"parseEmoji"` + Log LogConfig `yaml:"log"` } type PagingConfig struct { @@ -497,7 +496,6 @@ func GetDefaultConfig() *UserConfig { DisableForcePushing: false, CommitPrefixes: map[string]CommitPrefixConfig(nil), ParseEmoji: false, - DiffContextSize: 3, }, Refresher: RefresherConfig{ RefreshInterval: 10, diff --git a/pkg/gui/controllers/context_lines_controller.go b/pkg/gui/controllers/context_lines_controller.go index 5ec2d3167..d3ff7688d 100644 --- a/pkg/gui/controllers/context_lines_controller.go +++ b/pkg/gui/controllers/context_lines_controller.go @@ -2,6 +2,7 @@ package controllers import ( "errors" + "fmt" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" @@ -65,7 +66,7 @@ func (self *ContextLinesController) Increase() error { return self.c.Error(err) } - self.c.UserConfig.Git.DiffContextSize = self.c.UserConfig.Git.DiffContextSize + 1 + self.c.AppState.DiffContextSize++ return self.applyChange() } @@ -73,14 +74,14 @@ func (self *ContextLinesController) Increase() error { } func (self *ContextLinesController) Decrease() error { - old_size := self.c.UserConfig.Git.DiffContextSize + old_size := self.c.AppState.DiffContextSize if self.isShowingDiff() && old_size > 1 { if err := self.checkCanChangeContext(); err != nil { return self.c.Error(err) } - self.c.UserConfig.Git.DiffContextSize = old_size - 1 + self.c.AppState.DiffContextSize = old_size - 1 return self.applyChange() } @@ -88,6 +89,9 @@ func (self *ContextLinesController) Decrease() error { } func (self *ContextLinesController) applyChange() error { + self.c.Toast(fmt.Sprintf(self.c.Tr.DiffContextSizeChanged, self.c.AppState.DiffContextSize)) + self.c.SaveAppStateAndLogError() + currentContext := self.c.CurrentStaticContext() switch currentContext.GetKey() { // we make an exception for our staging and patch building contexts because they actually need to refresh their state afterwards. diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 3dbe405ff..5c7f99b55 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -512,6 +512,7 @@ type TranslationSet struct { IgnoreWhitespaceNotSupportedHere string IncreaseContextInDiffView string DecreaseContextInDiffView string + DiffContextSizeChanged string CreatePullRequestOptions string DefaultBranch string SelectBranch string @@ -1293,6 +1294,7 @@ func EnglishTranslationSet() TranslationSet { IgnoreWhitespaceNotSupportedHere: "Ignoring whitespace is not supported in this view", IncreaseContextInDiffView: "Increase the size of the context shown around changes in the diff view", DecreaseContextInDiffView: "Decrease the size of the context shown around changes in the diff view", + DiffContextSizeChanged: "Changed diff context size to %d", CreatePullRequestOptions: "Create pull request options", DefaultBranch: "Default branch", SelectBranch: "Select branch",