diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 7e653af18..3cb6da94f 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -2031,6 +2031,26 @@ func (v *View) BufferLines() []string { return lines } +// MarkedLines returns the lines of the view's content that the inclusion gutter is +// marking (see SetInclusionGutter), in the order they appear. Empty while the gutter +// is hidden. +func (v *View) MarkedLines() []string { + v.writeMutex.Lock() + defer v.writeMutex.Unlock() + + if !v.showInclusionGutter { + return nil + } + + lines := []string{} + for i, line := range v.buf.lines { + if i < len(v.inclusionGutterMarks) && v.inclusionGutterMarks[i] { + lines = append(lines, line.cells.String()) + } + } + return lines +} + // DiffLineContent holds what one line of a rendered diff offers to a reader trying // to recover which line of which file it came from: the line's text, which can be // parsed as a unified diff when the rendering preserves one, and the OSC 1717 diff --git a/pkg/gui/controllers/commit_diff_actions.go b/pkg/gui/controllers/commit_diff_actions.go index 9e03f98bf..4e6d97011 100644 --- a/pkg/gui/controllers/commit_diff_actions.go +++ b/pkg/gui/controllers/commit_diff_actions.go @@ -5,6 +5,7 @@ import ( "path/filepath" "strings" + "github.com/jesseduffield/generics/set" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/patch" "github.com/jesseduffield/lazygit/pkg/gocui" @@ -104,6 +105,10 @@ func (self *CommitDiffActions) PrimaryAction(pane types.DiffPaneContext, firstLi patchBuilder.Reset() } + // The diff on screen is the one the marks belong to, so they can be brought up + // to date at once rather than waiting for the render below. + self.c.Helpers().DiffLine.RefreshInclusionGutter() + // The selection moves on past the lines just toggled, to the next change of // the diff — which is still there, a toggle leaving the diff as it was, so // hold input back until it has moved: a second press meanwhile would toggle @@ -201,6 +206,37 @@ func (self *CommitDiffActions) DiscardSelectionDisabledReason(pane types.DiffPan return nil } +// PatchInclusion says which lines of the commit's diff are in the custom patch being +// built from it. nil when there is no such patch: none is being built at all, or the one +// being built is of another diff, whose lines are not these however alike they look. +func (self *CommitDiffActions) PatchInclusion() func(types.DiffLineInfo) bool { + patchBuilder := self.c.Git().Patch.PatchBuilder + target := self.target() + if !patchBuilder.Active() || target == nil { + return nil + } + from, reverse := self.patchEndpoints(target) + if patchBuilder.NewPatchRequired(from, target.to, reverse) { + return nil + } + + // Which lines of a file are in the patch is asked of the patch builder per file, and + // a diff can span many, so each is asked about when a line of it first comes up. + includedByPath := map[string]*set.Set[patch.LineIdentity]{} + return func(info types.DiffLineInfo) bool { + path := self.patchBuilderPath(info.Path) + if path == "" { + return false + } + included, asked := includedByPath[path] + if !asked { + included = set.NewFromSlice(patchBuilder.IncludedLineIdentities(path)) + includedByPath[path] = included + } + return included.Includes(info.PatchLineIdentity()) + } +} + // togglePatchLines takes the given lines of the commit's diff into the custom patch, or // out of it. The first line of the selection decides which of the two happens, once for // the whole selection: pointing at a line that is already in the patch takes the whole diff --git a/pkg/gui/controllers/helpers/diff_line_selection.go b/pkg/gui/controllers/helpers/diff_line_selection.go index a8b465140..62108f96d 100644 --- a/pkg/gui/controllers/helpers/diff_line_selection.go +++ b/pkg/gui/controllers/helpers/diff_line_selection.go @@ -131,3 +131,63 @@ func (self *DiffLineHelper) SelectedHunkBounds(view *gocui.View) (int, int, bool } return self.ChangeBlockBounds(view, anchor) } + +// RefreshInclusionGutter updates the marks drawn over the diff in the main pane, which +// say which of its lines are in the custom patch being built from it. +// +// They are shown while the focused main view holds the focus — either of its panes, so +// that moving between the diff and the patch previewed beside it doesn't make them come +// and go — and only over a diff a patch is being built from: a patch built from some +// other commit says nothing about the lines of this one. +// +// Call it whenever either of those can have changed: as a pane's content settles, when +// the focus arrives or leaves, and when the patch itself changes. +func (self *DiffLineHelper) RefreshInclusionGutter() { + view := self.c.Contexts().Normal.GetView() + + included := self.patchInclusion() + if included == nil { + view.SetInclusionGutter(false, nil) + return + } + + resolved := self.resolveDiffLines(view.DiffLineContents()) + marks := make([]bool, len(resolved)) + showsChanges := false + for i, row := range resolved { + if !row.ok || !row.info.IsChange() { + continue + } + showsChanges = true + marks[i] = included(row.info) + } + + // Nothing to mark and nowhere to mark it: the pane is showing a message rather than + // a diff, or a diff with nothing in it. + if !showsChanges { + view.SetInclusionGutter(false, nil) + return + } + view.SetInclusionGutter(true, marks) +} + +// patchInclusion asks the panel whose diff the focused main view is showing which of +// that diff's lines are in the custom patch being built from it, and answers nil where +// there is no such patch — including when the focus is elsewhere, the marks being an +// affordance of the focused view. +func (self *DiffLineHelper) patchInclusion() func(types.DiffLineInfo) bool { + if !self.mainViewIsFocused() { + return nil + } + // The panel beneath is found from the pane that holds the focus, which is not always + // the one the diff is in: moving to the pane beside it takes the other off the stack. + sidePanel := self.c.Context().NextInStack(self.c.Context().CurrentStatic()) + if sidePanel == nil { + return nil + } + actions, ok := sidePanel.GetFocusedMainViewDiffSource().(types.FocusedMainViewActions) + if !ok { + return nil + } + return actions.PatchInclusion() +} diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index b3b402412..c44f4dd61 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -248,6 +248,14 @@ func (self *MainViewController) Context() types.Context { return self.context } +// GetOnFocus brings on the marks over the lines that are in the custom patch, which +// are an affordance of the focused view, so they arrive with the focus. +func (self *MainViewController) GetOnFocus() func(types.OnFocusOpts) { + return func(types.OnFocusOpts) { + self.c.Helpers().DiffLine.RefreshInclusionGutter() + } +} + func (self *MainViewController) togglePanel() error { if !self.otherContext.GetView().Visible { return nil @@ -564,6 +572,10 @@ func (self *MainViewController) GetOnFocusLost() func(types.OnFocusLostOpts) { self.draggingWithMouse = false self.c.GocuiGui().CancelMouseCapture() } + // Where the focus has gone is already known here, so asking again keeps the + // patch marks over a move to the pane beside this one, and takes them away + // when the focus leaves the pair. + self.c.Helpers().DiffLine.RefreshInclusionGutter() } } diff --git a/pkg/gui/controllers/working_tree_diff_actions.go b/pkg/gui/controllers/working_tree_diff_actions.go index d19cd205b..c4ec68b3f 100644 --- a/pkg/gui/controllers/working_tree_diff_actions.go +++ b/pkg/gui/controllers/working_tree_diff_actions.go @@ -188,6 +188,12 @@ func (self *WorkingTreeDiffActions) EditHunk( return nil } +// PatchInclusion is nil: a custom patch is built from a commit's diff, never from the +// working tree's, so no line of this diff is ever in one. +func (self *WorkingTreeDiffActions) PatchInclusion() func(types.DiffLineInfo) bool { + return nil +} + // diffLineSelection resolves what the user has selected in a pane of the focused main // view to the change lines to act on, and reports whether they are the staged side of // the diff — which is a question about the pane, so it is the same for every file of a diff --git a/pkg/gui/main_panels.go b/pkg/gui/main_panels.go index 9c1db7290..6160ecfb9 100644 --- a/pkg/gui/main_panels.go +++ b/pkg/gui/main_panels.go @@ -376,8 +376,11 @@ func (gui *Gui) clearMainView(mainContext types.Context) { } } -// updateDiffSelectionVisibility works out whether a main pane holds anything for a -// selection to sit on, from what it is now showing: only beneath a panel whose main +// updateDiffPaneDecorations re-derives what is drawn over a main pane's content, rather +// than being part of it: whether a selection is shown, and which lines are marked as +// being in the custom patch. +// +// A pane holds something for a selection to sit on only beneath a panel whose main // view is a diff, only while the pane is showing that diff rather than a message like // "No changed files", and only while the diff holds something to select — never over // one with nothing in it, such as a binary file's or an empty commit's. Whether the @@ -389,7 +392,7 @@ func (gui *Gui) clearMainView(mainContext types.Context) { // still being read can leave the question open (see diffPaneHasSomethingToSelect). The // pane never answers from the render before it, and a render that leaves the question // open is read on until it doesn't, so the answer is always about what is there. -func (gui *Gui) updateDiffSelectionVisibility(view *gocui.View, contentIsComplete bool) { +func (gui *Gui) updateDiffPaneDecorations(view *gocui.View, contentIsComplete bool) { mainContext := gui.mainContextForView(view) if mainContext == nil { return @@ -405,6 +408,12 @@ func (gui *Gui) updateDiffSelectionVisibility(view *gocui.View, contentIsComplet } else { gui.readOnUntilTheDiffPaneCanTell(view) } + + // The marks are over the diff in the upper pane; the lower one shows the patch + // they are marks of. + if view == gui.Views.Main { + gui.helpers.DiffLine.RefreshInclusionGutter() + } } // dropAnAnswerAboutAnotherRender takes away what the pane worked out about the content diff --git a/pkg/gui/tasks_adapter.go b/pkg/gui/tasks_adapter.go index e89a83a47..67614d3bf 100644 --- a/pkg/gui/tasks_adapter.go +++ b/pkg/gui/tasks_adapter.go @@ -94,7 +94,7 @@ func (gui *Gui) newStringTaskWithoutScroll(view *gocui.View, str string) error { f := func(tasks.TaskOpts) error { return gui.g.OnUIThreadAndWaitBackground(func() { gui.c.SetViewContent(view, str) - gui.updateDiffSelectionVisibility(view, true) + gui.updateDiffPaneDecorations(view, true) gui.reApplySearch(view) }) } @@ -116,7 +116,7 @@ func (gui *Gui) newStringTaskWithScroll(view *gocui.View, str string, originX in return gui.g.OnUIThreadAndWaitBackground(func() { gui.c.SetViewContent(view, str) view.SetOrigin(originX, originY) - gui.updateDiffSelectionVisibility(view, true) + gui.updateDiffPaneDecorations(view, true) gui.reApplySearch(view) }) } @@ -138,7 +138,7 @@ func (gui *Gui) newStringTaskWithKey(view *gocui.View, str string, key string) e return gui.g.OnUIThreadAndWaitBackground(func() { gui.c.ResetViewOrigin(view) gui.c.SetViewContent(view, str) - gui.updateDiffSelectionVisibility(view, true) + gui.updateDiffPaneDecorations(view, true) gui.reApplySearch(view) }) } @@ -177,7 +177,7 @@ func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager { // to say whether there is anything to select, and for a diff that // opens with a long diffstat it isn't. gui.c.OnUIThreadContentOnly(func() error { - gui.updateDiffSelectionVisibility(view, false) + gui.updateDiffPaneDecorations(view, false) return nil }) }, @@ -196,7 +196,7 @@ func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager { view.SetOrigin(0, newOriginY) } - gui.updateDiffSelectionVisibility(view, true) + gui.updateDiffPaneDecorations(view, true) gui.clampDiffSelectionToContent(view) gui.reApplySearch(view) }, @@ -210,7 +210,7 @@ func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager { // The content the pane is being given is on display from here on, so // what is drawn over it is settled against that content rather than // against the render before it. - gui.updateDiffSelectionVisibility(view, false) + gui.updateDiffPaneDecorations(view, false) }, func() gocui.Task { // A background task: rendering content into a view is display diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index 93cbd53d8..b6058cbd3 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -264,6 +264,12 @@ type FocusedMainViewActions interface { // is, and nil when it can. Taking lines out of a commit means rewriting it, which // isn't always something we may do; the working tree has no such condition. DiscardSelectionDisabledReason(pane DiffPaneContext) *DisabledReason + + // PatchInclusion says which lines of the diff this panel shows are in the custom + // patch being built from it. The marks over those lines are drawn from this. nil + // where nothing about this diff is being built into a patch, which is always so + // for a diff that can't be. + PatchInclusion() func(info DiffLineInfo) bool } type IListContext interface { diff --git a/pkg/integration/components/view_driver.go b/pkg/integration/components/view_driver.go index 9388f6aa0..e270e75d9 100644 --- a/pkg/integration/components/view_driver.go +++ b/pkg/integration/components/view_driver.go @@ -404,6 +404,48 @@ func (self *ViewDriver) Content(matcher *TextMatcher) *ViewDriver { return self } +// MarkedLines asserts which lines of the view are marked as being in the custom patch +// being built. The marks are drawn over the content rather than being part of it, so +// they are read from the view rather than matched against what Content returns. +func (self *ViewDriver) MarkedLines(matchers ...*TextMatcher) *ViewDriver { + self.validateMatchersPassed(matchers) + + self.t.assertWithRetries(func() (bool, string) { + markedLines := self.getView().MarkedLines() + + markedContent := strings.Join(markedLines, "\n") + expectedContent := expectedContentFromMatchers(matchers) + + if len(markedLines) != len(matchers) { + return false, fmt.Sprintf("%s: Expected the following lines to be marked as being in the custom patch:\n-----\n%s\n-----\nBut got:\n-----\n%s\n-----", self.context, expectedContent, markedContent) + } + + for i, line := range markedLines { + ok, message := matchers[i].test(line) + if !ok { + return false, fmt.Sprintf("%s: Error: %s. Expected the following lines to be marked as being in the custom patch:\n-----\n%s\n-----\nBut got:\n-----\n%s\n-----", self.context, message, expectedContent, markedContent) + } + } + + return true, "" + }) + + return self +} + +// NoMarkedLines asserts that no line of the view is marked as being in the custom +// patch, which is also what a view showing no marks at all reports. +func (self *ViewDriver) NoMarkedLines() *ViewDriver { + self.t.assertWithRetries(func() (bool, string) { + markedLines := self.getView().MarkedLines() + return len(markedLines) == 0, fmt.Sprintf( + "%s: Expected no line to be marked as being in the custom patch, but these were:\n-----\n%s\n-----", + self.context, strings.Join(markedLines, "\n")) + }) + + return self +} + // SelectionIsActive asserts that the view draws its selection as the one the user // is working in. These three assertions read the highlight flags rather than the // selected lines, which say nothing about whether the selection is drawn at all. diff --git a/pkg/integration/tests/main_view/build_patch_from_a_commits_diff.go b/pkg/integration/tests/main_view/build_patch_from_a_commits_diff.go index b60ff404c..acc021e4a 100644 --- a/pkg/integration/tests/main_view/build_patch_from_a_commits_diff.go +++ b/pkg/integration/tests/main_view/build_patch_from_a_commits_diff.go @@ -53,6 +53,10 @@ var BuildPatchFromACommitsDiff = NewIntegrationTest(NewIntegrationTestArgs{ Contains("-one"), Contains(" two"), ) + // The line that is in the patch is marked as such over the diff itself. + t.Views().Main().MarkedLines( + Contains("-one"), + ) // The addition of the same modification goes in too, and the patch holds both. t.Views().Main(). @@ -67,6 +71,10 @@ var BuildPatchFromACommitsDiff = NewIntegrationTest(NewIntegrationTestArgs{ Contains("+ONE"), Contains(" two"), ) + t.Views().Main().MarkedLines( + Contains("-one"), + Contains("+ONE"), + ) // Pointing at a line that is in the patch takes it back out. t.Views().Main(). @@ -80,6 +88,9 @@ var BuildPatchFromACommitsDiff = NewIntegrationTest(NewIntegrationTestArgs{ Contains(" two"), ). Content(DoesNotContain("+ONE")) + t.Views().Main().MarkedLines( + Contains("-one"), + ) // Taking the last line out ends the patch, so the pane previewing it goes away. t.Views().Main(). @@ -88,5 +99,6 @@ var BuildPatchFromACommitsDiff = NewIntegrationTest(NewIntegrationTestArgs{ PressPrimaryAction() t.Views().Information().Content(DoesNotContain("Building patch")) + t.Views().Main().NoMarkedLines() }, }) diff --git a/pkg/integration/tests/main_view/patch_marks_follow_a_renderer_switch.go b/pkg/integration/tests/main_view/patch_marks_follow_a_renderer_switch.go new file mode 100644 index 000000000..4e49c321b --- /dev/null +++ b/pkg/integration/tests/main_view/patch_marks_follow_a_renderer_switch.go @@ -0,0 +1,58 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var PatchMarksFollowARendererSwitch = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Switching diff renderers mid-build leaves the marks on the lines that are in the custom patch, wherever the new rendering puts them", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(cfg *config.AppConfig) { + cfg.GetUserConfig().Gui.UseHunkModeInStagingView = false + // Two renderers that announce the metadata protocol — so that focusing the main + // view keeps their output rather than falling back to git's own — and pass the + // diff through under a banner of their own. The second one's banner is a line + // longer, so every line of the diff it renders is a line further down than the + // first one's. + cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{ + {Name: "one", Command: `printf '\033]1717;1\007RENDERED BY ONE\n'; cat`}, + {Name: "two", Command: `printf '\033]1717;1\007RENDERED BY TWO\nAND ONE MORE LINE\n'; cat`}, + } + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "one\nTWO\nthree\n") + shell.Commit("second commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + Content(Contains("RENDERED BY ONE")). + SelectedLines( + Contains("-two"), + ). + PressPrimaryAction(). + MarkedLines( + Contains("-two"), + ). + Press(keys.Universal.CycleDiffRenderers) + + t.ExpectToast(Equals("Diff renderer: two (2 of 2)")) + + // The marks are of lines of the diff, not of rows of the rendering, so the new + // rendering has them on the same line of the file. + t.Views().Main(). + Content(Contains("AND ONE MORE LINE")). + MarkedLines( + Contains("-two"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/patch_marks_show_while_the_diff_is_focused.go b/pkg/integration/tests/main_view/patch_marks_show_while_the_diff_is_focused.go new file mode 100644 index 000000000..c0870f63c --- /dev/null +++ b/pkg/integration/tests/main_view/patch_marks_show_while_the_diff_is_focused.go @@ -0,0 +1,61 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var PatchMarksShowWhileTheDiffIsFocused = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "The marks over the lines in the custom patch are shown while either pane of the focused main view holds the focus, and not once it leaves", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInStagingView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "one\nTWO\nthree\n") + shell.Commit("second commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("-two"), + ). + PressPrimaryAction(). + MarkedLines( + Contains("-two"), + ). + // Moving to the pane previewing the patch is still working on the same patch, + // so the marks stay. + Press(keys.Universal.TogglePanel) + + t.Views().Secondary().IsFocused() + t.Views().Main().MarkedLines( + Contains("-two"), + ) + + // Leaving the diff behind takes them away: they say what pressing space here + // would act on. + t.Views().Secondary().Press(keys.Universal.Return) + + t.Views().Commits().IsFocused() + t.Views().Main().NoMarkedLines() + + // And they are back with the focus. + t.Views().Commits().Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + MarkedLines( + Contains("-two"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 9bd83087b..009036d45 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -409,6 +409,8 @@ var tests = []*components.IntegrationTest{ main_view.NoSelectionOverACommitLog, main_view.NoSelectionOverAConflictHint, main_view.NoSelectionWhenNoChanges, + main_view.PatchMarksFollowARendererSwitch, + main_view.PatchMarksShowWhileTheDiffIsFocused, main_view.RangeSelectDiffLines, main_view.RawFallbackUnderAnExternalDiff, main_view.ResetAPatchBuiltFromACommitsDiff,