diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index fd9f6da55..e08ffd576 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -621,6 +621,12 @@ func (v *View) SetRangeSelectStart(rangeSelectStartY int) { v.rangeSelectStartY = rangeSelectStartY } +// RangeSelectStartY returns the view line the range selection is anchored on, +// or -1 when there is no range. +func (v *View) RangeSelectStartY() int { + return v.rangeSelectStartY +} + func (v *View) CancelRangeSelect() { v.rangeSelectStartY = -1 } diff --git a/pkg/gui/controllers/helpers/diff_line_queries.go b/pkg/gui/controllers/helpers/diff_line_queries.go index 478adcab2..3f7522ec2 100644 --- a/pkg/gui/controllers/helpers/diff_line_queries.go +++ b/pkg/gui/controllers/helpers/diff_line_queries.go @@ -2,6 +2,7 @@ package helpers import ( "github.com/jesseduffield/lazygit/pkg/gocui" + "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/samber/lo" ) @@ -44,6 +45,33 @@ func (self *DiffLineHelper) FirstChangeLineInView(view *gocui.View) (int, bool) return 0, false } +// FirstChangeBlockInView returns the view line of the first change block on screen: +// the first one that *begins* in the viewport, and failing that the one that reaches +// into the viewport from above, whose start is off screen. Hunk mode wants that order +// for the block it offers up on focus: preferably a block whose beginning the user can +// see, rather than the tail of one they have scrolled past the start of. The block +// bleeding in from above is kept as the answer for a change too long to fit on screen, +// where there is no other. ok is false when the viewport shows no change line. +func (self *DiffLineHelper) FirstChangeBlockInView(view *gocui.View) (int, bool) { + top, bottom, ok := visibleBufferLines(view) + if !ok { + return 0, false + } + + isChange := self.changeLines(view) + for i := top; i <= min(bottom, len(isChange)-1); i++ { + if isChange[i] && (i == 0 || !isChange[i-1]) { + return view.ViewLineForBufferLine(i) + } + } + // A block covering the top line is one that began above it: nothing else can put a + // change there once no block starts on screen. + if top < len(isChange) && isChange[top] { + return view.ViewLineForBufferLine(top) + } + return 0, false +} + // visibleBufferLines returns the first and last line of view's content that the // viewport shows any part of, for the queries that only care about what the user can // see. The last line is the one at the bottom edge, or the content's last when the @@ -78,6 +106,48 @@ func (self *DiffLineHelper) IsChangeLine(view *gocui.View, viewLineIdx int) bool return ok && info.IsChange() } +// IsSingleHunkForWholeFile reports whether the file the given change line belongs to +// is shown as one solid block of changes — every row of its diff a change of the same +// kind, no context — which is what a newly added or deleted file looks like. That is +// the case where widening the selection to the change block would select the file +// entire, so hunk mode drops to a single line there instead. It asks of a rendered +// diff the question patch.Patch.IsSingleHunkForWholeFile asks of a patch. +// +// It says false while the diff is still being read in, since the rows that would +// answer otherwise — a context line, a change of the other kind — may not have +// arrived yet. That errs towards hunk mode, which is what the user asked for. +func (self *DiffLineHelper) IsSingleHunkForWholeFile(view *gocui.View, changeViewLine int) bool { + if manager := self.c.GetViewBufferManagerForView(view); manager != nil && manager.IsLoading() { + return false + } + + anchor, ok := view.BufferLineForViewLine(changeViewLine) + if !ok { + return false + } + resolved := self.resolveDiffLines(view.DiffLineContents()) + if anchor >= len(resolved) || !resolved[anchor].ok { + return false + } + + // The question is per file: a commit's diff may hold a newly added file next to an + // edited one. + path := resolved[anchor].info.Path + kind := resolved[anchor].info.Type + for _, row := range resolved { + if !row.ok || row.info.Path != path { + continue + } + if row.info.Type == types.DiffLineContext { + return false + } + if row.info.IsChange() && row.info.Type != kind { + return false + } + } + return true +} + // ChangeBlockBounds returns the inclusive view-line range of the change block to // select in hunk mode around anchorViewLine. A change block is lazygit's notion of a // hunk — a run of consecutive added or deleted lines bounded by context, of which a @@ -121,6 +191,17 @@ func (self *DiffLineHelper) ChangeBlockBounds(view *gocui.View, anchorViewLine i return startView, endView, true } +// SelectedHunkBounds returns the change block selected in hunk mode. The range +// anchor stays on the block's far end when a click moves the cursor before its +// handler runs, so it still identifies the selected block. +func (self *DiffLineHelper) SelectedHunkBounds(view *gocui.View) (int, int, bool) { + anchor := view.RangeSelectStartY() + if anchor < 0 { + return 0, 0, false + } + return self.ChangeBlockBounds(view, anchor) +} + // AdjacentChangeBlock returns the view line to move to for next/previous change-block // navigation in view's rendered diff, starting from anchorViewLine. A change block is // lazygit's notion of a hunk (see ChangeBlockBounds). forward=true targets the start diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index 67b8b6929..91cd66ae4 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -241,19 +241,25 @@ func (self *MainViewController) onClickInOtherViewOfMainViewPair(opts gocui.View } // selectClickedDiffLine sets the focused main view's selection from a click at the -// given view line. In hunk mode a click on a change line keeps hunk mode and selects -// that whole block, so clicking from hunk to hunk stays ready to act on one; a click -// on context drops to a single line, as does any click when we weren't in hunk mode — -// the click points at that line precisely, e.g. to edit it. +// given view line. In hunk mode, clicking inside the selected block collapses it to +// that line; clicking a change line outside it keeps hunk mode and selects that block. +// A click on context, or any click outside hunk mode, selects just that line too. func (self *MainViewController) selectClickedDiffLine(viewLine int) { if !self.isDiffView() { return } view := self.context.GetView() - if self.diffSelectState().Mode == types.DiffSelectModeHunk && - self.c.Helpers().DiffLine.IsChangeLine(view, viewLine) { - self.selectHunkAround(viewLine, false) - return + if self.diffSelectState().Mode == types.DiffSelectModeHunk { + if start, end, ok := self.c.Helpers().DiffLine.SelectedHunkBounds(view); ok && + viewLine >= start && viewLine <= end { + self.context.ResetDiffSelectMode() + showSelectionAtLine(view, viewLine, false) + return + } + if self.c.Helpers().DiffLine.IsChangeLine(view, viewLine) { + self.selectHunkAround(viewLine, false) + return + } } self.context.ResetDiffSelectMode() showSelectionAtLine(view, viewLine, false) @@ -269,6 +275,11 @@ func (self *MainViewController) selectClickedDiffLine(viewLine int) { // view going where the selection would like to be. With no change line on screen at // all — a long stretch of context — it lands on the middle visible line, the likeliest // one to be the one being read. +// +// With hunk mode configured as the default the selection widens to the whole change +// block: keyboard focus lands on the first block on screen, and a click on a change +// line selects that line's block, ready to act on. A click on context still selects +// just that line — the click points at it precisely, so it stays editable. func establishDiffSelection(c *ControllerCommon, mainContext *context.MainContext, clickedViewLine int) { mainContext.ResetDiffSelectMode() view := mainContext.GetView() @@ -281,18 +292,51 @@ func establishDiffSelection(c *ControllerCommon, mainContext *context.MainContex } if clickedViewLine >= 0 { + if hunkModeApplies(c, view, clickedViewLine) && + c.Helpers().DiffLine.IsChangeLine(view, clickedViewLine) { + mainContext.DiffSelectState().Mode = types.DiffSelectModeHunk + selectDiffHunk(c, mainContext, clickedViewLine, false) + return + } showSelectionAtLine(view, clickedViewLine, false) return } - target, ok := c.Helpers().DiffLine.FirstChangeLineInView(view) + target, ok := changeToSelectOnScreen(c, view) if !ok { showSelectionAtLine(view, view.MiddleVisibleLineIdx(), false) return } + if hunkModeApplies(c, view, target) { + mainContext.DiffSelectState().Mode = types.DiffSelectModeHunk + selectDiffHunk(c, mainContext, target, false) + return + } showSelectionAtLine(view, target, false) } +// changeToSelectOnScreen returns the change line keyboard focus establishes the +// selection on. In hunk mode that is the first block that begins on screen, so that +// the block being offered up is one the user can see the extent of, falling back to a +// block that reaches into the view from above — a change longer than the screen, where +// there is nothing else to offer. Line by line it is simply the first change line on +// screen. ok is false when the viewport shows no change at all. +func changeToSelectOnScreen(c *ControllerCommon, view *gocui.View) (int, bool) { + if c.UserConfig().Gui.UseHunkModeInStagingView { + return c.Helpers().DiffLine.FirstChangeBlockInView(view) + } + return c.Helpers().DiffLine.FirstChangeLineInView(view) +} + +// hunkModeApplies reports whether an established selection should start out as the +// whole change block around the given change line. That's what the config asks for, +// except over a file shown as one solid block of changes, where it would select the +// whole file — see DiffLineHelper.IsSingleHunkForWholeFile. +func hunkModeApplies(c *ControllerCommon, view *gocui.View, changeViewLine int) bool { + return c.UserConfig().Gui.UseHunkModeInStagingView && + !c.Helpers().DiffLine.IsSingleHunkForWholeFile(view, changeViewLine) +} + // showSelectionAtLine moves the focused main view's selection to the given view line, // clamped to the content. scrollIntoView scrolls the line into view when it's // off-screen, for navigating to it; a click leaves it false, the clicked line being on diff --git a/pkg/integration/tests/main_view/select_hunk_on_focusing_main_view.go b/pkg/integration/tests/main_view/select_hunk_on_focusing_main_view.go new file mode 100644 index 000000000..51ca4b04e --- /dev/null +++ b/pkg/integration/tests/main_view/select_hunk_on_focusing_main_view.go @@ -0,0 +1,67 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var SelectHunkOnFocusingMainView = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "When hunk mode is the default, focusing the main view selects the first whole change block", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInStagingView = true + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\nfour\nfive\nsix\nseven\neight\nnine\nten\n") + shell.Commit("one") + + shell.UpdateFile("file1", "one\ntwo\nTHREE\nfour\nfive\nsix\nseven\neight\nNINE\nten\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Press(keys.Universal.FocusMainView) + + // No key press needed: the whole block is selected just by focusing. + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("-three"), + Contains("+THREE"), + ). + // Hunk mode being the configured default, it isn't something escape gives up: + // escape leaves the view. + Press(keys.Universal.Return) + + t.Views().Files(). + IsFocused(). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + // A click on a change line keeps hunk mode and selects that line's block. + Click(0, 14). + SelectedLines( + Contains("-nine"), + Contains("+NINE"), + ). + // A click inside the selected block collapses hunk mode to that line. + Click(0, 15). + SelectedLines( + Contains("+NINE"), + ). + // Switch back to hunk mode so the context click below proves that it gives + // hunk mode up, rather than merely keeping line mode. + Press(keys.Main.ToggleSelectHunk). + SelectedLines( + Contains("-nine"), + Contains("+NINE"), + ). + // A click on a context line points at it precisely, so it selects that line. + Click(0, 12). + SelectedLines( + Contains(" seven"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/select_line_when_whole_file_is_one_hunk.go b/pkg/integration/tests/main_view/select_line_when_whole_file_is_one_hunk.go new file mode 100644 index 000000000..af176bdd8 --- /dev/null +++ b/pkg/integration/tests/main_view/select_line_when_whole_file_is_one_hunk.go @@ -0,0 +1,43 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var SelectLineWhenWholeFileIsOneHunk = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Hunk mode falls back to a single line for a file that is one solid block of changes, rather than selecting all of it", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInStagingView = true + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("one") + shell.CreateFileAndAdd("added", "one\ntwo\nthree\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Lines( + Contains("added").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + // Every line of the file is an addition, so widening to the change block would + // select the file entire; hunk mode gives way to a single line. + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("+one"), + ). + // Toggling hunk mode on explicitly still selects the whole block: the fallback + // is about what the default does, not about forbidding the selection. + Press(keys.Main.ToggleSelectHunk). + SelectedLines( + Contains("+one"), + Contains("+two"), + Contains("+three"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/select_visible_hunk_on_focusing_main_view.go b/pkg/integration/tests/main_view/select_visible_hunk_on_focusing_main_view.go new file mode 100644 index 000000000..ca3dedefe --- /dev/null +++ b/pkg/integration/tests/main_view/select_visible_hunk_on_focusing_main_view.go @@ -0,0 +1,93 @@ +package main_view + +import ( + "fmt" + "strings" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var SelectVisibleHunkOnFocusingMainView = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Focusing the main view in hunk mode picks a block that begins on screen, leaving the diff where it is", + ExtraCmdArgs: []string{}, + Skip: false, + Width: 120, + Height: 30, + SetupConfig: func(cfg *config.AppConfig) { + cfg.GetUserConfig().Gui.UseHunkModeInStagingView = true + // One line per scroll, so that the test can put the top of the view exactly + // where it wants it, and enough context to scroll about within one hunk. + cfg.GetUserConfig().Gui.ScrollHeight = 1 + cfg.GetUserConfig().Git.DiffContextSize = 20 + }, + SetupRepo: func(shell *Shell) { + lines := make([]string, 80) + for i := range lines { + lines[i] = fmt.Sprintf("line%02d", i+1) + } + shell.CreateFileAndAdd("file1", strings.Join(lines, "\n")+"\n") + shell.Commit("one") + + for _, i := range []int{10, 20, 70} { + lines[i-1] = strings.ToUpper(lines[i-1]) + } + shell.UpdateFile("file1", strings.Join(lines, "\n")+"\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + scrollDown := func(lines int) { + for range lines { + t.Views().Files().Press(keys.Universal.ScrollDownMain) + } + } + + t.Views().Files().IsFocused() + + // The top of the view is the second line of the first change block, so that + // block is only half on screen; the second one begins below it, in full. + scrollDown(15) + + t.Views().Main(). + OriginY(15). + Tap(func() { + t.Views().Files().Press(keys.Universal.FocusMainView) + }). + IsFocused(). + SelectedLines( + Contains("-line20"), + Contains("+LINE20"), + ). + OriginY(15). + PressEscape() + + // Now nothing begins on screen: the second block starts just above the top and + // the third change is far below. Only the half-visible block is on screen, so + // it is selected, with its first line off screen, since the view stays put. + scrollDown(11) + + t.Views().Main(). + OriginY(26). + Tap(func() { + t.Views().Files().Press(keys.Universal.FocusMainView) + }). + IsFocused(). + SelectedLines( + Contains("-line20"), + Contains("+LINE20"), + ). + SelectedLineIdx(25). + OriginY(26). + PressEscape() + + // And a click inside a block that begins above the viewport selects the whole + // block without pulling the view up to its start either. + t.Views().Main(). + Click(0, 0). + IsFocused(). + SelectedLines( + Contains("-line20"), + Contains("+LINE20"), + ). + OriginY(26) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 1fe47c203..7512f68d0 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -378,8 +378,11 @@ var tests = []*components.IntegrationTest{ main_view.SelectDiffLines, main_view.SelectHunkBelowLastChange, main_view.SelectHunkInDiff, + main_view.SelectHunkOnFocusingMainView, main_view.SelectInADiffReadInPart, + main_view.SelectLineWhenWholeFileIsOneHunk, main_view.SelectVisibleChangeOnFocusingMainView, + main_view.SelectVisibleHunkOnFocusingMainView, main_view.SelectionCommandsOnlyWhereTheyApply, main_view.SelectionOverTheCustomPatch, misc.ConfirmOnQuit,