From e4f819904912d3aee1ab6b52154ada5aebad144c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 19 Aug 2026 17:45:06 +0200 Subject: [PATCH] Mark the lines of a commit's diff that are in the custom patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A patch built from what is on screen has to show what is in it, over the diff those lines were taken from — which may be a diff renderer's rendering of it, whose bytes are none of ours to touch. So the marks are drawn in a gutter over the content, from the lines the patch holds rather than from where they were drawn last: they are worked out again whenever a pane's content settles, which is what keeps them right across a renderer switch, a change of context size, or a walk through the commits, and whenever the patch itself changes. They come and go with the focus, being what the space key in the focused view would act on, and stay while the focus moves between the diff and the patch previewed beside it. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/gocui/view.go | 20 ++++++ pkg/gui/controllers/commit_diff_actions.go | 36 +++++++++++ .../helpers/diff_line_selection.go | 60 ++++++++++++++++++ pkg/gui/controllers/main_view_controller.go | 12 ++++ .../controllers/working_tree_diff_actions.go | 6 ++ pkg/gui/main_panels.go | 15 ++++- pkg/gui/tasks_adapter.go | 12 ++-- pkg/gui/types/context.go | 6 ++ pkg/integration/components/view_driver.go | 42 +++++++++++++ .../build_patch_from_a_commits_diff.go | 12 ++++ .../patch_marks_follow_a_renderer_switch.go | 58 ++++++++++++++++++ ...ch_marks_show_while_the_diff_is_focused.go | 61 +++++++++++++++++++ pkg/integration/tests/test_list.go | 2 + 13 files changed, 333 insertions(+), 9 deletions(-) create mode 100644 pkg/integration/tests/main_view/patch_marks_follow_a_renderer_switch.go create mode 100644 pkg/integration/tests/main_view/patch_marks_show_while_the_diff_is_focused.go 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,