From a7df0d5f4135a640e867ae350bf9a3bd6997ce79 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 7 Aug 2026 10:44:55 +0800 Subject: [PATCH] fix: git diff blob excerpt (#38808) 1. refactor the legacy code and add more comments, remove the "+1/-1" tricks, clarify the BuildBlobExcerptDiffSection behavior 2. fix a line-counting bug (see screenshot below) --- modules/git/blob.go | 20 ++++++-- modules/git/blob_test.go | 18 +++++++ services/gitdiff/gitdiff.go | 46 +++++++++-------- services/gitdiff/gitdiff_excerpt.go | 66 ++++++++++++++----------- services/gitdiff/gitdiff_render_test.go | 4 +- 5 files changed, 98 insertions(+), 56 deletions(-) diff --git a/modules/git/blob.go b/modules/git/blob.go index 0565353ef7f..8118eadccf1 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -49,20 +49,32 @@ func (b *Blob) GetBlobLineCount(ctx context.Context, w io.Writer) (size int64, c return 0, 0, err } defer reader.Close() + return getBlobLineCount(reader, w) +} + +func getBlobLineCount(r io.Reader, w io.Writer) (size int64, count int, _ error) { buf := make([]byte, 32*1024) - size, count = 0, 1 - lineSep := []byte{'\n'} + size, count = 0, 0 + var lastChar byte + lineSep := []byte("\n") for { - c, err := reader.Read(buf) + c, err := r.Read(buf) size += int64(c) if w != nil { if _, err := w.Write(buf[:c]); err != nil { return size, count, err } } - count += bytes.Count(buf[:c], lineSep) + if c > 0 { + count += bytes.Count(buf[:c], lineSep) + lastChar = buf[c-1] + } switch { case errors.Is(err, io.EOF): + if size > 0 && lastChar != '\n' { + // it should match "git diff" hunk line number. "a\nb" => 2 lines, "a\nb\n" => 2 lines + count++ + } return size, count, nil case err != nil: return size, count, err diff --git a/modules/git/blob_test.go b/modules/git/blob_test.go index 6fe4418420d..c06c821466d 100644 --- a/modules/git/blob_test.go +++ b/modules/git/blob_test.go @@ -7,6 +7,7 @@ package git import ( "io" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -56,3 +57,20 @@ func Benchmark_Blob_Data(b *testing.B) { _ = r.Close() } } + +func TestGetBlobLineCount(t *testing.T) { + size, count, err := getBlobLineCount(strings.NewReader(""), nil) + assert.NoError(t, err) + assert.EqualValues(t, 0, size) + assert.Equal(t, 0, count) + + size, count, err = getBlobLineCount(strings.NewReader("\n"), nil) + assert.NoError(t, err) + assert.EqualValues(t, 1, size) + assert.Equal(t, 1, count) + + size, count, err = getBlobLineCount(strings.NewReader("a\nb"), nil) + assert.NoError(t, err) + assert.EqualValues(t, 3, size) + assert.Equal(t, 2, count) +} diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 1b17aec973e..6fde7daf68e 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -83,37 +83,41 @@ type DiffLine struct { cachedDiffInline *DiffInline } -// DiffLineSectionInfo represents diff line section meta data +// DiffLineSectionInfo represents diff line section metadata type DiffLineSectionInfo struct { language *diffVarMutable[string] Path string - // These line "idx" are 1-based line numbers + // These line "idx" are 1-based line numbers (inclusive) // Left/Right refer to the left/right side of the diff: // - // LastLeftIdx | LastRightIdx - // [up/down expander] @@ hunk info @@ - // LeftIdx | RightIdx - - LastLeftIdx int - LastRightIdx int - LeftIdx int - RightIdx int - - // Hunk sizes of the hidden lines - LeftHunkSize int - RightHunkSize int - + // LastLeftIdx | LastRightIdx (the last rendered line number before this hunk) + // [up/down/single expander] @@ hunk info @@ + // LeftIdx | RightIdx (the next rendered line number after this hunk) + // The hunk has LeftHunkSize lines on left side, RightHunkSize lines on right side. + // // For example: - // 17 | 31 - // [up/down] @@ -40,23 +54,9 @@ .... - // 40 | 54 + // 17 | 31 diff line ... + // [up/down] @@ -40,23 +54,7 @@ .... + // 40 | 54 diff line ... + // ... diff line ... + // 62 | 60 diff line ... + // (then file end or another hunk) // // In this case: - // LastLeftIdx = 17, LastRightIdx = 31 - // LeftHunkSize = 23, RightHunkSize = 9 - // LeftIdx = 40, RightIdx = 54 + // LastLeftIdx = 17, LastRightIdx = 31 + // (left lines 18-39, right lines 31-53 are hidden) + // LeftIdx = 40, RightIdx = 54 + // LeftHunkSize = 23, RightHunkSize = 7 + // Left hunk ends at line 40+23-1=62 (23 lines), right: 54+7-1=60 (7 lines) + + LastLeftIdx int + LastRightIdx int + LeftIdx int + RightIdx int + LeftHunkSize int + RightHunkSize int HiddenCommentIDs []int64 // IDs of hidden comments in this section } diff --git a/services/gitdiff/gitdiff_excerpt.go b/services/gitdiff/gitdiff_excerpt.go index f5ff64830ff..6604795abac 100644 --- a/services/gitdiff/gitdiff_excerpt.go +++ b/services/gitdiff/gitdiff_excerpt.go @@ -16,6 +16,7 @@ import ( ) type BlobExcerptOptions struct { + // More details in DiffLineSectionInfo struct LastLeft int LastRight int LeftIndex int @@ -26,11 +27,11 @@ type BlobExcerptOptions struct { Language string } -func fillExcerptLines(section *DiffSection, filePath string, reader io.Reader, lang string, idxLeft, idxRight, chunkSize int) error { +func (diffSection *DiffSection) fillExcerptLines(reader io.Reader, leftStart, rightStart, chunkSize int) error { buf := &bytes.Buffer{} scanner := bufio.NewScanner(reader) var diffLines []*DiffLine - for line := 0; line < idxRight+chunkSize; line++ { + for rightLineIdx := 1; rightLineIdx < rightStart+chunkSize; rightLineIdx++ { if ok := scanner.Scan(); !ok { break } @@ -39,12 +40,12 @@ func fillExcerptLines(section *DiffSection, filePath string, reader io.Reader, l buf.WriteString(lineText) buf.WriteByte('\n') } - if line < idxRight { + if rightLineIdx < rightStart { continue } diffLine := &DiffLine{ - LeftIdx: idxLeft + (line - idxRight) + 1, - RightIdx: line + 1, + LeftIdx: leftStart + (rightLineIdx - rightStart), + RightIdx: rightLineIdx, Type: DiffLinePlain, Content: " " + lineText, } @@ -53,46 +54,53 @@ func fillExcerptLines(section *DiffSection, filePath string, reader io.Reader, l if err := scanner.Err(); err != nil { return fmt.Errorf("fillExcerptLines scan: %w", err) } - section.Lines = diffLines + diffSection.Lines = diffLines // DiffLinePlain always uses right lines - section.highlightedRightLines.value = highlightCodeLines(filePath, lang, []*DiffSection{section}, false /* right */, buf.Bytes()) + diffSection.highlightedRightLines.value = highlightCodeLines(diffSection.FileName, diffSection.language.value, []*DiffSection{diffSection}, false /* right */, buf.Bytes()) return nil } func BuildBlobExcerptDiffSection(filePath string, reader io.Reader, opts BlobExcerptOptions) (*DiffSection, error) { lastLeft, lastRight, idxLeft, idxRight := opts.LastLeft, opts.LastRight, opts.LeftIndex, opts.RightIndex leftHunkSize, rightHunkSize, direction := opts.LeftHunkSize, opts.RightHunkSize, opts.Direction - language := opts.Language - chunkSize := BlobExcerptChunkSize + expandLimit := BlobExcerptChunkSize section := &DiffSection{ - language: &diffVarMutable[string]{value: language}, + language: &diffVarMutable[string]{value: opts.Language}, highlightLexer: &diffVarMutable[chroma.Lexer]{}, highlightedLeftLines: &diffVarMutable[map[int]template.HTML]{}, highlightedRightLines: &diffVarMutable[map[int]template.HTML]{}, FileName: filePath, } var err error - if direction == "up" && (idxLeft-lastLeft) > chunkSize { - idxLeft -= chunkSize - idxRight -= chunkSize - leftHunkSize += chunkSize - rightHunkSize += chunkSize - err = fillExcerptLines(section, filePath, reader, language, idxLeft-1, idxRight-1, chunkSize) - } else if direction == "down" && (idxLeft-lastLeft) > chunkSize { - err = fillExcerptLines(section, filePath, reader, language, lastLeft, lastRight, chunkSize) - lastLeft += chunkSize - lastRight += chunkSize - } else { - offset := -1 - if direction == "down" { - offset = 0 + remainingLines := idxRight - lastRight + if direction == "up" && remainingLines > expandLimit { + idxLeft -= expandLimit + idxRight -= expandLimit + leftHunkSize += expandLimit + rightHunkSize += expandLimit + err = section.fillExcerptLines(reader, idxLeft, idxRight, expandLimit) + } else if direction == "down" && remainingLines > expandLimit { + err = section.fillExcerptLines(reader, lastLeft+1, lastRight+1, expandLimit) + lastLeft += expandLimit + lastRight += expandLimit + } else /* "single" or [ ("up" or "down") and (remainingLines <= expandLimit) ] */ { + if direction == "up" || direction == "single" { + // if the direction is "up" or "single": + // * top: last=0, idx=11, chunk=11: line 11 is already rendered, line 0 can be considered as a "virtually rendered line" + // * then need to expand line 10 lines (1-10), so "-1". + // * middle: last=100, idx=106, chunk=6: line 100 and 106 are both already rendered + // * then need to expand 5 lines (101-105), so "-1". + expandLimit = remainingLines - 1 + } else { + // if the direction is "down": either the hidden lines are too many in the middle (otherwise "single"), or are at the bottom + // * "last" line is already rendered, so just render the remaining lines from the next line + expandLimit = remainingLines } - err = fillExcerptLines(section, filePath, reader, language, lastLeft, lastRight, idxRight-lastRight+offset) - leftHunkSize = 0 - rightHunkSize = 0 - idxLeft = lastLeft - idxRight = lastRight + err = section.fillExcerptLines(reader, lastLeft+1, lastRight+1, expandLimit) + // now, the hidden lines are fewer than "expand limit", after expand, no hidden lines anymore, + // no need to show new "expand buttons" (setting them to 0 will make GetExpandDirection returns "no direction") + leftHunkSize, rightHunkSize, idxLeft, idxRight = 0, 0, 0, 0 } if err != nil { return nil, err diff --git a/services/gitdiff/gitdiff_render_test.go b/services/gitdiff/gitdiff_render_test.go index 8146f725cae..bf0057cf0af 100644 --- a/services/gitdiff/gitdiff_render_test.go +++ b/services/gitdiff/gitdiff_render_test.go @@ -98,8 +98,8 @@ func TestGetDiffForRender(t *testing.T) { ExpandDirection: "down", LastLeftIdx: 76, LastRightIdx: 73, - LeftIdx: 104, - RightIdx: 101, + LeftIdx: 103, // left has 103 lines + RightIdx: 100, // right has 100 lines }, } for idx, exp := range expectedSections {