diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 1dd1aabb2..337826b2b 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -2339,6 +2339,11 @@ func (v *View) SelectedLineIdx() int { return seletedLineIdx } +// IsLineVisible reports whether the given view line is one of those on screen. +func (v *View) IsLineVisible(viewLine int) bool { + return viewLine >= v.OriginY() && viewLine < v.OriginY()+v.InnerHeight() +} + // MiddleVisibleLineIdx returns the view line halfway down the visible content. It // stands in for a cursor in a view that has none: of the lines on screen, the one in // the middle is the likeliest to be the one being read. diff --git a/pkg/gui/controllers/context_lines_controller.go b/pkg/gui/controllers/context_lines_controller.go index 022364c07..f7077b299 100644 --- a/pkg/gui/controllers/context_lines_controller.go +++ b/pkg/gui/controllers/context_lines_controller.go @@ -83,6 +83,11 @@ func (self *ContextLinesController) applyChange() error { case context.STAGING_MAIN_CONTEXT_KEY, context.STAGING_SECONDARY_CONTEXT_KEY: self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.STAGING}}) default: + // The diff is about to be rendered again with more or less context around + // each change, which reads as the lines you were looking at moving up or down + // the view; keep them where they are instead. + self.c.Helpers().DiffLine.PreserveDiffPositionOnRerender(self.c.Contexts().Normal.GetView()) + self.c.Helpers().DiffLine.PreserveDiffPositionOnRerender(self.c.Contexts().NormalSecondary.GetView()) currentContext.HandleRenderToMain() } return nil diff --git a/pkg/gui/controllers/helpers/diff_line_helper.go b/pkg/gui/controllers/helpers/diff_line_helper.go index 212cbff0b..533b63127 100644 --- a/pkg/gui/controllers/helpers/diff_line_helper.go +++ b/pkg/gui/controllers/helpers/diff_line_helper.go @@ -66,14 +66,21 @@ func (self *DiffLineHelper) GetDiffLineInfo(view *gocui.View, viewLineIdx int) ( // (a side-by-side row shows a deletion and the addition replacing it); the leftmost // is the one a reader would call the row's own, so it is the row's identity. func (self *DiffLineHelper) diffLineInfoFromRecords(metadata []string) (types.DiffLineInfo, bool) { - if len(metadata) == 0 { + identities := self.diffLineIdentitiesFromRecords(metadata) + if len(identities) == 0 { return types.DiffLineInfo{}, false } - parsed, ok := parseDiffLineMetadata(metadata[0]) - if !ok { - return types.DiffLineInfo{}, false - } - return self.diffLineInfo(parsed), true + return identities[0], true +} + +// diffLineIdentitiesFromRecords recovers the identity of every diff line the row's +// records state, left to right. Which of them a reader is after depends on the +// reader: the one the row leads with is the row's own identity (see +// diffLineInfoFromRecords), while a reader looking for a particular line has to +// consider them all, since which of a modification's two halves leads a row is up to +// the rendering. +func (self *DiffLineHelper) diffLineIdentitiesFromRecords(metadata []string) []types.DiffLineInfo { + return self.diffLineInfos(parseDiffLineRecords(metadata)) } // resolvedDiffLine is one rendered row's recovered identity, plus whether it could @@ -86,36 +93,59 @@ type resolvedDiffLine struct { // resolveDiffLines recovers the identity of every row of a rendered diff in one // pass, indexed 1:1 with contents. It is the batch form of GetDiffLineInfo, for the // whole-buffer scans (which change lines are where, which file each row belongs -// to), and reads the rendering the same way: by the renderer's records, or by -// parsing it as a unified diff (see renderingStatesDiffLines). Resolving row by row -// would re-run the buffer parser's whole-section parse once per row — O(n²) on a -// large single-file diff — so the buffer parser runs once for the whole buffer. +// to). A row's identity is the line it leads with, of those resolveDiffLineIdentities +// finds on it. func (self *DiffLineHelper) resolveDiffLines(contents []gocui.DiffLineContent) []resolvedDiffLine { resolved := make([]resolvedDiffLine, len(contents)) - if renderingStatesDiffLines(contents) { - for i, content := range contents { - if info, ok := self.diffLineInfoFromRecords(content.Metadata); ok { - resolved[i] = resolvedDiffLine{info, true} - } - } - return resolved - } - - for i, parsed := range parseAllDiffLinesFromBuffer(diffLineTexts(contents)) { - if parsed.ok { - resolved[i] = resolvedDiffLine{self.diffLineInfo(parsed.parsed), true} + for i, identities := range self.resolveDiffLineIdentities(contents) { + if len(identities) > 0 { + resolved[i] = resolvedDiffLine{identities[0], true} } } return resolved } +// resolveDiffLineIdentities recovers every diff line each row of a rendered diff +// shows, in one pass, indexed 1:1 with contents. It reads the rendering the way +// GetDiffLineInfo does, by the renderer's records or by parsing it as a unified diff +// (see parseDiffLineIdentities), and is the form of the batch resolver for the +// readers that can't settle for the line a row leads with: looking for a remembered +// line in a new rendering has to consider both halves of a modification, since a +// side-by-side row leads with the deletion whose addition was what got remembered +// under a unified one. +func (self *DiffLineHelper) resolveDiffLineIdentities(contents []gocui.DiffLineContent) [][]types.DiffLineInfo { + identities := make([][]types.DiffLineInfo, len(contents)) + for i, parsed := range parseDiffLineIdentities(contents) { + if len(parsed) > 0 { + identities[i] = self.diffLineInfos(parsed) + } + } + return identities +} + // diffLineInfo turns a parser's result into the absolute-path identity consumers // work with. The path arrives repo-relative from the diff header, but a renderer // states it however it likes, absolute paths included. func (self *DiffLineHelper) diffLineInfo(parsed parsedDiffLine) types.DiffLineInfo { + return diffLineInfoIn(self.c.Git().RepoPaths.WorktreePath(), parsed) +} + +// diffLineInfos is diffLineInfo over every line of a row. +func (self *DiffLineHelper) diffLineInfos(parsed []parsedDiffLine) []types.DiffLineInfo { + infos := make([]types.DiffLineInfo, len(parsed)) + for i, line := range parsed { + infos[i] = self.diffLineInfo(line) + } + return infos +} + +// diffLineInfoIn is diffLineInfo against a given worktree, for the callers that can't +// ask which repo we are in where they run: a repo switch replaces it, so only the UI +// thread may read it. +func diffLineInfoIn(worktreePath string, parsed parsedDiffLine) types.DiffLineInfo { path := parsed.Path if !filepath.IsAbs(path) { - path = filepath.Join(self.c.Git().RepoPaths.WorktreePath(), path) + path = filepath.Join(worktreePath, path) } return types.DiffLineInfo{ diff --git a/pkg/gui/controllers/helpers/diff_line_parser.go b/pkg/gui/controllers/helpers/diff_line_parser.go index dd7270af2..8f8420d97 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser.go +++ b/pkg/gui/controllers/helpers/diff_line_parser.go @@ -125,6 +125,46 @@ func renderingStatesDiffLines(contents []gocui.DiffLineContent) bool { }) } +// parseDiffLineRecords parses the records a row carries, left to right, leaving out +// the ones we don't understand. A row carries more than one when the rendering puts +// two diff lines on it, as a side-by-side row does with a deletion and the addition +// replacing it. +func parseDiffLineRecords(metadata []string) []parsedDiffLine { + parsed := make([]parsedDiffLine, 0, len(metadata)) + for _, record := range metadata { + if line, ok := parseDiffLineMetadata(record); ok { + parsed = append(parsed, line) + } + } + return parsed +} + +// parseDiffLineIdentities recovers, for every row of a rendering, the diff lines it +// shows, indexed 1:1 with contents; a row that shows none we can place gets an empty +// entry. The rendering is read the way renderingStatesDiffLines settles: by the +// renderer's records, every one a row carries, or else by parsing the rendering as a +// unified diff, where each row shows one line. Each file's section is parsed once; +// resolving row by row would re-run that parse once per row, O(n²) on a large +// single-file diff. +func parseDiffLineIdentities(contents []gocui.DiffLineContent) [][]parsedDiffLine { + identities := make([][]parsedDiffLine, len(contents)) + if renderingStatesDiffLines(contents) { + for i, content := range contents { + if parsed := parseDiffLineRecords(content.Metadata); len(parsed) > 0 { + identities[i] = parsed + } + } + return identities + } + + for i, parsed := range parseAllDiffLinesFromBuffer(diffLineTexts(contents)) { + if parsed.ok { + identities[i] = []parsedDiffLine{parsed.parsed} + } + } + return identities +} + // fileSectionBounds returns the half-open range [start, end) of the file section // containing targetIdx: the nearest line starting a section at or above it, up to // where that section ends. start is -1 when targetIdx is above the first file diff --git a/pkg/gui/controllers/helpers/diff_line_parser_test.go b/pkg/gui/controllers/helpers/diff_line_parser_test.go index f320a6ef5..5c5c41297 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser_test.go +++ b/pkg/gui/controllers/helpers/diff_line_parser_test.go @@ -420,3 +420,61 @@ func TestRenderingStatesDiffLines(t *testing.T) { }) } } + +func TestParseDiffLineIdentities(t *testing.T) { + row := func(text string, records ...string) gocui.DiffLineContent { + return gocui.DiffLineContent{Text: text, Metadata: records} + } + + t.Run("a rendering with records is read by them alone", func(t *testing.T) { + // A renderer's picture of a commit that adds a test whose input is a diff. The + // test's "diff --git" line is an added line of the test's file and is shown on + // a row of its own, and the rows the renderer puts between hunks carry no + // record. Parsed as a diff, that row would open a section of a file the diff + // doesn't have and claim every untagged row below it. + contents := []gocui.DiffLineContent{ + row("src/parser.rs", "1;f;;;src/parser.rs"), + row(`let input = "\`, "1;c;10;;src/parser.rs"), + row("diff --git a/img.png b/img.png", "1;a;11;;src/parser.rs"), + row("Binary files a/img.png and b/img.png differ", "1;a;12;;src/parser.rs"), + row(""), + row("fn later() {}", "1;c;40;;src/parser.rs"), + } + + assert.Equal(t, [][]parsedDiffLine{ + {{Path: "src/parser.rs", Type: types.DiffLineFileHeader}}, + {{Path: "src/parser.rs", Type: types.DiffLineContext, NewLine: 10}}, + {{Path: "src/parser.rs", Type: types.DiffLineAdded, NewLine: 11}}, + {{Path: "src/parser.rs", Type: types.DiffLineAdded, NewLine: 12}}, + nil, + {{Path: "src/parser.rs", Type: types.DiffLineContext, NewLine: 40}}, + }, parseDiffLineIdentities(contents)) + }) + + t.Run("a row with two records shows both of their lines", func(t *testing.T) { + contents := []gocui.DiffLineContent{ + row("two │ TWO", "1;d;2;2;file1", "1;a;2;;file1"), + } + + assert.Equal(t, [][]parsedDiffLine{ + { + {Path: "file1", Type: types.DiffLineDeleted, NewLine: 2, OldLine: 2}, + {Path: "file1", Type: types.DiffLineAdded, NewLine: 2}, + }, + }, parseDiffLineIdentities(contents)) + }) + + t.Run("a rendering without records is parsed as a diff", func(t *testing.T) { + bufferLines := strings.Split(twoFileDiff, "\n") + contents := make([]gocui.DiffLineContent, len(bufferLines)) + for i, line := range bufferLines { + contents[i] = row(line) + } + + identities := parseDiffLineIdentities(contents) + for i, parsed := range parseAllDiffLinesFromBuffer(bufferLines) { + assert.True(t, parsed.ok, "line %d", i) + assert.Equal(t, []parsedDiffLine{parsed.parsed}, identities[i], "line %d", i) + } + }) +} diff --git a/pkg/gui/controllers/helpers/diff_line_restore.go b/pkg/gui/controllers/helpers/diff_line_restore.go new file mode 100644 index 000000000..769364e0e --- /dev/null +++ b/pkg/gui/controllers/helpers/diff_line_restore.go @@ -0,0 +1,312 @@ +package helpers + +import ( + "github.com/jesseduffield/lazygit/pkg/gocui" + "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/tasks" + "github.com/samber/lo" +) + +// Keeping a diff view where it is when the same diff is rendered again differently. +// The line the user is on is remembered by identity (diff_line_helper.go), because a +// new rendering puts it on a different line of the view — and may not have it at all, +// which is what the fallbacks below are for. + +// diffLineAnchor is a line for a restore to land on: the identity to find it by in +// the new rendering, and the screen row it was on, so that it can be put back there. +type diffLineAnchor struct { + identity types.DiffLineInfo + row int +} + +// PreserveDiffPositionOnRerender remembers where a diff view is and puts it back +// there as it next re-renders, instead of leaving the user at the top of a new +// rendering of the diff they were already reading. Call it on the view about to be +// re-rendered, right before triggering the re-render — on both panes of the main +// window where both are being rendered again, since either of them may hold the diff +// being read; a pane that isn't showing is left alone. +// +// The line to keep is the selected one, and the middle visible line when there is no +// selection or it has been scrolled out of sight — what the user is looking at, rather +// than the view's top edge or a selection they have long since left behind. It may not +// survive the re-render: a context line goes when the context size shrinks, and a +// whole hunk or file goes when whitespace stops counting. So the lines around it come +// along as fallbacks and the view lands on the nearest one that is still there, put +// back on the screen row it was on. With none of them left — and with a renderer that +// says nothing about its rows there is nothing to look for in the first place — the +// view keeps the scroll offset it had, which is still nearer to what was being read +// than the top of the diff. +// +// An off-screen selection is still put back on the diff line it was on, wherever the +// new rendering has that; it is only the view that stays where it is. +func (self *DiffLineHelper) PreserveDiffPositionOnRerender(view *gocui.View) { + // A view that isn't the one its window is currently showing — the merge-conflicts + // view takes the main window over — isn't the one about to be re-rendered, so a + // restore installed on it would sit there and claim a later render instead. + if !view.Visible { + return + } + + // The re-render is produced by a different command from the one behind what is on + // screen — another context size, another renderer — so without being told otherwise + // it would be taken for content the user has never seen and shown from the top. + // Whether or not a line of the old rendering can be found in the new one, the offset + // into it is nearer to where they were reading than the top is. + if manager := self.c.GetViewBufferManagerForView(view); manager != nil { + manager.SetKeepScrollPositionForNextTask() + } + + showSelection := view.Highlight + selectionOnScreen := showSelection && view.IsLineVisible(view.SelectedLineIdx()) + anchorViewLine := view.MiddleVisibleLineIdx() + if selectionOnScreen { + anchorViewLine = view.SelectedLineIdx() + } + // A selection that has been scrolled away from is put back by its own lines rather + // than by the anchor's, so that it comes out on the same line of the diff without + // the view having to go there. + var selectionCandidates []diffLineAnchor + if showSelection && !selectionOnScreen { + selectionCandidates = self.nearbyDiffLines(view, view.SelectedLineIdx()) + } + + self.restoreDiffLinePositionOnRerender(view, self.nearbyDiffLines(view, anchorViewLine), + func(anchor diffLineAnchor, viewLine int) { + // Put the line back on the screen row it was on, clamped into the view for + // the fallback lines, which can come from off screen. + row := lo.Clamp(anchor.row, 0, max(0, view.InnerHeight()-1)) + view.SetOrigin(0, max(0, viewLine-row)) + if showSelection { + // A range's other end is a view line, which the new rendering has made + // mean something else, so the selection collapses to the line we landed + // on. The origin is already where it should be, so moving the cursor + // there mustn't scroll. + view.CancelRangeSelect() + view.FocusPoint(0, self.selectionLine(view, selectionCandidates, viewLine), false) + } + }) +} + +// selectionLine returns the line to put the cursor on once a re-render is on screen: +// the line the position anchor landed on, which is the selected one whenever it was +// on screen, and otherwise the nearest surviving line to where the selection was — +// found among its own candidates, since the anchor's are a search of the diff from +// somewhere else entirely. +func (self *DiffLineHelper) selectionLine( + view *gocui.View, candidates []diffLineAnchor, anchorViewLine int, +) int { + if len(candidates) == 0 { + return anchorViewLine + } + _, bufferLine := self.nearestSurvivingCandidate(view.DiffLineContents(), candidates) + if bufferLine == -1 { + return anchorViewLine + } + if viewLine, ok := view.ViewLineForBufferLine(bufferLine); ok { + return viewLine + } + return anchorViewLine +} + +// restoreDiffLinePositionOnRerender arranges for view's next re-render to land on the +// first of the given candidate lines the new rendering still has, calling place with +// that candidate and the view line it ended up on. The candidates are in priority +// order (see nearbyDiffLines); if the rendering has none of them, place isn't called +// and the view re-renders as it otherwise would. +// +// The nearest candidate is looked for as the content loads, so that the re-render can +// be revealed at the right position as soon as that line and a screenful below it +// have arrived. Only the nearest one, because the candidates aren't in load order: a +// farther one can load first, and landing on it while a nearer one is still on its +// way would be settling for worse. The rest are considered together once the whole +// rendering is there. +func (self *DiffLineHelper) restoreDiffLinePositionOnRerender( + view *gocui.View, candidates []diffLineAnchor, place func(anchor diffLineAnchor, viewLine int), +) { + manager := self.c.GetViewBufferManagerForView(view) + if manager == nil || len(candidates) == 0 { + return + } + + // The readiness check below runs on the task's own goroutine, where neither the + // view's dimensions nor the repo we are in may be read — a repo switch replaces + // the latter — so take both here, on the UI thread, for it to work from. + viewHeight := view.InnerHeight() + worktreePath := self.c.Git().RepoPaths.WorktreePath() + + // What the search of the loading content has found, and how far it has looked, so + // that each line is looked at once. + found := diffLineAnchor{} + foundLine := -1 + scanned := 0 + + manager.SetRestoreForNextTask(&tasks.RenderRestore{ + FirstPaintReady: func() bool { + if foundLine == -1 { + rows := view.OffscreenDiffLineContentsFrom(scanned) + for i, row := range rows { + if rowShowsDiffLine(row, worktreePath, candidates[0].identity) { + found, foundLine = candidates[0], scanned+i + break + } + } + scanned += len(rows) + if foundLine == -1 { + return false + } + } + // Wait for a screenful below the line as well, so that the re-render isn't + // revealed with it stranded at the bottom of a half-filled view. + return view.OffscreenLineCount() >= foundLine+viewHeight + }, + Apply: func(swapIn func()) bool { + anchor, bufferLine := found, foundLine + if bufferLine == -1 { + anchor, bufferLine = self.nearestSurvivingCandidate(view.OffscreenDiffLineContents(), candidates) + } + + swapIn() + + if bufferLine == -1 { + return false + } + viewLine, ok := view.ViewLineForBufferLine(bufferLine) + if !ok { + return false + } + place(anchor, viewLine) + return true + }, + }) +} + +// nearbyDiffLines collects the lines of view's rendered diff as candidates for a +// restore to land on, ordered by proximity to the anchor line — the anchor itself +// first, then outward, preferring at-or-below on ties — each tagged with the screen +// row it is on. A restore lands on the first of them its re-render still has, so this +// order makes it land as near as possible to where the user was. +// +// The walk covers the whole diff rather than stopping at the change lines on either +// side of the anchor, which a context-size change always keeps: ignoring whitespace +// keeps nothing in particular, and can take a hunk or a whole file out of the diff, +// leaving the nearest surviving line in a neighbouring file. +func (self *DiffLineHelper) nearbyDiffLines(view *gocui.View, anchorViewLine int) []diffLineAnchor { + anchor, ok := view.BufferLineForViewLine(anchorViewLine) + if !ok { + return nil + } + resolved := self.resolveDiffLines(view.DiffLineContents()) + if anchor >= len(resolved) { + return nil + } + rows := screenRows(view, len(resolved)) + + candidates := make([]diffLineAnchor, 0, len(resolved)) + collect := func(bufferLine int) { + if line := resolved[bufferLine]; line.ok { + candidates = append(candidates, diffLineAnchor{identity: line.info, row: rows[bufferLine]}) + } + } + collect(anchor) + for below, above := anchor+1, anchor-1; below < len(resolved) || above >= 0; below, above = below+1, above-1 { + if below < len(resolved) { + collect(below) + } + if above >= 0 { + collect(above) + } + } + return candidates +} + +// screenRows maps each line of view's content to the screen row it is drawn on. The +// lines above the visible ones get -1 and those below them the view's height, so that +// putting one of them back where it was lands it at the top or bottom edge. +func screenRows(view *gocui.View, bufferLineCount int) []int { + height := view.InnerHeight() + originY := view.OriginY() + + rows := make([]int, bufferLineCount) + for i := range rows { + rows[i] = -1 + } + lastVisible := -1 + for y := originY; y < min(originY+height, view.ViewLinesHeight()); y++ { + bufferLine, ok := view.BufferLineForViewLine(y) + if !ok || bufferLine >= bufferLineCount { + continue + } + if rows[bufferLine] == -1 { + rows[bufferLine] = y - originY + } + lastVisible = bufferLine + } + for i := lastVisible + 1; i < bufferLineCount; i++ { + rows[i] = height + } + return rows +} + +// nearestSurvivingCandidate returns the first of the candidates that the given +// rendering still shows, and the line of it that does. The rendering is indexed +// first, rather than searched once per candidate: the candidate list is as long as +// the diff, and so is the rendering. +func (self *DiffLineHelper) nearestSurvivingCandidate( + contents []gocui.DiffLineContent, candidates []diffLineAnchor, +) (diffLineAnchor, int) { + lines := map[patchLine]int{} + for i, identities := range self.resolveDiffLineIdentities(contents) { + for _, identity := range identities { + if _, seen := lines[patchLineOf(identity)]; !seen { + lines[patchLineOf(identity)] = i + } + } + } + + for _, candidate := range candidates { + if line, ok := lines[patchLineOf(candidate.identity)]; ok { + return candidate, line + } + } + return diffLineAnchor{}, -1 +} + +// rowShowsDiffLine reports whether the given row of a rendering shows the given diff +// line — among any others it shows, since a side-by-side rendering puts a deletion +// beside the addition replacing it. It only knows what the renderer states about the +// row, since the alternative, parsing the rendering as a diff, needs whole hunks and +// this is asked of content that is still loading. It takes the repo's worktree path +// rather than reading it, being asked off the UI thread. +func rowShowsDiffLine(row gocui.DiffLineContent, worktreePath string, target types.DiffLineInfo) bool { + return lo.SomeBy(row.Metadata, func(record string) bool { + parsed, ok := parseDiffLineMetadata(record) + return ok && patchLineOf(diffLineInfoIn(worktreePath, parsed)) == patchLineOf(target) + }) +} + +// patchLine records what stays the same about a diff line when the same diff is +// rendered again differently: which file it belongs to, the line number that +// identifies it on the side it belongs to, and what kind of line it is. +type patchLine struct { + path string + // Every kind of content line collapses into DiffLineContext, since an addition + // and the context line it turns into when whitespace stops counting are the same + // line of the same file. The header rows keep their kind: a file's header and the + // first line of the file it heads are not the same place. + kind types.DiffLineType + // The old file's line number for a deletion, since two consecutive deletions + // share a new-file position and differ only here; the new file's otherwise. + line int + isDeletion bool +} + +func patchLineOf(info types.DiffLineInfo) patchLine { + switch info.Type { + case types.DiffLineFileHeader, types.DiffLineHunkHeader: + return patchLine{path: info.Path, kind: info.Type, line: info.NewLine} + case types.DiffLineDeleted: + return patchLine{path: info.Path, kind: types.DiffLineContext, line: info.OldLine, isDeletion: true} + default: + return patchLine{path: info.Path, kind: types.DiffLineContext, line: info.NewLine} + } +} diff --git a/pkg/integration/tests/main_view/keep_position_in_both_panes_when_changing_context_size.go b/pkg/integration/tests/main_view/keep_position_in_both_panes_when_changing_context_size.go new file mode 100644 index 000000000..ec686ede7 --- /dev/null +++ b/pkg/integration/tests/main_view/keep_position_in_both_panes_when_changing_context_size.go @@ -0,0 +1,71 @@ +package main_view + +import ( + "fmt" + "strings" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var KeepPositionInBothPanesWhenChangingContextSize = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Changing the diff's context size keeps the place in the lower pane too, not only in the upper one", + ExtraCmdArgs: []string{}, + Skip: false, + Width: 120, + Height: 30, + SetupConfig: func(cfg *config.AppConfig) { + cfg.GetUserConfig().Gui.UseHunkModeInStagingView = false + }, + SetupRepo: func(shell *Shell) { + lines := make([]string, 40) + for i := range lines { + lines[i] = fmt.Sprintf("line%02d", i+1) + } + shell.CreateFileAndAdd("file1", strings.Join(lines, "\n")+"\n") + shell.Commit("one") + + // Four staged changes, far enough apart that they stay four hunks as the + // context size grows, and one unstaged one to split the file's diff across + // both panes. + for _, i := range []int{5, 15, 25, 35} { + lines[i-1] = strings.ToUpper(lines[i-1]) + } + shell.UpdateFile("file1", strings.Join(lines, "\n")+"\n") + shell.GitAddAll() + + lines[39] = strings.ToUpper(lines[39]) + shell.UpdateFile("file1", strings.Join(lines, "\n")+"\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Press(keys.Universal.FocusMainView) + + // The lower pane holds the staged changes; getting to the last of them scrolls + // it, so there is a position to lose. + t.Views().Main(). + IsFocused(). + PressTab() + + t.Views().Secondary(). + IsFocused(). + Press(keys.Main.NextHunk). + Press(keys.Main.NextHunk). + Press(keys.Main.NextHunk). + SelectedLines( + Contains("-line35"), + ). + SelectedLineIdx(35). + OriginY(14). + Press(keys.Universal.IncreaseContextInDiffView). + Tap(func() { + t.ExpectToast(Equals("Changed diff context size to 4")) + }). + SelectedLines( + Contains("-line35"), + ). + SelectedLineIdx(42). + OriginY(21) + }, +}) diff --git a/pkg/integration/tests/main_view/keep_position_when_changing_context_size.go b/pkg/integration/tests/main_view/keep_position_when_changing_context_size.go new file mode 100644 index 000000000..ba99eb072 --- /dev/null +++ b/pkg/integration/tests/main_view/keep_position_when_changing_context_size.go @@ -0,0 +1,97 @@ +package main_view + +import ( + "fmt" + "strings" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var KeepPositionWhenChangingContextSize = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Changing the diff's context size keeps the line you were looking at where it was", + ExtraCmdArgs: []string{}, + Skip: false, + Width: 120, + Height: 30, + SetupConfig: func(cfg *config.AppConfig) { + cfg.GetUserConfig().Gui.UseHunkModeInStagingView = false + }, + SetupRepo: func(shell *Shell) { + lines := make([]string, 40) + for i := range lines { + lines[i] = fmt.Sprintf("line%02d", i+1) + } + shell.CreateFileAndAdd("file1", strings.Join(lines, "\n")+"\n") + shell.Commit("one") + + // Four changes, far enough apart that they stay four hunks as the context + // size grows. + for _, i := range []int{5, 15, 25, 35} { + lines[i-1] = strings.ToUpper(lines[i-1]) + } + shell.UpdateFile("file1", strings.Join(lines, "\n")+"\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + Press(keys.Main.NextHunk). + Press(keys.Main.NextHunk). + Press(keys.Main.NextHunk). + SelectedLines( + Contains("-line35"), + ). + // The diff is longer than the view, so getting to the last hunk scrolled + // it: the selected line sits 21 rows down the screen. + SelectedLineIdx(35). + OriginY(14). + Press(keys.Universal.IncreaseContextInDiffView). + Tap(func() { + t.ExpectToast(Equals("Changed diff context size to 4")) + }). + // A context line more on either side of each of the four hunks pushes the + // selected line seven lines further into the diff. The view follows it, so + // it is still the same line on the same screen row (42 - 21 = 21). + SelectedLines( + Contains("-line35"), + ). + SelectedLineIdx(42). + OriginY(21). + Press(keys.Universal.DecreaseContextInDiffView). + Tap(func() { + t.ExpectToast(Equals("Changed diff context size to 3")) + }). + Press(keys.Universal.DecreaseContextInDiffView). + Tap(func() { + t.ExpectToast(Equals("Changed diff context size to 2")) + }). + // And the same the other way (28 - 21 = 7). + SelectedLines( + Contains("-line35"), + ). + SelectedLineIdx(28). + OriginY(7). + // Leaving the view gives up the selection but not the scroll position, and + // with no selection to keep, it is the middle visible line that stays put. + PressEscape() + + t.Views().Files(). + IsFocused(). + Press(keys.Universal.IncreaseContextInDiffView). + Tap(func() { + t.ExpectToast(Equals("Changed diff context size to 3")) + }) + + // The middle visible line here is a hunk's header, and a context-size change + // rewrites those — they name the lines the hunk covers. So the restore falls + // back to the nearest line that does survive, the context line just below it, + // and puts that back on the row it was on. + t.Views().Main(). + SelectionIsHidden(). + OriginY(12) + }, +}) diff --git a/pkg/integration/tests/main_view/keep_position_when_the_selection_is_off_screen.go b/pkg/integration/tests/main_view/keep_position_when_the_selection_is_off_screen.go new file mode 100644 index 000000000..d195cb7bb --- /dev/null +++ b/pkg/integration/tests/main_view/keep_position_when_the_selection_is_off_screen.go @@ -0,0 +1,65 @@ +package main_view + +import ( + "fmt" + "strings" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var KeepPositionWhenTheSelectionIsOffScreen = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "A re-render keeps the lines that are on screen where they are, not a selection scrolled away from", + ExtraCmdArgs: []string{}, + Skip: false, + Width: 120, + Height: 30, + SetupConfig: func(cfg *config.AppConfig) { + cfg.GetUserConfig().Gui.UseHunkModeInStagingView = false + // Half a diff per scroll, to leave the selection well behind in two presses. + cfg.GetUserConfig().Gui.ScrollHeight = 15 + }, + SetupRepo: func(shell *Shell) { + lines := make([]string, 60) + 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{5, 15, 25, 35, 45, 55} { + lines[i-1] = strings.ToUpper(lines[i-1]) + } + shell.UpdateFile("file1", strings.Join(lines, "\n")+"\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("-line05"), + ). + // Reading on past the selection leaves it far behind, off the top of the + // view. + Press(keys.Universal.ScrollDownMain). + Press(keys.Universal.ScrollDownMain). + OriginY(30). + Press(keys.Universal.DecreaseContextInDiffView). + Tap(func() { + t.ExpectToast(Equals("Changed diff context size to 2")) + }). + // A context line less on either side of the four hunks above what is on + // screen pulls it nine lines up the diff, and the view follows it there: the + // lines the user was reading are still on the rows they were on. + OriginY(21). + // The selection is where it always was, on its own line of the diff, rather + // than having been dragged back into view. + SelectedLines( + Contains("-line05"), + ). + SelectedLineIdx(7) + }, +}) diff --git a/pkg/integration/tests/main_view/keep_scroll_when_the_diff_cant_be_read.go b/pkg/integration/tests/main_view/keep_scroll_when_the_diff_cant_be_read.go new file mode 100644 index 000000000..9cda012c9 --- /dev/null +++ b/pkg/integration/tests/main_view/keep_scroll_when_the_diff_cant_be_read.go @@ -0,0 +1,58 @@ +package main_view + +import ( + "fmt" + "strings" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var KeepScrollWhenTheDiffCantBeRead = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Changing the context size under a diff renderer whose rows can't be placed keeps the scroll position rather than jumping to the top", + ExtraCmdArgs: []string{}, + Skip: false, + Width: 120, + Height: 30, + SetupConfig: func(cfg *config.AppConfig) { + cfg.GetUserConfig().Gui.UseHunkModeInStagingView = false + // A renderer that says nothing about which line of which file each row shows, + // and mangles the diff enough that it can't be read back as one either: no line + // of it can be looked for in the re-render. + cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{ + {Name: "opaque", Command: `sed -e 's/^/| /'`}, + } + }, + SetupRepo: func(shell *Shell) { + lines := make([]string, 40) + 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{5, 15, 25, 35} { + lines[i-1] = strings.ToUpper(lines[i-1]) + } + shell.UpdateFile("file1", strings.Join(lines, "\n")+"\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Press(keys.Universal.ScrollDownMain). + Press(keys.Universal.ScrollDownMain). + Press(keys.Universal.ScrollDownMain) + + t.Views().Main(). + Content(Contains("| +LINE05")). + OriginY(6). + Tap(func() { + t.Views().Files().Press(keys.Universal.IncreaseContextInDiffView) + t.ExpectToast(Equals("Changed diff context size to 4")) + }). + // The re-render is a different command, and nothing in its output can be + // matched up with what was on screen, so the offset is all there is to keep — + // and it is a good deal closer than the top. + OriginY(6) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index e4df6b23f..ab8978354 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -371,6 +371,10 @@ var tests = []*components.IntegrationTest{ main_view.EscapeDismissesSelection, main_view.FileNavigationScrollsToTheTop, main_view.HideSelectionWhenChangesVanish, + main_view.KeepPositionInBothPanesWhenChangingContextSize, + main_view.KeepPositionWhenChangingContextSize, + main_view.KeepPositionWhenTheSelectionIsOffScreen, + main_view.KeepScrollWhenTheDiffCantBeRead, main_view.KeepSelectionVisibleWhenDiffShrinks, main_view.NavigateByHunkAndFile, main_view.NoSelectionOverABinaryDiff,