diff --git a/pkg/commands/patch/hunk.go b/pkg/commands/patch/hunk.go index 568b312a7..e539f3a47 100644 --- a/pkg/commands/patch/hunk.go +++ b/pkg/commands/patch/hunk.go @@ -16,6 +16,11 @@ type Hunk struct { newStart int // the context at the end of the header line (' func (f *CommitFile) Description() string {' in the above example) headerContext string + // the lengths declared in the header line ('2' and '3' in the above example), + // kept so that we can check the parsed body against them (see + // Patch.IsWellFormed). Only set by Parse. + declaredOldLength int + declaredNewLength int // the body of the hunk, excluding the header line bodyLines []*PatchLine } diff --git a/pkg/commands/patch/parse.go b/pkg/commands/patch/parse.go index fee7d2918..a26332869 100644 --- a/pkg/commands/patch/parse.go +++ b/pkg/commands/patch/parse.go @@ -7,7 +7,9 @@ import ( "github.com/jesseduffield/lazygit/pkg/utils" ) -var hunkHeaderRegexp = regexp.MustCompile(`(?m)^@@ -(\d+)[^\+]+\+(\d+)[^@]+@@(.*)$`) +// Captures, in order: the old start, the old length (omitted by git when it is +// 1), the new start, the new length (likewise), and the trailing context. +var hunkHeaderRegexp = regexp.MustCompile(`(?m)^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*)$`) func Parse(patchStr string) *Patch { // ignore trailing newline. @@ -19,13 +21,15 @@ func Parse(patchStr string) *Patch { var currentHunk *Hunk for _, line := range lines { if strings.HasPrefix(line, "@@") { - oldStart, newStart, headerContext := headerInfo(line) + oldStart, oldLength, newStart, newLength, headerContext := headerInfo(line) currentHunk = &Hunk{ - oldStart: oldStart, - newStart: newStart, - headerContext: headerContext, - bodyLines: []*PatchLine{}, + oldStart: oldStart, + newStart: newStart, + declaredOldLength: oldLength, + declaredNewLength: newLength, + headerContext: headerContext, + bodyLines: []*PatchLine{}, } hunks = append(hunks, currentHunk) } else if currentHunk != nil { @@ -41,14 +45,25 @@ func Parse(patchStr string) *Patch { } } -func headerInfo(header string) (int, int, string) { +func headerInfo(header string) (oldStart int, oldLength int, newStart int, newLength int, headerContext string) { match := hunkHeaderRegexp.FindStringSubmatch(header) - oldStart := utils.MustConvertToInt(match[1]) - newStart := utils.MustConvertToInt(match[2]) - headerContext := match[3] + oldStart = utils.MustConvertToInt(match[1]) + oldLength = declaredLength(match[2]) + newStart = utils.MustConvertToInt(match[3]) + newLength = declaredLength(match[4]) + headerContext = match[5] - return oldStart, newStart, headerContext + return oldStart, oldLength, newStart, newLength, headerContext +} + +// declaredLength parses a length capture of a hunk header, which git omits when +// it is 1 (e.g. "@@ -0,0 +1 @@"). +func declaredLength(match string) int { + if match == "" { + return 1 + } + return utils.MustConvertToInt(match) } func newHunkLine(line string) *PatchLine { diff --git a/pkg/commands/patch/patch.go b/pkg/commands/patch/patch.go index fbbf3c935..32c4788fb 100644 --- a/pkg/commands/patch/patch.go +++ b/pkg/commands/patch/patch.go @@ -79,6 +79,44 @@ func (self *Patch) HunkEndIdx(hunkIndex int) int { return self.HunkStartIdx(hunkIndex) + self.hunks[hunkIndex].lineCount() - 1 } +// IsWellFormed reports whether every hunk's body matches the lengths declared in +// its header. A faithful unified diff always satisfies this; a rendering that +// restructured the diff body does not — a diff renderer that puts line numbers in +// a gutter, say, shifts the +/- marker off the start of each line, so every body +// line reads as context and the computed lengths no longer match the header. That +// makes this the test for whether a rendered diff can be parsed as a unified diff +// at all, rather than trusting a mis-parse. Only meaningful for patches produced +// by Parse, which is where the declared lengths come from. +func (self *Patch) IsWellFormed() bool { + return self.isWellFormed(false) +} + +// IsWellFormedSoFar is IsWellFormed for a patch parsed from a diff we have only the +// beginning of. Its last hunk holds the first lines of a body that hasn't all arrived, +// so every hunk but the last has to match its header exactly, as before, while the last +// one only has to fit within what its header declares. +// +// The check exists to tell a faithful rendering from a restructured one, and it still +// does that. A rendering that moves the +/- marker off the start of the line makes us +// read a change as context, and a context line counts towards both lengths, so such a +// hunk comes out longer than its header declares rather than shorter. +func (self *Patch) IsWellFormedSoFar() bool { + return self.isWellFormed(true) +} + +func (self *Patch) isWellFormed(lastHunkMayBeIncomplete bool) bool { + for i, hunk := range self.hunks { + if lastHunkMayBeIncomplete && i == len(self.hunks)-1 { + return hunk.oldLength() <= hunk.declaredOldLength && + hunk.newLength() <= hunk.declaredNewLength + } + if hunk.oldLength() != hunk.declaredOldLength || hunk.newLength() != hunk.declaredNewLength { + return false + } + } + return true +} + func (self *Patch) ContainsChanges() bool { return lo.SomeBy(self.hunks, func(hunk *Hunk) bool { return hunk.containsChanges() @@ -114,6 +152,37 @@ func (self *Patch) LineNumberOfLine(idx int) int { return hunk.newStart + offset } +// Takes a line index in the patch and returns the line number in the old file. +// This is the old-file counterpart of LineNumberOfLine; for a deletion it gives +// the line's position in the old file (additions get the position they sit at). +// If the line is a header line, returns 1. +// If the line is a hunk header line, returns the first old-file line number in that hunk. +// If the line is out of range below, returns the last old-file line number in the last hunk. +func (self *Patch) OldLineNumberOfLine(idx int) int { + if idx < len(self.header) || len(self.hunks) == 0 { + return 1 + } + + hunkIdx := self.HunkContainingLine(idx) + // cursor out of range, return last file line number + if hunkIdx == -1 { + lastHunk := self.hunks[len(self.hunks)-1] + return lastHunk.oldStart + lastHunk.oldLength() - 1 + } + + hunk := self.hunks[hunkIdx] + hunkStartIdx := self.HunkStartIdx(hunkIdx) + idxInHunk := idx - hunkStartIdx + + if idxInHunk == 0 { + return hunk.oldStart + } + + lines := hunk.bodyLines[:idxInHunk-1] + offset := nLinesWithKind(lines, []PatchLineKind{DELETION, CONTEXT}) + return hunk.oldStart + offset +} + // Returns hunk index containing the line at the given patch line index func (self *Patch) HunkContainingLine(idx int) int { for hunkIdx, hunk := range self.hunks { diff --git a/pkg/commands/patch/patch_test.go b/pkg/commands/patch/patch_test.go index 4f84041d6..1d925d399 100644 --- a/pkg/commands/patch/patch_test.go +++ b/pkg/commands/patch/patch_test.go @@ -120,6 +120,20 @@ index 9320895..6d79956 100644 lemon ` +// Two deletions with no line between them: they share a new-file line number +// (both sit at the same new-file position), so only their old-file line numbers +// tell them apart. +const consecutiveDeletions = `diff --git a/filename b/filename +index 9320895..6d79956 100644 +--- a/filename ++++ b/filename +@@ -1,4 +1,2 @@ + apple +-grape +-pear + lemon +` + const newFile = `diff --git a/newfile b/newfile new file mode 100644 index 0000000..4e680cc @@ -682,6 +696,148 @@ func TestLineNumberOfLine(t *testing.T) { } } +func TestIsWellFormed(t *testing.T) { + // The body of a diff as rendered with the +/- markers moved out of the text + // and into a gutter: every body line now reads as context, so the lengths no + // longer match the header. + const gutterMangled = `diff --git a/filename b/filename +index 9320895..6d79956 100644 +--- a/filename ++++ b/filename +@@ -1,4 +1,2 @@ + apple + grape + pear + lemon +` + + scenarios := []struct { + testName string + patchStr string + expected bool + }{ + {"simpleDiff", simpleDiff, true}, + {"renameWithModificationDiff", renameWithModificationDiff, true}, + {"addNewlineToEndOfFile", addNewlineToEndOfFile, true}, + {"twoHunks", twoHunks, true}, + {"consecutiveDeletions", consecutiveDeletions, true}, + {"newFile", newFile, true}, + {"deletedFile", deletedFile, true}, + {"addNewlineToPreviouslyEmptyFile", addNewlineToPreviouslyEmptyFile, true}, + {"exampleHunk", exampleHunk, true}, + {"gutterMangled", gutterMangled, false}, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + assert.Equal(t, s.expected, Parse(s.patchStr).IsWellFormed()) + }) + } +} + +func TestIsWellFormedSoFar(t *testing.T) { + // A diff read only as far as the middle of its second hunk. + const cutShort = `diff --git a/filename b/filename +index e48a11c..b2ab81b 100644 +--- a/filename ++++ b/filename +@@ -1,5 +1,5 @@ + apple +-grape ++orange + ... + ... + ... +@@ -8,6 +8,8 @@ grape + ... + ... +` + + // The same diff cut short in its first hunk, so that the second is missing + // entirely rather than short. + const cutShortInTheFirstHunk = `diff --git a/filename b/filename +index e48a11c..b2ab81b 100644 +--- a/filename ++++ b/filename +@@ -1,5 +1,5 @@ + apple +-grape +` + + // A rendering with the +/- markers moved into a gutter, cut short: reading the + // changes as context makes the hunk longer than its header declares, not shorter, + // so it doesn't pass for a diff we only have the beginning of. + const gutterMangledAndCutShort = `diff --git a/filename b/filename +index 9320895..6d79956 100644 +--- a/filename ++++ b/filename +@@ -1,4 +1,2 @@ + apple + grape + pear + lemon + melon +` + + scenarios := []struct { + testName string + patchStr string + expected bool + }{ + {"simpleDiff", simpleDiff, true}, + {"twoHunks", twoHunks, true}, + {"cutShort", cutShort, true}, + {"cutShortInTheFirstHunk", cutShortInTheFirstHunk, true}, + {"gutterMangledAndCutShort", gutterMangledAndCutShort, false}, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + assert.Equal(t, s.expected, Parse(s.patchStr).IsWellFormedSoFar()) + }) + } +} + +func TestOldLineNumberOfLine(t *testing.T) { + type scenario struct { + testName string + patchStr string + indexes []int + expecteds []int + } + + scenarios := []scenario{ + { + testName: "twoChangesInOneHunk", + patchStr: twoChangesInOneHunk, + indexes: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 1000}, + expecteds: []int{1, 1, 1, 1, 1, 1, 2, 3, 3, 4, 5, 5, 5}, + }, + { + testName: "consecutiveDeletions", + patchStr: consecutiveDeletions, + indexes: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 1000}, + expecteds: []int{1, 1, 1, 1, 1, 1, 2, 3, 4, 4}, + }, + { + testName: "renameWithModificationDiff", + patchStr: renameWithModificationDiff, + indexes: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 1000}, + expecteds: []int{1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 3, 3, 4, 5, 5}, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + for i, idx := range s.indexes { + patch := Parse(s.patchStr) + result := patch.OldLineNumberOfLine(idx) + assert.Equal(t, s.expecteds[i], result) + } + }) + } +} + func TestGetNextStageableLineIndex(t *testing.T) { type scenario struct { testName string diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 12968fa95..5af5a7cca 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -438,7 +438,7 @@ func (g *Gui) SetView(name string, x0, y0, x1, y1 int, overlaps byte) (*View, er v.y1 = y1 if sizeChanged { - v.ClearViewLines() + v.RewrapContent() if v.Editable { cursorX, cursorY := v.TextArea.GetCursorXY() @@ -1582,7 +1582,7 @@ func (g *Gui) flush() error { // if GUI's size has changed, we need to redraw all views if maxX != g.maxX || maxY != g.maxY { for _, v := range g.views { - v.ClearViewLines() + v.RewrapContent() } } g.maxX, g.maxY = maxX, maxY diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 6fbbecdb6..2961447f6 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -5,6 +5,7 @@ package gocui import ( + "bytes" "fmt" "io" "slices" @@ -251,23 +252,138 @@ type pos struct { x, y int } -// call this in the event of a view resize, or if you want to render new content -// without the chance of old content still appearing, or if you want to remove -// a line from the existing content +// call this if you want to render new content without the chance of old content +// still appearing, or if you want to remove a line from the existing content. For +// a view whose size has changed, whose content is the same but has to be wrapped +// afresh, call RewrapContent instead. func (v *View) clearViewLines() { v.tainted = true v.viewLines = nil v.clearHover() } -// ClearViewLines is clearViewLines guarded by writeMutex. It's for callers on -// the UI thread (the layout pass) that touch a view whose content a task -// goroutine may be writing concurrently: viewLines/tainted/hover are all -// buffer state that writeMutex protects. -func (v *View) ClearViewLines() { +// RewrapContent wraps the view's content for the size the view has now, and puts +// the positions into that content — the scroll offset, the cursor, a range's +// anchor — back on the lines they were on. They are all view lines, which count +// the segments each line is wrapped into, so wrapping the content at another +// width leaves every one of them pointing at a different line. +// +// Call it on the UI thread whenever the view's size changes; a task goroutine may +// be writing the content concurrently, and all of this is state writeMutex +// protects. +func (v *View) RewrapContent() { v.writeMutex.Lock() defer v.writeMutex.Unlock() + + v.refreshViewLinesIfNeeded() + origin := v.contentPosOf(v.oy) + cursor := v.contentPosOf(v.oy + v.cy) + anchor := v.contentPosOf(v.rangeSelectStartY) + cursorRow := v.cy + v.clearViewLines() + v.refreshViewLinesIfNeeded() + + if !origin.ok { + return + } + + cursorLine, cursorOk := v.viewLineOf(cursor) + if anchorLine, ok := v.viewLineOf(anchor); ok { + v.rangeSelectStartY = anchorLine + if cursorOk { + // A range covers lines of content, not the wrapped segments those + // lines are drawn as, so its ends go back on the outermost segments + // of their lines: a line that was covered whole stays covered whole. + cursorLine = v.viewLineOfRangeEnd(cursor, anchor) + v.rangeSelectStartY = v.viewLineOfRangeEnd(anchor, cursor) + } + } + + // The line the cursor is on keeps the row it was drawn on, so that it doesn't + // move under the user; with no cursor on screen the view keeps its own place + // in the content instead. + if v.Highlight && cursorOk && cursorRow >= 0 && cursorRow < v.InnerHeight() { + v.SetOriginY(cursorLine - cursorRow) + } else if originLine, ok := v.viewLineOf(origin); ok { + v.SetOriginY(originLine) + } + if cursorOk { + v.cy = cursorLine - v.oy + } +} + +// contentPos is a position in a view's content in terms that survive the content +// being wrapped again: which line of it, and which of that line's segments. +type contentPos struct { + line, segment int + ok bool +} + +// contentPosOf returns where the given view line sits in the content. Only call +// this with a lock on writeMutex, and with the view lines up to date. +func (v *View) contentPosOf(viewLine int) contentPos { + if viewLine < 0 || viewLine >= len(v.viewLines) { + return contentPos{} + } + return contentPos{ + line: v.viewLines[viewLine].linesY, + segment: v.viewLines[viewLine].linesX, + ok: true, + } +} + +// viewLineOf returns the view line drawing the given position in the content, +// on the nearest segment its line still has. Only call this with a lock on +// writeMutex, and with the view lines up to date. +func (v *View) viewLineOf(pos contentPos) (int, bool) { + first, last, ok := v.segmentSpanOf(pos) + if !ok { + return 0, false + } + return min(first+pos.segment, last), true +} + +// viewLineOfRangeEnd returns the view line for one end of a range selection: the +// outermost segment of its line, so that the range covers that line whole. other +// is the range's other end, which says which way is outward. Both ends have to be +// positions whose lines are drawn, which viewLineOf answers. +func (v *View) viewLineOfRangeEnd(pos contentPos, other contentPos) int { + first, last, _ := v.segmentSpanOf(pos) + if pos.line <= other.line { + return first + } + return last +} + +// segmentSpanOf returns the first and last view line drawing the given position's +// line of the content. ok is false when the position was never taken, or its line +// isn't drawn at all. +func (v *View) segmentSpanOf(pos contentPos) (int, int, bool) { + if !pos.ok { + return 0, 0, false + } + return v.viewLineSpanOfBufferLine(pos.line) +} + +// viewLineSpanOfBufferLine returns the first and last view line drawing the given +// buffer line, i.e. the first and last segment it is wrapped into. Both are the +// same view line when the line doesn't wrap. ok is false when the line isn't drawn +// at all. Only call this with a lock on writeMutex, and with the view lines up to +// date. +func (v *View) viewLineSpanOfBufferLine(bufferLine int) (int, int, bool) { + first, last := -1, -1 + for i, vline := range v.viewLines { + if vline.linesY == bufferLine { + if first == -1 { + first = i + } + last = i + } else if first != -1 { + break + } + } + return first, last, first != -1 } type searcher struct { @@ -578,6 +694,23 @@ type lineType struct { // matches the current width; nil means nothing is cached yet. wrappedCells [][]cell wrappedColumns int + + // asWritten is the text of the line as its writer wrote it, escape sequences + // left out, kept from the first character on that the cells spell differently: + // a tab, which the cells hold as the spaces it fills, or a carriage return, + // which they hold as the overwrite it caused. nil while the cells spell the + // line as it was written, as they do for most lines. + asWritten []byte +} + +// textAsWritten returns the line's text as its writer wrote it, escape sequences +// left out. A reader that parses a view's content rather than showing it wants +// this form; the cells' text is for showing. +func (l *lineType) textAsWritten() string { + if l.asWritten != nil { + return string(l.asWritten) + } + return l.cells.String() } // trailingFillAttributes describes the fg/bg colors that draw() should @@ -835,8 +968,7 @@ func (v *View) SetWritePos(x, y int) { y = 0 } - v.buf.wx = x - v.buf.wy = y + v.buf.seekWrite(x, y) // Changing the write position makes a pending newline obsolete v.buf.pendingNewline = false @@ -920,6 +1052,35 @@ func (b *viewBuffer) writeCells(cells []cell) { b.wx += len(cells) } +// seekWrite moves the write cursor to (x, y). Writing there starts the line over, +// so whatever it kept of its text as written is dropped; the text a line keeps is +// the text written to it from its start. A carriage return continues a line +// instead, and moves the cursor without this (see write). +func (b *viewBuffer) seekWrite(x, y int) { + b.wx = x + b.wy = y + if y < len(b.lines) { + b.lines[y].asWritten = nil + } +} + +// startAsWritten begins keeping the current line's text as written (see +// lineType.asWritten), at the first character the cells won't spell the same way. +// Up to here they spell it exactly, so their text is what was written so far. +func (b *viewBuffer) startAsWritten() { + if line := &b.lines[b.wy]; line.asWritten == nil { + line.asWritten = append([]byte{}, line.cells.String()...) + } +} + +// noteAsWritten records text the writer wrote to the current line, once the line +// keeps its text as written at all. +func (b *viewBuffer) noteAsWritten(text []byte) { + if line := &b.lines[b.wy]; line.asWritten != nil { + line.asWritten = append(line.asWritten, text...) + } +} + // Write appends a byte slice into the view's internal buffer. Because // View implements the io.Writer interface, it can be passed as parameter // of functions like fmt.Fprintf, fmt.Fprintln, io.Copy, etc. Clear must @@ -966,8 +1127,7 @@ func (b *viewBuffer) write(v *View, p []byte) { } advanceToNextLine := func() { - b.wx = 0 - b.wy++ + b.seekWrite(0, b.wy+1) if b.wy >= len(b.lines) { b.lines = append(b.lines, lineType{}) } @@ -1000,6 +1160,10 @@ func (b *viewBuffer) write(v *View, p []byte) { b.ei.notifyRowAdvance() case characterEquals(chr, '\r'): finishLine() + // The cells will hold what follows as an overwrite of what came + // before; the text as written keeps the return itself. + b.startAsWritten() + b.noteAsWritten(chr) b.wx = 0 b.ei.notifyColumnReset() default: @@ -1116,7 +1280,8 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo isEscape, err := b.ei.parseOne(ch) if err != nil { - for _, chr := range b.ei.characters() { + characters := b.ei.characters() + for _, chr := range characters { c := cell{ fgColor: v.FgColor, bgColor: v.BgColor, @@ -1125,6 +1290,7 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo } cells = append(cells, c) } + b.noteAsWritten([]byte(strings.Join(characters, ""))) b.ei.reset() } else { repeatCount := 1 @@ -1150,10 +1316,15 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo repeatCount = cf.n ch = []byte{' '} width = 1 + b.noteAsWritten(bytes.Repeat(ch, repeatCount)) } else if isEscape { // do not output anything return truncateLine, nil } else if characterEquals(ch, '\t') { + // The cells hold a tab as the spaces it fills; the text as written + // keeps the tab itself. + b.startAsWritten() + b.noteAsWritten(ch) // fill tab-sized space tabWidth := v.TabWidth if tabWidth < 1 { @@ -1162,6 +1333,8 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo ch = []byte{' '} width = 1 repeatCount = tabWidth - (x % tabWidth) + } else { + b.noteAsWritten(ch) } c := cell{ fgColor: b.ei.curFgColor, @@ -1745,6 +1918,64 @@ func (v *View) BufferLines() []string { return lines } +// LinesAsWritten returns the lines of the view's internal buffer as their writer +// wrote them, escape sequences left out, where BufferLines returns them as the +// cells spell them. The two differ where the cells can't spell what was written: +// a tab, which they hold as the spaces it fills, and a carriage return, which +// they hold as the overwrite it caused. A reader that parses the content rather +// than showing it wants this form. git, for one, terminates a path containing a +// space with a tab in a diff header, and a parser of the diff has to see the tab. +func (v *View) LinesAsWritten() []string { + v.writeMutex.Lock() + defer v.writeMutex.Unlock() + + lines := make([]string, len(v.buf.lines)) + for i := range v.buf.lines { + lines[i] = v.buf.lines[i].textAsWritten() + } + return lines +} + +// BufferLineForViewLine maps a view line index (which counts wrapped lines) to +// the index of the corresponding line in the unwrapped internal buffer (as +// returned by BufferLines). Several view lines map to the same buffer line when +// that line wraps. Returns false if the view line is out of range. +func (v *View) BufferLineForViewLine(y int) (int, bool) { + v.writeMutex.Lock() + defer v.writeMutex.Unlock() + + return v.bufferLineForViewLine(y) +} + +// ViewLineForBufferLine maps an unwrapped buffer line index to the index of the +// first view line that renders it — the inverse of BufferLineForViewLine, for +// turning a line found by examining the buffer into a line to scroll to or +// select. Returns false if the buffer line isn't rendered into any view line. +func (v *View) ViewLineForBufferLine(bufferLineIdx int) (int, bool) { + v.writeMutex.Lock() + defer v.writeMutex.Unlock() + + v.refreshViewLinesIfNeeded() + + first, _, ok := v.viewLineSpanOfBufferLine(bufferLineIdx) + return first, ok +} + +// LastViewLineForBufferLine maps an unwrapped buffer line index to the index of +// the last view line that renders it, which for a line that doesn't wrap is the +// same as the first. It is where the far end of a range goes: a range is over +// buffer lines, so it has to cover the last one of them to its final segment +// rather than stopping where that line begins. +func (v *View) LastViewLineForBufferLine(bufferLineIdx int) (int, bool) { + v.writeMutex.Lock() + defer v.writeMutex.Unlock() + + v.refreshViewLinesIfNeeded() + + _, last, ok := v.viewLineSpanOfBufferLine(bufferLineIdx) + return last, ok +} + // Buffer returns a string with the contents of the view's internal // buffer. func (v *View) Buffer() string { @@ -1985,11 +2216,12 @@ func (v *View) SelectedLine() string { v.writeMutex.Lock() defer v.writeMutex.Unlock() - if len(v.buf.lines) == 0 { + idx, ok := v.bufferLineForViewLine(v.SelectedLineIdx()) + if !ok { return "" } - return v.lineContentAtIdx(v.SelectedLineIdx()) + return v.lineContentAtIdx(idx) } // expected to only be used in tests @@ -2004,8 +2236,17 @@ func (v *View) SelectedLines() []string { startIdx, endIdx := v.SelectedLineRange() lines := make([]string, 0, endIdx-startIdx+1) + previous := -1 for i := startIdx; i <= endIdx; i++ { - lines = append(lines, v.lineContentAtIdx(i)) + // The selection is in view lines, which count the segments a wrapped line + // is drawn as; a line the selection covers several segments of is still + // the one line it is. + idx, ok := v.bufferLineForViewLine(i) + if !ok || idx == previous { + continue + } + previous = idx + lines = append(lines, v.lineContentAtIdx(idx)) } return lines @@ -2015,6 +2256,19 @@ func (v *View) lineContentAtIdx(idx int) string { return v.buf.lines[idx].cells.String() } +// bufferLineForViewLine maps a view line index, which counts the wrapped +// segments of the lines it draws, to the index of the line of content it is a +// segment of. Only call this with a lock on writeMutex. +func (v *View) bufferLineForViewLine(y int) (int, bool) { + v.refreshViewLinesIfNeeded() + + if y < 0 || y >= len(v.viewLines) { + return 0, false + } + + return v.viewLines[y].linesY, true +} + func (v *View) SelectedPoint() (int, int) { cx, cy := v.Cursor() ox, oy := v.Origin() @@ -2085,8 +2339,7 @@ func (v *View) ClearTextArea() { func (v *View) overwriteLines(y int, content string) { // break by newline, then for each line, write it, then add that erase command - v.buf.wx = 0 - v.buf.wy = y + v.SetWritePos(0, y) v.clearViewLines() lines := strings.ReplaceAll(content, "\n", "\x1b[K\n") diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index 3ffd56e7c..57d18abde 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -113,6 +113,94 @@ func TestWriteString(t *testing.T) { } } +func TestOverwriteLinesAfterContentEndingInANewline(t *testing.T) { + v := NewView("name", 0, 0, 20, 10, OutputNormal) + // The trailing newline is held back until more content arrives, so that the + // view doesn't end in an empty line. + v.writeString("a\nb\n") + + v.OverwriteLines(0, "x") + + assert.Equal(t, []string{"x", "b"}, v.BufferLines()) +} + +func TestLinesAsWritten(t *testing.T) { + tests := []struct { + name string + stringsToWrite []string + expectedShown []string + expectedAsWritten []string + }{ + { + name: "a line the cells spell as written", + stringsToWrite: []string{"abc\n"}, + expectedShown: []string{"abc"}, + expectedAsWritten: []string{"abc"}, + }, + { + name: "a tab is kept rather than the spaces it fills", + stringsToWrite: []string{"a\tb\n"}, + expectedShown: []string{"a b"}, + expectedAsWritten: []string{"a\tb"}, + }, + { + name: "a carriage return is kept rather than the overwrite it causes", + stringsToWrite: []string{"abc\rde\n"}, + expectedShown: []string{"dec"}, + expectedAsWritten: []string{"abc\rde"}, + }, + { + // git writes a CRLF file's lines as "+foo\r", the color reset, "\n". + name: "escape sequences are left out", + stringsToWrite: []string{"\x1b[32m+foo\r\x1b[m\n"}, + expectedShown: []string{"+foo"}, + expectedAsWritten: []string{"+foo\r"}, + }, + { + // ConPTY writes a run of spaces as a cursor-forward escape. + name: "a cursor-forward escape stands for the spaces it skips", + stringsToWrite: []string{"\ta\x1b[2Cb\n"}, + expectedShown: []string{" a b"}, + expectedAsWritten: []string{"\ta b"}, + }, + { + name: "a line written in two parts", + stringsToWrite: []string{"a\t", "b\n"}, + expectedShown: []string{"a b"}, + expectedAsWritten: []string{"a\tb"}, + }, + { + name: "only the lines with a tab or a return are kept separately", + stringsToWrite: []string{"x\n", "y\tz\n", "w\n"}, + expectedShown: []string{"x", "y z", "w"}, + expectedAsWritten: []string{"x", "y\tz", "w"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + v := NewView("name", 0, 0, 20, 10, OutputNormal) + for _, s := range test.stringsToWrite { + v.writeString(s) + } + assert.Equal(t, test.expectedShown, v.BufferLines()) + assert.Equal(t, test.expectedAsWritten, v.LinesAsWritten()) + }) + } +} + +func TestLinesAsWrittenOfAnOverwrittenLine(t *testing.T) { + v := NewView("name", 0, 0, 20, 10, OutputNormal) + v.writeString("a\tb\nc\td") + + // Overwriting a line starts it over: what it kept of its earlier text goes, + // and the line below is left alone. + v.OverwriteLines(0, "xy") + + assert.Equal(t, []string{"xy", "c d"}, v.BufferLines()) + assert.Equal(t, []string{"xy", "c\td"}, v.LinesAsWritten()) +} + func TestUpdatedCursorAndOrigin(t *testing.T) { tests := []struct { prevOrigin int @@ -204,6 +292,61 @@ func TestViewLinesTruncatedByShorterRender(t *testing.T) { assert.Equal(t, []string{"aaa", "bbb", "ccc"}, v.ViewBufferLines()) } +func TestBufferLineForViewLine(t *testing.T) { + v := NewView("name", 0, 0, 10, 10, OutputNormal) // InnerWidth is 9 + v.Wrap = true + + // Buffer line 0 is short (view line 0); buffer line 1 wraps into three view + // lines (1, 2, 3); buffer line 2 is short again (view line 4). + v.writeString("short\n" + strings.Repeat("b", 27) + "\nlast") + + for viewLine, wantBufferLine := range []int{0, 1, 1, 1, 2} { + bufferLine, ok := v.BufferLineForViewLine(viewLine) + assert.True(t, ok) + assert.Equal(t, wantBufferLine, bufferLine) + } + + _, ok := v.BufferLineForViewLine(5) + assert.False(t, ok) + + _, ok = v.BufferLineForViewLine(-1) + assert.False(t, ok) +} + +func TestViewLineForBufferLine(t *testing.T) { + v := NewView("name", 0, 0, 10, 10, OutputNormal) // InnerWidth is 9 + v.Wrap = true + + // A wrapped buffer line maps to the first of the view lines it spans. + v.writeString("short\n" + strings.Repeat("b", 27) + "\nlast") + + for bufferLine, wantViewLine := range []int{0, 1, 4} { + viewLine, ok := v.ViewLineForBufferLine(bufferLine) + assert.True(t, ok) + assert.Equal(t, wantViewLine, viewLine) + } + + _, ok := v.ViewLineForBufferLine(3) + assert.False(t, ok) +} + +func TestLastViewLineForBufferLine(t *testing.T) { + v := NewView("name", 0, 0, 10, 10, OutputNormal) // InnerWidth is 9 + v.Wrap = true + + // A wrapped buffer line maps to the last of the view lines it spans. + v.writeString("short\n" + strings.Repeat("b", 27) + "\nlast") + + for bufferLine, wantViewLine := range []int{0, 3, 4} { + viewLine, ok := v.LastViewLineForBufferLine(bufferLine) + assert.True(t, ok) + assert.Equal(t, wantViewLine, viewLine) + } + + _, ok := v.LastViewLineForBufferLine(3) + assert.False(t, ok) +} + // While an async re-render loads, it swaps in only a partially-filled buffer at // its first paint and keeps appending lines afterwards. The scrollbar must keep // using the pre-load height until the load ends, so the thumb doesn't shrink and @@ -820,3 +963,53 @@ func TestApplySelTextColor(t *testing.T) { }) } } + +// A view that wraps draws one line of its content as several view lines, and the +// cursor and the range anchor count those. What is asked about a selection is +// which lines of the content it covers, so those are what it has to be reported +// in. +func TestSelectedLinesOfWrappedContent(t *testing.T) { + v := NewView("name", 0, 0, 11, 10, OutputNormal) // InnerWidth 10 + v.Wrap = true + v.Highlight = true + + // "a line that wraps" takes two view lines, so the four lines of content are + // drawn as five: "one", "two", "a line th", "at wraps", "four". + v.writeString("one\ntwo\na line that wraps\nfour\n") + assert.Equal(t, 5, v.ViewLinesHeight()) + + // The cursor on the wrapped line's second half is on that line. + v.FocusPoint(0, 3, false) + assert.Equal(t, "a line that wraps", v.SelectedLine()) + + // A range over both halves of the wrapped line covers one line of content. + v.SetRangeSelectStart(2) + assert.Equal(t, []string{"a line that wraps"}, v.SelectedLines()) +} + +// Resizing a view throws away the wrapping of its content and wraps it again for +// the new width, which moves every line of it to a different view line. The +// positions into the view count view lines, so they all have to come along. +func TestResizingAWrappingViewKeepsItsPlaceInTheContent(t *testing.T) { + g := &Gui{} + v, _ := g.SetView("name", 0, 0, 11, 10, 0) // InnerWidth 10 + v.Wrap = true + v.Highlight = true + + // Two wrapping lines, with a single line between them: eight view lines for + // five lines of content. + v.writeString("one\na line that wraps\ntwo\nanother wrapping line\nthree\n") + assert.Equal(t, 8, v.ViewLinesHeight()) + + // A range over the whole of the second wrapping line, which is drawn as view + // lines 4 to 6. + v.SetRangeSelectStart(4) + v.FocusPoint(0, 6, false) + assert.Equal(t, []string{"another wrapping line"}, v.SelectedLines()) + + // Widen the view so that nothing wraps any more. + _, _ = g.SetView("name", 0, 0, 31, 10, 0) // InnerWidth 30 + assert.Equal(t, 5, v.ViewLinesHeight()) + + assert.Equal(t, []string{"another wrapping line"}, v.SelectedLines()) +} diff --git a/pkg/gui/controllers.go b/pkg/gui/controllers.go index f21fb607f..dc5a77834 100644 --- a/pkg/gui/controllers.go +++ b/pkg/gui/controllers.go @@ -68,6 +68,7 @@ func (gui *Gui) resetHelpersAndControllers() { searchHelper, ) diffHelper := helpers.NewDiffHelper(helperCommon) + diffLineHelper := helpers.NewDiffLineHelper(helperCommon) cherryPickHelper := helpers.NewCherryPickHelper( helperCommon, rebaseHelper, @@ -110,6 +111,7 @@ func (gui *Gui) resetHelpersAndControllers() { SuspendResume: helpers.NewSuspendResumeHelper(helperCommon), Snake: helpers.NewSnakeHelper(helperCommon), Diff: diffHelper, + DiffLine: diffLineHelper, Repos: reposHelper, RecordDirectory: recordDirectoryHelper, Update: helpers.NewUpdateHelper(helperCommon, gui.Updater), diff --git a/pkg/gui/controllers/helpers/diff_line_helper.go b/pkg/gui/controllers/helpers/diff_line_helper.go new file mode 100644 index 000000000..de4a60960 --- /dev/null +++ b/pkg/gui/controllers/helpers/diff_line_helper.go @@ -0,0 +1,55 @@ +package helpers + +import ( + "path/filepath" + + "github.com/jesseduffield/lazygit/pkg/gocui" + "github.com/jesseduffield/lazygit/pkg/gui/types" +) + +type DiffLineHelper struct { + c *HelperCommon +} + +func NewDiffLineHelper(c *HelperCommon) *DiffLineHelper { + return &DiffLineHelper{c: c} +} + +// GetDiffLineInfo recovers the identity — file, kind, and old/new line number — +// of the diff row at the given (wrapped) view line of the given view. It is the +// seam every consumer of a diff row goes through, so that how we recover that +// identity can change without them noticing: today the only way is to parse the +// view's contents as a unified diff, which works for the renderings that keep a +// diff's structure (no renderer, `git diff --color`, a renderer that only +// colorizes) and fails for the ones that restructure it. +// +// ok is false when the row's identity can't be recovered, in which case the +// caller must not act on the line at all. +func (self *DiffLineHelper) GetDiffLineInfo(view *gocui.View, viewLineIdx int) (types.DiffLineInfo, bool) { + // The cursor and clicks land on a view line, which counts wrapped segments; + // the contents are indexed by unwrapped buffer line. + bufferLineIdx, ok := view.BufferLineForViewLine(viewLineIdx) + if !ok { + return types.DiffLineInfo{}, false + } + + // The lines as written, not as shown: git ends the path field of a diff header + // with a tab when the path contains a space, and the view shows a tab as spaces. + parsed, ok := parseDiffLineFromBuffer(view.LinesAsWritten(), bufferLineIdx) + if !ok { + return types.DiffLineInfo{}, false + } + + return self.diffLineInfoFromParsed(parsed), true +} + +// diffLineInfoFromParsed turns the parser's repo-relative result into the +// absolute-path identity consumers work with. +func (self *DiffLineHelper) diffLineInfoFromParsed(parsed parsedDiffLine) types.DiffLineInfo { + return types.DiffLineInfo{ + Path: filepath.Join(self.c.Git().RepoPaths.WorktreePath(), parsed.RelPath), + Type: parsed.Type, + NewLine: parsed.NewLine, + OldLine: parsed.OldLine, + } +} diff --git a/pkg/gui/controllers/helpers/diff_line_parser.go b/pkg/gui/controllers/helpers/diff_line_parser.go new file mode 100644 index 000000000..d58633f7e --- /dev/null +++ b/pkg/gui/controllers/helpers/diff_line_parser.go @@ -0,0 +1,318 @@ +package helpers + +import ( + "regexp" + "strconv" + "strings" + + "github.com/jesseduffield/lazygit/pkg/commands/patch" + "github.com/jesseduffield/lazygit/pkg/gui/types" +) + +// diffFilePrefix marks the start of a file's section in a (possibly multi-file) +// unified diff. +const diffFilePrefix = "diff --git " + +// submodulePrefix opens the line a submodule's section of a diff starts with, which +// stands in for the "diff --git" header a file of the repo gets. +const submodulePrefix = "Submodule " + +// submoduleSectionPattern matches that line and captures the submodule's path. git +// writes one of two kinds: the commit the submodule is checked out at has moved +// ("Submodule sub a32f27c..2d9f921:", with "..." in place of ".." where the move is +// no fast-forward, " (rewind)" where it goes backwards, and a message in brackets in +// place of the colon where the two commits can't both be read), or its working tree +// is dirty ("Submodule sub contains untracked content"). +// +// The path is captured greedily: git writes it unquoted, so a path that itself ends +// in something reading like a range of commits is told apart by taking the last such +// range on the line. +var submoduleSectionPattern = regexp.MustCompile( + `^Submodule (.+) (?:contains (?:untracked|modified) content|[0-9a-f]+\.{2,3}[0-9a-f]+(?: \(.*\))?:?)$`) + +// parsedDiffLine is what the parser recovers about a row of a rendered diff. +// RelPath is the path as the diff header spells it, i.e. relative to the repo +// root; the caller turns it into the absolute path of types.DiffLineInfo. +type parsedDiffLine struct { + RelPath string + Type types.DiffLineType + NewLine int + OldLine int +} + +// bufferLineParse is the parser's result for one buffer line: the recovered +// identity, and whether the line could be resolved at all (false for a line in +// an unparseable section, or outside any file section). +type bufferLineParse struct { + parsed parsedDiffLine + ok bool +} + +// parseDiffLineFromBuffer recovers the identity of a row of a rendered diff by +// parsing the view's decolorized contents. +// +// bufferLines is the full unwrapped view buffer; targetIdx is the buffer line to +// resolve. A commit's diff spans several files, so we isolate the file section +// containing targetIdx and parse just that one (see parseFileSection). Use this +// for a single line, e.g. the one under the cursor; to resolve every line of a +// buffer, use parseAllDiffLinesFromBuffer, which parses each section only once. +// +// ok is false when the buffer isn't a parseable unified diff at targetIdx, +// because the diff renderer restructured it, so that the caller can fall back. +func parseDiffLineFromBuffer(bufferLines []string, targetIdx int) (parsedDiffLine, bool) { + if targetIdx < 0 || targetIdx >= len(bufferLines) { + return parsedDiffLine{}, false + } + start, end := fileSectionBounds(bufferLines, targetIdx) + if start == -1 { + return parsedDiffLine{}, false + } + r := parseFileSection(bufferLines[start:end], end == len(bufferLines))[targetIdx-start] + return r.parsed, r.ok +} + +// parseAllDiffLinesFromBuffer resolves every line of a (possibly multi-file) +// diff buffer in one pass, parsing each file section exactly once. It is the +// batch form of parseDiffLineFromBuffer, for callers that scan a whole buffer: +// resolving line by line would re-parse a section once per line of it — O(n²) on +// a large single-file diff — whereas this is O(n). The result is indexed 1:1 +// with bufferLines; a line in an unparseable section, or above the first one, is +// left ok=false. +func parseAllDiffLinesFromBuffer(bufferLines []string) []bufferLineParse { + result := make([]bufferLineParse, len(bufferLines)) + for i := 0; i < len(bufferLines); { + if !startsFileSection(bufferLines[i]) { + i++ // in no file section; leave it unresolved + continue + } + end := fileSectionEnd(bufferLines, i) + copy(result[i:end], parseFileSection(bufferLines[i:end], end == len(bufferLines))) + i = end + } + return result +} + +// 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 +// section, or below the end of the last one that begins above it. +func fileSectionBounds(bufferLines []string, targetIdx int) (start, end int) { + for start = targetIdx; start >= 0; start-- { + if !startsFileSection(bufferLines[start]) { + continue + } + if end = fileSectionEnd(bufferLines, start); targetIdx < end { + return start, end + } + return -1, -1 + } + return -1, -1 +} + +// fileSectionEnd returns the line the file section beginning at start ends before. +// +// A file's section runs to the next one, since every line between them is part of +// its diff. A submodule's runs only as far as what git writes for it — the line +// naming it, and the log of the commits it moved over — because the lines after +// that need not belong to any section at all. A diff renderer's output has no +// "diff --git" line to stop at, and the rows it puts between one file and the next +// belong to neither. +func fileSectionEnd(bufferLines []string, start int) int { + if submodulePath(bufferLines[start]) != "" { + end := start + 1 + for end < len(bufferLines) && isSubmoduleLogLine(bufferLines[end]) { + end++ + } + return end + } + + for i := start + 1; i < len(bufferLines); i++ { + if startsFileSection(bufferLines[i]) { + return i + } + } + return len(bufferLines) +} + +// startsFileSection reports whether the line opens a section of a diff: git's header +// for a file of the repo, or the line a submodule's section begins with. +func startsFileSection(line string) bool { + return strings.HasPrefix(line, diffFilePrefix) || submodulePath(line) != "" +} + +// isSubmoduleLogLine reports whether the line is one of the commits git lists under +// a submodule's header, which it writes as two spaces, the direction the commit was +// moved in, and the commit's subject. +func isSubmoduleLogLine(line string) bool { + return strings.HasPrefix(line, " > ") || strings.HasPrefix(line, " < ") +} + +// submodulePath returns the submodule whose section the given line opens, and "" for +// every other line. +// +// A submodule gets no "diff --git" header and no hunks: git states which commits it +// moved between and lists them, so that one line is all there is to take the path +// from. The prefix is tested first so that the pattern is run over next to no lines +// of a diff. +func submodulePath(line string) string { + if !strings.HasPrefix(line, submodulePrefix) { + return "" + } + if match := submoduleSectionPattern.FindStringSubmatch(line); match != nil { + return match[1] + } + return "" +} + +// parseFileSection parses one file's diff section (fileLines, starting at the line +// that opens it) a single time and returns the identity of each of its +// lines, indexed 1:1 with fileLines. patch.Parse's line indices line up with the +// section's buffer lines, so the type and the old/new line numbers fall out of +// the patch arithmetic. A submodule's section has no hunks at all, so every row of +// it comes out as a header of the submodule, which is what they are: what git states +// there is which commits it moved between, not lines of a file. +// +// Every line is left ok=false when the section has no +// recoverable path or isn't a well-formed unified diff — the rendering +// restructured it, and acting on a mis-parse would land us on the wrong line, so +// the caller should fall back. +// +// endsTheBuffer says the section runs to the end of what we were given. That is +// where a diff we have only part of breaks off. A long one is read a screenful +// at a time and the rest as the user scrolls, so its last hunk holds fewer lines +// than its header declares until the reading is done. Insisting on the whole +// hunk there would leave every line of the file unresolved while the diff is the +// one on screen, so a section in that position is held to what has arrived. +func parseFileSection(fileLines []string, endsTheBuffer bool) []bufferLineParse { + result := make([]bufferLineParse, len(fileLines)) + + relPath := pathFromDiffHeader(fileLines) + if relPath == "" { + return result + } + p := patch.Parse(strings.Join(fileLines, "\n")) + isWellFormed := p.IsWellFormed + if endsTheBuffer { + isWellFormed = p.IsWellFormedSoFar + } + if !isWellFormed() { + return result + } + patchLines := p.Lines() + for i := range fileLines { + if i >= len(patchLines) { + break + } + parsed := parsedDiffLine{ + RelPath: relPath, + Type: diffLineTypeForKind(patchLines[i].Kind), + NewLine: p.LineNumberOfLine(i), + } + if parsed.Type == types.DiffLineDeleted { + parsed.OldLine = p.OldLineNumberOfLine(i) + } + result[i] = bufferLineParse{parsed, true} + } + return result +} + +func diffLineTypeForKind(kind patch.PatchLineKind) types.DiffLineType { + switch kind { + case patch.PATCH_HEADER: + return types.DiffLineFileHeader + case patch.HUNK_HEADER: + return types.DiffLineHunkHeader + case patch.ADDITION: + return types.DiffLineAdded + case patch.DELETION: + return types.DiffLineDeleted + case patch.CONTEXT: + return types.DiffLineContext + default: + return types.DiffLineOther + } +} + +// pathFromDiffHeader extracts the new-file path of a single diff section. A +// submodule's section states its path in the line it opens with. For a file of the +// repo the path comes from the "+++ b/" line, falling back to "--- a/" +// when the new path is /dev/null (a deleted file), and to the "diff --git" line when +// there are no such lines at all (a pure rename, which has no hunks). +func pathFromDiffHeader(fileLines []string) string { + if path := submodulePath(fileLines[0]); path != "" { + return path + } + + var oldPath, newPath string + for _, line := range fileLines { + if strings.HasPrefix(line, "@@") { + break // past the header + } + switch { + case strings.HasPrefix(line, "+++ "): + newPath = pathFromDiffHeaderField(strings.TrimPrefix(line, "+++ ")) + case strings.HasPrefix(line, "--- "): + oldPath = pathFromDiffHeaderField(strings.TrimPrefix(line, "--- ")) + } + } + + if newPath != "" && newPath != "/dev/null" { + return newPath + } + if oldPath != "" && oldPath != "/dev/null" { + return oldPath + } + return pathFromDiffGitLine(fileLines[0]) +} + +// pathFromDiffHeaderField decodes one path field of a diff header — the part +// after "--- " or "+++ ", or one of the two paths on the "diff --git" line — +// into the repo-relative path it names. +// +// git spells such a field in three ways: plain; terminated by a tab, when the +// path contains a space; or C-quoted as a whole, when the path contains +// characters git won't print raw — which, with core.quotePath enabled (the +// default), includes every non-ASCII byte, so `café` arrives as +// `"b/caf\303\251"`. The quoting is Go's string syntax, octal escapes included, +// so strconv decodes it for us. +// +// Returns "" for a quoted field we can't decode: better to resolve nothing than +// to point a consumer at a path that doesn't exist. +func pathFromDiffHeaderField(field string) string { + field = strings.TrimSuffix(field, "\t") + + if strings.HasPrefix(field, `"`) { + unquoted, err := strconv.Unquote(field) + if err != nil { + return "" + } + field = unquoted + } + + return stripDiffPathPrefix(field) +} + +// stripDiffPathPrefix removes the a/ or b/ prefix git puts on the paths in a +// diff header. We ask git for these prefixes explicitly (diff.noprefix=false), +// so they are always there. +func stripDiffPathPrefix(path string) string { + if strings.HasPrefix(path, "a/") || strings.HasPrefix(path, "b/") { + return path[2:] + } + return path +} + +// pathFromDiffGitLine extracts the new-file path from a "diff --git a/X b/X" +// line, where the two paths are separated by a space and either may be quoted. +// A path containing " b/" (or ` "b/`) would defeat this, but the +++/--- lines +// are unambiguous and we only get here when they are absent. +func pathFromDiffGitLine(line string) string { + rest := strings.TrimPrefix(line, diffFilePrefix) + if idx := strings.LastIndex(rest, ` "b/`); idx != -1 { + return pathFromDiffHeaderField(rest[idx+1:]) + } + if idx := strings.LastIndex(rest, " b/"); idx != -1 { + return pathFromDiffHeaderField(rest[idx+1:]) + } + return "" +} diff --git a/pkg/gui/controllers/helpers/diff_line_parser_test.go b/pkg/gui/controllers/helpers/diff_line_parser_test.go new file mode 100644 index 000000000..ae620d23c --- /dev/null +++ b/pkg/gui/controllers/helpers/diff_line_parser_test.go @@ -0,0 +1,322 @@ +package helpers + +import ( + "slices" + "strings" + "testing" + + "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/stretchr/testify/assert" +) + +// A two-file commit diff as it appears (decolorized) in the main view. file1 has +// two consecutive deletions (grape, pear) that share a new-file line number; +// file2 has two consecutive additions. +const twoFileDiff = `diff --git a/file1.go b/file1.go +index 1111111..2222222 100644 +--- a/file1.go ++++ b/file1.go +@@ -1,4 +1,2 @@ + apple +-grape +-pear + lemon +diff --git a/dir/file2.go b/dir/file2.go +index 3333333..4444444 100644 +--- a/dir/file2.go ++++ b/dir/file2.go +@@ -10,2 +9,4 @@ func foo() { + ctx ++added1 ++added2 + ctx2` + +func TestParseDiffLineFromBuffer(t *testing.T) { + bufferLines := strings.Split(twoFileDiff, "\n") + + scenarios := []struct { + name string + targetIdx int + expected parsedDiffLine + expectOk bool + }{ + {"file header", 0, parsedDiffLine{RelPath: "file1.go", Type: types.DiffLineFileHeader, NewLine: 1}, true}, + {"hunk header", 4, parsedDiffLine{RelPath: "file1.go", Type: types.DiffLineHunkHeader, NewLine: 1}, true}, + {"context line", 5, parsedDiffLine{RelPath: "file1.go", Type: types.DiffLineContext, NewLine: 1}, true}, + // The two deletions share new-file line 2 but have distinct old-file lines. + {"first deletion", 6, parsedDiffLine{RelPath: "file1.go", Type: types.DiffLineDeleted, NewLine: 2, OldLine: 2}, true}, + {"second deletion", 7, parsedDiffLine{RelPath: "file1.go", Type: types.DiffLineDeleted, NewLine: 2, OldLine: 3}, true}, + // The second file: its path comes from the second "diff --git" section, + // and its additions get distinct new-file line numbers. + {"first addition", 15, parsedDiffLine{RelPath: "dir/file2.go", Type: types.DiffLineAdded, NewLine: 10}, true}, + {"second addition", 16, parsedDiffLine{RelPath: "dir/file2.go", Type: types.DiffLineAdded, NewLine: 11}, true}, + {"out of range", 999, parsedDiffLine{}, false}, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + result, ok := parseDiffLineFromBuffer(bufferLines, s.targetIdx) + assert.Equal(t, s.expectOk, ok) + if s.expectOk { + assert.Equal(t, s.expected, result) + } + }) + } +} + +func TestParseDiffLineFromBufferRename(t *testing.T) { + // A rename with no content change has no hunks and no +++/--- lines, so the + // path has to come from the "diff --git" line; a rename with a content + // change has them, and they carry the new path. + pureRename := strings.Split(`diff --git a/old.go b/new.go +similarity index 100% +rename from old.go +rename to new.go`, "\n") + + result, ok := parseDiffLineFromBuffer(pureRename, 2) + assert.True(t, ok) + assert.Equal(t, parsedDiffLine{RelPath: "new.go", Type: types.DiffLineFileHeader, NewLine: 1}, result) + + renameWithModification := strings.Split(`diff --git a/old.go b/new.go +similarity index 62% +rename from old.go +rename to new.go +index 1111111..2222222 100644 +--- a/old.go ++++ b/new.go +@@ -1,2 +1,2 @@ + apple +-grape ++kiwi`, "\n") + + result, ok = parseDiffLineFromBuffer(renameWithModification, 10) + assert.True(t, ok) + assert.Equal(t, parsedDiffLine{RelPath: "new.go", Type: types.DiffLineAdded, NewLine: 2}, result) +} + +func TestParseDiffLineFromBufferDeletedFile(t *testing.T) { + // The new path is /dev/null, so the identity comes from the old path. + deletedFile := strings.Split(`diff --git a/gone.go b/gone.go +deleted file mode 100644 +index 1111111..0000000 +--- a/gone.go ++++ /dev/null +@@ -1,2 +0,0 @@ +-apple +-grape`, "\n") + + result, ok := parseDiffLineFromBuffer(deletedFile, 7) + assert.True(t, ok) + assert.Equal(t, parsedDiffLine{RelPath: "gone.go", Type: types.DiffLineDeleted, NewLine: 0, OldLine: 2}, result) +} + +func TestParseDiffLineFromBufferSubmodule(t *testing.T) { + // A submodule has no "diff --git" header and no hunks: git opens its section with + // the commits it moved between and lists them below. So the section ends the one + // above it, and every row of it belongs to the submodule as a whole. + withSubmodule := strings.Split(`diff --git a/file.txt b/file.txt +index 1111111..2222222 100644 +--- a/file.txt ++++ b/file.txt +@@ -1,2 +1,3 @@ + hello + world ++world +Submodule modules/xyz a32f27c..2d9f921: + > bump the thing`, "\n") + + for _, targetIdx := range []int{8, 9} { + result, ok := parseDiffLineFromBuffer(withSubmodule, targetIdx) + assert.True(t, ok) + assert.Equal(t, + parsedDiffLine{RelPath: "modules/xyz", Type: types.DiffLineFileHeader, NewLine: 1}, + result, "line %d", targetIdx) + } + + result, ok := parseDiffLineFromBuffer(withSubmodule, 7) + assert.True(t, ok) + assert.Equal(t, parsedDiffLine{RelPath: "file.txt", Type: types.DiffLineAdded, NewLine: 3}, result) +} + +func TestParseDiffLineFromBufferSubmoduleInARendering(t *testing.T) { + // A diff renderer passes the lines git writes for a submodule through as they + // are, while printing nothing below them that a section could end at. The + // section has to end where what git writes for the submodule ends all the same: + // the rows below belong to the other files of the diff, and can only be placed + // by the records the renderer states for them. + rendered := strings.Split(`Submodule modules/xyz a32f27c..2d9f921: + > bump the thing + +products/a.txt + + one + two`, "\n") + + all := parseAllDiffLinesFromBuffer(rendered) + assert.Equal(t, "modules/xyz", all[0].parsed.RelPath) + assert.Equal(t, "modules/xyz", all[1].parsed.RelPath) + for i := 2; i < len(rendered); i++ { + assert.False(t, all[i].ok, "line %d: %q", i, rendered[i]) + } +} + +func TestSubmodulePath(t *testing.T) { + scenarios := []struct { + name string + line string + expected string + }{ + {"moved on", "Submodule modules/xyz a32f27c..2d9f921:", "modules/xyz"}, + {"moved back", "Submodule modules/xyz 2d9f921..a32f27c (rewind):", "modules/xyz"}, + {"moved sideways", "Submodule modules/xyz a32f27c...2d9f921:", "modules/xyz"}, + {"added", "Submodule modules/xyz 0000000...2d9f921 (new submodule)", "modules/xyz"}, + {"removed", "Submodule modules/xyz a32f27c...0000000 (submodule deleted)", "modules/xyz"}, + {"commits missing", "Submodule modules/xyz a32f27c...2d9f921 (commits not present)", "modules/xyz"}, + {"dirty", "Submodule modules/xyz contains modified content", "modules/xyz"}, + {"with something new in it", "Submodule modules/xyz contains untracked content", "modules/xyz"}, + // git writes the path unquoted, so one with a space in it, or one ending in + // something that reads like a range of commits, is told apart by matching the + // last range on the line. + {"path with a space", "Submodule my modules/xyz a32f27c..2d9f921:", "my modules/xyz"}, + {"path reading like a range", "Submodule a32f27c..2d9f921 deadbee..fa1afe1:", "a32f27c..2d9f921"}, + // A line of a file that reads like one of these is indented by the column the + // diff states the line's side in, so it cannot be mistaken for one. + {"a line of a file", " Submodule modules/xyz contains modified content", ""}, + {"something else entirely", "Submodule support was added", ""}, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, submodulePath(s.line)) + }) + } +} + +func TestParseDiffLineFromBufferNotADiff(t *testing.T) { + // A rendering with no "diff --git" line can't be parsed, so the caller falls + // back rather than acting on the line. + bufferLines := []string{"some", "lines", "that", "are not a diff"} + _, ok := parseDiffLineFromBuffer(bufferLines, 2) + assert.False(t, ok) +} + +func TestParseDiffLineFromBufferGutterMangled(t *testing.T) { + // A diff renderer that moves the line numbers into a gutter keeps the diff + // and hunk headers but pushes the +/- markers off the start of each body + // line, so every line reads as context. The body no longer matches the hunk + // header, so we refuse to parse rather than return a confident mis-parse. + mangled := strings.Split(`diff --git a/file1.txt b/file1.txt +index 1111111..2222222 100644 +--- a/file1.txt ++++ b/file1.txt +@@ -1,5 +1,3 @@ + 1 ⋮ 1 │ apple + 2 ⋮ │-grape + 3 ⋮ │-pear + 4 ⋮ 2 │ lemon + 5 ⋮ 3 │ mango`, "\n") + + _, ok := parseDiffLineFromBuffer(mangled, 6) + assert.False(t, ok) +} + +func TestParseDiffLineFromBufferReadInPart(t *testing.T) { + lines := strings.Split(twoFileDiff, "\n") + + // A long diff is read a screenful at a time, so the buffer breaks off part way + // through a hunk. The lines that did arrive are resolved all the same, since + // holding out for the whole hunk would leave the diff on screen with nothing to + // act on. + cutShort := lines[:len(lines)-1] + result, ok := parseDiffLineFromBuffer(cutShort, 15) + assert.True(t, ok) + assert.Equal(t, parsedDiffLine{RelPath: "dir/file2.go", Type: types.DiffLineAdded, NewLine: 10}, result) + + // Only the section the buffer breaks off in is read that way. One that another + // section follows is all there, so a hunk short of what its header declares means + // the rendering restructured the diff, and none of it is resolved. + shortFirstSection := append(slices.Clone(lines[:8]), lines[9:]...) + _, ok = parseDiffLineFromBuffer(shortFirstSection, 5) + assert.False(t, ok) +} + +func TestPathFromDiffHeaderField(t *testing.T) { + scenarios := []struct { + name string + field string + expected string + }{ + {"new side", "b/file.go", "file.go"}, + {"old side", "a/file.go", "file.go"}, + {"a missing file", "/dev/null", "/dev/null"}, + // git terminates the field with a tab when the path has a space in it. + {"path with a space", "b/with space.go\t", "with space.go"}, + // With core.quotePath enabled (the default) every non-ASCII byte is + // escaped, and the field is quoted as a whole, prefix included. + {"non-ASCII path", `"b/caf\303\251.go"`, "café.go"}, + {"non-ASCII path with a space", "\"b/caf\\303\\251 x.go\"\t", "café x.go"}, + {"path with a double quote", `"b/we\"ird.go"`, `we"ird.go`}, + {"path with a backslash", `"b/back\\slash.go"`, `back\slash.go`}, + {"path with a tab", `"b/tab\there.go"`, "tab\there.go"}, + {"undecodable", `"b/unterminated`, ""}, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, pathFromDiffHeaderField(s.field)) + }) + } +} + +func TestParseDiffLineFromBufferQuotedPath(t *testing.T) { + // A rename of a file whose name needs quoting, with a content change: the + // path is quoted on the "diff --git" line and on both of the +++/--- lines. + renamed := []string{ + `diff --git "a/caf\303\251 old.go" "b/caf\303\251 new.go"`, + "similarity index 62%", + `rename from "caf\303\251 old.go"`, + `rename to "caf\303\251 new.go"`, + "index 1111111..2222222 100644", + "--- \"a/caf\\303\\251 old.go\"\t", + "+++ \"b/caf\\303\\251 new.go\"\t", + "@@ -1,2 +1,2 @@", + " apple", + "-grape", + "+kiwi", + } + + result, ok := parseDiffLineFromBuffer(renamed, 10) + assert.True(t, ok) + assert.Equal(t, parsedDiffLine{RelPath: "café new.go", Type: types.DiffLineAdded, NewLine: 2}, result) + + // The same rename without a content change has no +++/--- lines, so the path + // comes from the "diff --git" line, where both paths are quoted. + result, ok = parseDiffLineFromBuffer(renamed[:4], 2) + assert.True(t, ok) + assert.Equal(t, parsedDiffLine{RelPath: "café new.go", Type: types.DiffLineFileHeader, NewLine: 1}, result) +} + +func TestParseAllDiffLinesFromBuffer(t *testing.T) { + // Some decoration above the diff, which belongs to no file section: a commit + // message and a diffstat, as `git show` renders them. + bufferLines := append( + []string{"commit 1234567", "", " do a thing", "", " file1.go | 2 --", ""}, + strings.Split(twoFileDiff, "\n")..., + ) + + all := parseAllDiffLinesFromBuffer(bufferLines) + + // The batch parse resolves each file section once, and has to agree with + // resolving the lines one at a time. + assert.Len(t, all, len(bufferLines)) + for i := range bufferLines { + parsed, ok := parseDiffLineFromBuffer(bufferLines, i) + assert.Equal(t, bufferLineParse{parsed, ok}, all[i], "line %d: %q", i, bufferLines[i]) + } + + // The lines above the first file section are left unresolved. + for i := range 6 { + assert.False(t, all[i].ok) + } + assert.True(t, all[6].ok) +} diff --git a/pkg/gui/controllers/helpers/helpers.go b/pkg/gui/controllers/helpers/helpers.go index 4c9c79f3d..ea1214699 100644 --- a/pkg/gui/controllers/helpers/helpers.go +++ b/pkg/gui/controllers/helpers/helpers.go @@ -39,6 +39,7 @@ type Helpers struct { Snake *SnakeHelper // lives in context package because our contexts need it to render to main Diff *DiffHelper + DiffLine *DiffLineHelper Repos *ReposHelper RecordDirectory *RecordDirectoryHelper Update *UpdateHelper @@ -76,6 +77,7 @@ func NewStubHelpers() *Helpers { Commits: &CommitsHelper{}, Snake: &SnakeHelper{}, Diff: &DiffHelper{}, + DiffLine: &DiffLineHelper{}, Repos: &ReposHelper{}, RecordDirectory: &RecordDirectoryHelper{}, Update: &UpdateHelper{}, diff --git a/pkg/gui/types/diff_line_info.go b/pkg/gui/types/diff_line_info.go new file mode 100644 index 000000000..ae3d8eee9 --- /dev/null +++ b/pkg/gui/types/diff_line_info.go @@ -0,0 +1,42 @@ +package types + +// DiffLineType classifies a row of a rendered diff. +type DiffLineType int + +const ( + DiffLineFileHeader DiffLineType = iota + DiffLineHunkHeader + DiffLineContext + DiffLineAdded + DiffLineDeleted + // DiffLineOther is anything that isn't one of the above, e.g. the + // "\ No newline at end of file" marker. + DiffLineOther +) + +// DiffLineInfo is the identity of a row of a rendered diff in terms of the patch +// it was rendered from: which file the row belongs to, what kind of row it is, +// and where the line sits in the old and new versions of that file. This lets us +// act on the line the user is pointing at in a diff view: stage it, open it in an +// editor, keep the cursor on it across a re-render. The rendered text alone tells +// us none of that. +type DiffLineInfo struct { + // Path is the absolute path of the file the line belongs to. + Path string + Type DiffLineType + // NewLine is the line's position in the new version of the file. Set for all + // content lines (for a deletion it is the position the deletion sits at) and + // for hunk headers (the first line of the hunk they head). + NewLine int + // OldLine is the line's position in the old version of the file. Set only + // for deletions, which are the only rows that need it: two consecutive + // deletions share a new-file position and differ only here. + OldLine int +} + +// IsChange reports whether the row is an added or deleted line, as opposed to a +// context line or a header. It mirrors patch.PatchLine.IsChange: those are the +// rows a patch is built from, and the rows navigation moves between. +func (self DiffLineInfo) IsChange() bool { + return self.Type == DiffLineAdded || self.Type == DiffLineDeleted +}