feat(diff): tell why a diff file has no hunks
FileDiff.Truncated becomes Omitted: generated, large, out of the total budget, or unavailable over the memory limit. DiffLimits gains SkipGenerated, zero line limits mean no limit, and OneFileDiffLimits reads one file with the byte guard only. The API and the CLI JSON carry the reason next to truncated.
- commit
- 0e23da444f82a7a29466ef6e9a380d76c0306a04
- author
- brice · · 2026-10-20 07:31 UTC
- committer
- brice · · 2026-10-20 09:12 UTC
- parent
- 7fe3b18
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
21 files · +412 -153
M internal/adapters/gitcli/diff.go +33 -27
| @@ -115,7 +115,7 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||
| 115 | 115 | // rename, mode change) have a section without hunks. | |
| 116 | 116 | lastRead := -1 | |
| 117 | 117 | for i := from; i < to; i++ { | |
| 118 | - | if budget.WantsHunks(entries[i].file.Binary, entries[i].added+entries[i].del) { | |
| 118 | + | if read, _ := budget.Admit(entries[i].file.Binary, entries[i].file.CommentPath(), entries[i].added+entries[i].del); read { | |
| 119 | 119 | lastRead = i | |
| 120 | 120 | } | |
| 121 | 121 | } | |
| @@ -134,19 +134,23 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||
| 134 | 134 | sections = 2 | |
| 135 | 135 | } | |
| 136 | 136 | inWindow := i >= from && i < to | |
| 137 | + | read, why := false, domain.OmitNone | |
| 138 | + | if inWindow { | |
| 139 | + | read, why = pr.budget.Admit(e.file.Binary, e.file.CommentPath(), e.added+e.del) | |
| 140 | + | } | |
| 137 | 141 | switch { | |
| 138 | - | case !inWindow, !pr.budget.ChangedLines(e.file.Binary, e.added+e.del): | |
| 142 | + | case !inWindow: | |
| 139 | 143 | // Nothing to show. | |
| 140 | - | case !pr.budget.WantsHunks(e.file.Binary, e.added+e.del): | |
| 141 | - | e.file.Truncated = true | |
| 144 | + | case !read: | |
| 145 | + | e.file.Omitted = why | |
| 142 | 146 | default: | |
| 143 | 147 | before := pr.budget | |
| 144 | - | hunks, _, truncated, err := pr.file(sections, pr.budget.FileLines()) | |
| 148 | + | hunks, _, why, err := pr.file(sections, pr.budget.FileLines()) | |
| 145 | 149 | if err != nil { | |
| 146 | 150 | return domain.Diff{}, domain.DiffStats{}, err | |
| 147 | 151 | } | |
| 148 | - | e.file.Hunks, e.file.Truncated = hunks, truncated | |
| 149 | - | if truncated { | |
| 152 | + | e.file.Hunks, e.file.Omitted = hunks, why | |
| 153 | + | if why != domain.OmitNone { | |
| 150 | 154 | pr.budget = before // nothing of this file is kept | |
| 151 | 155 | } | |
| 152 | 156 | sections = 0 | |
| @@ -156,7 +160,6 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||
| 156 | 160 | return domain.Diff{}, domain.DiffStats{}, err | |
| 157 | 161 | } | |
| 158 | 162 | } | |
| 159 | - | diff.Truncated = diff.Truncated || e.file.Truncated | |
| 160 | 163 | diff.Files = append(diff.Files, e.file) | |
| 161 | 164 | } | |
| 162 | 165 | if lastRead < len(entries)-1 { | |
| @@ -379,55 +382,56 @@ func (p *patchReader) skip(n int) error { | |||
| 379 | 382 | } | |
| 380 | 383 | ||
| 381 | 384 | // file reads the sections of one file and returns its hunks, the number of | |
| 382 | - | // changed lines, and whether it exceeded max changed lines or had a line over | |
| 383 | - | // maxDiffLine. When truncated, the rest of the file is skipped. | |
| 384 | - | func (p *patchReader) file(sections, max int) (hunks []domain.Hunk, lines int, truncated bool, err error) { | |
| 385 | + | // changed lines, and why it was omitted: it exceeded max changed lines or the | |
| 386 | + | // budget, or had a line over maxDiffLine. When omitted, the rest of the file is | |
| 387 | + | // skipped. | |
| 388 | + | func (p *patchReader) file(sections, max int) (hunks []domain.Hunk, lines int, why domain.FileOmission, err error) { | |
| 385 | 389 | for done := range sections { | |
| 386 | 390 | ok, err := p.enterSection() | |
| 387 | 391 | if err != nil { | |
| 388 | - | return nil, 0, false, err | |
| 392 | + | return nil, 0, domain.OmitNone, err | |
| 389 | 393 | } | |
| 390 | 394 | if !ok { | |
| 391 | 395 | break | |
| 392 | 396 | } | |
| 393 | - | h, n, trunc, err := p.section(max - lines) | |
| 397 | + | h, n, why, err := p.section(max - lines) | |
| 394 | 398 | if err != nil { | |
| 395 | - | return nil, 0, false, err | |
| 399 | + | return nil, 0, domain.OmitNone, err | |
| 396 | 400 | } | |
| 397 | 401 | hunks = append(hunks, h...) | |
| 398 | 402 | lines += n | |
| 399 | - | if trunc { | |
| 403 | + | if why != domain.OmitNone { | |
| 400 | 404 | // Leave the stream at the start of the next file. | |
| 401 | - | return nil, lines, true, p.skip(sections - done - 1) | |
| 405 | + | return nil, lines, why, p.skip(sections - done - 1) | |
| 402 | 406 | } | |
| 403 | 407 | } | |
| 404 | - | return hunks, lines, false, nil | |
| 408 | + | return hunks, lines, domain.OmitNone, nil | |
| 405 | 409 | } | |
| 406 | 410 | ||
| 407 | 411 | // section reads the hunks of the current section up to the next marker, which | |
| 408 | - | // stays pending. It gives up (truncated) after more than max changed lines. | |
| 409 | - | func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncated bool, err error) { | |
| 412 | + | // stays pending. It gives up (with a reason) after more than max changed lines. | |
| 413 | + | func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, why domain.FileOmission, err error) { | |
| 410 | 414 | var cur *domain.Hunk | |
| 411 | 415 | var oldN, newN int | |
| 412 | 416 | for { | |
| 413 | 417 | line, ok, err := p.line() | |
| 414 | 418 | if err != nil { | |
| 415 | - | return nil, 0, false, err | |
| 419 | + | return nil, 0, domain.OmitNone, err | |
| 416 | 420 | } | |
| 417 | 421 | if !ok { | |
| 418 | - | return hunks, lines, false, nil | |
| 422 | + | return hunks, lines, domain.OmitNone, nil | |
| 419 | 423 | } | |
| 420 | 424 | if bytes.HasPrefix(line, []byte(sectionMarker)) { | |
| 421 | 425 | p.unread(line) | |
| 422 | - | return hunks, lines, false, nil | |
| 426 | + | return hunks, lines, domain.OmitNone, nil | |
| 423 | 427 | } | |
| 424 | 428 | if p.cut { | |
| 425 | - | return nil, lines, true, nil | |
| 429 | + | return nil, lines, domain.OmitUnavailable, nil | |
| 426 | 430 | } | |
| 427 | 431 | if bytes.HasPrefix(line, []byte("@@ ")) { | |
| 428 | 432 | h, err := parseHunkHeader(string(line)) | |
| 429 | 433 | if err != nil { | |
| 430 | - | return nil, 0, false, err | |
| 434 | + | return nil, 0, domain.OmitNone, err | |
| 431 | 435 | } | |
| 432 | 436 | hunks = append(hunks, h) | |
| 433 | 437 | cur = &hunks[len(hunks)-1] | |
| @@ -463,9 +467,11 @@ func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncate | |||
| 463 | 467 | if changed { | |
| 464 | 468 | lines++ | |
| 465 | 469 | } | |
| 466 | - | fits := p.budget.ChargeLine(len(text), changed) | |
| 467 | - | if lines > max || !fits { | |
| 468 | - | return nil, lines, true, nil | |
| 470 | + | if why := p.budget.ChargeLine(len(text), changed); why != domain.OmitNone { | |
| 471 | + | return nil, lines, why, nil | |
| 472 | + | } | |
| 473 | + | if lines > max { | |
| 474 | + | return nil, lines, domain.OmitLarge, nil | |
| 469 | 475 | } | |
| 470 | 476 | cur.Lines = append(cur.Lines, l) | |
| 471 | 477 | } | |
| @@ -115,7 +115,7 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||||
| 115 | // rename, mode change) have a section without hunks. | 115 | // rename, mode change) have a section without hunks. | ||
| 116 | lastRead := -1 | 116 | lastRead := -1 | ||
| 117 | for i := from; i < to; i++ { | 117 | for i := from; i < to; i++ { | ||
| 118 | - | if budget.WantsHunks(entries[i].file.Binary, entries[i].added+entries[i].del) { | 118 | + | if read, _ := budget.Admit(entries[i].file.Binary, entries[i].file.CommentPath(), entries[i].added+entries[i].del); read { |
| 119 | lastRead = i | 119 | lastRead = i | ||
| 120 | } | 120 | } | ||
| 121 | } | 121 | } | ||
| @@ -134,19 +134,23 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||||
| 134 | sections = 2 | 134 | sections = 2 | ||
| 135 | } | 135 | } | ||
| 136 | inWindow := i >= from && i < to | 136 | inWindow := i >= from && i < to | ||
| 137 | + | read, why := false, domain.OmitNone | |||
| 138 | + | if inWindow { | |||
| 139 | + | read, why = pr.budget.Admit(e.file.Binary, e.file.CommentPath(), e.added+e.del) | |||
| 140 | + | } | |||
| 137 | switch { | 141 | switch { | ||
| 138 | - | case !inWindow, !pr.budget.ChangedLines(e.file.Binary, e.added+e.del): | 142 | + | case !inWindow: |
| 139 | // Nothing to show. | 143 | // Nothing to show. | ||
| 140 | - | case !pr.budget.WantsHunks(e.file.Binary, e.added+e.del): | 144 | + | case !read: |
| 141 | - | e.file.Truncated = true | 145 | + | e.file.Omitted = why |
| 142 | default: | 146 | default: | ||
| 143 | before := pr.budget | 147 | before := pr.budget | ||
| 144 | - | hunks, _, truncated, err := pr.file(sections, pr.budget.FileLines()) | 148 | + | hunks, _, why, err := pr.file(sections, pr.budget.FileLines()) |
| 145 | if err != nil { | 149 | if err != nil { | ||
| 146 | return domain.Diff{}, domain.DiffStats{}, err | 150 | return domain.Diff{}, domain.DiffStats{}, err | ||
| 147 | } | 151 | } | ||
| 148 | - | e.file.Hunks, e.file.Truncated = hunks, truncated | 152 | + | e.file.Hunks, e.file.Omitted = hunks, why |
| 149 | - | if truncated { | 153 | + | if why != domain.OmitNone { |
| 150 | pr.budget = before // nothing of this file is kept | 154 | pr.budget = before // nothing of this file is kept | ||
| 151 | } | 155 | } | ||
| 152 | sections = 0 | 156 | sections = 0 | ||
| @@ -156,7 +160,6 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||||
| 156 | return domain.Diff{}, domain.DiffStats{}, err | 160 | return domain.Diff{}, domain.DiffStats{}, err | ||
| 157 | } | 161 | } | ||
| 158 | } | 162 | } | ||
| 159 | - | diff.Truncated = diff.Truncated || e.file.Truncated | |||
| 160 | diff.Files = append(diff.Files, e.file) | 163 | diff.Files = append(diff.Files, e.file) | ||
| 161 | } | 164 | } | ||
| 162 | if lastRead < len(entries)-1 { | 165 | if lastRead < len(entries)-1 { | ||
| @@ -379,55 +382,56 @@ func (p *patchReader) skip(n int) error { | |||||
| 379 | } | 382 | } | ||
| 380 | 383 | ||||
| 381 | // file reads the sections of one file and returns its hunks, the number of | 384 | // file reads the sections of one file and returns its hunks, the number of | ||
| 382 | - | // changed lines, and whether it exceeded max changed lines or had a line over | 385 | + | // changed lines, and why it was omitted: it exceeded max changed lines or the |
| 383 | - | // maxDiffLine. When truncated, the rest of the file is skipped. | 386 | + | // budget, or had a line over maxDiffLine. When omitted, the rest of the file is |
| 384 | - | func (p *patchReader) file(sections, max int) (hunks []domain.Hunk, lines int, truncated bool, err error) { | 387 | + | // skipped. |
| 388 | + | func (p *patchReader) file(sections, max int) (hunks []domain.Hunk, lines int, why domain.FileOmission, err error) { | |||
| 385 | for done := range sections { | 389 | for done := range sections { | ||
| 386 | ok, err := p.enterSection() | 390 | ok, err := p.enterSection() | ||
| 387 | if err != nil { | 391 | if err != nil { | ||
| 388 | - | return nil, 0, false, err | 392 | + | return nil, 0, domain.OmitNone, err |
| 389 | } | 393 | } | ||
| 390 | if !ok { | 394 | if !ok { | ||
| 391 | break | 395 | break | ||
| 392 | } | 396 | } | ||
| 393 | - | h, n, trunc, err := p.section(max - lines) | 397 | + | h, n, why, err := p.section(max - lines) |
| 394 | if err != nil { | 398 | if err != nil { | ||
| 395 | - | return nil, 0, false, err | 399 | + | return nil, 0, domain.OmitNone, err |
| 396 | } | 400 | } | ||
| 397 | hunks = append(hunks, h...) | 401 | hunks = append(hunks, h...) | ||
| 398 | lines += n | 402 | lines += n | ||
| 399 | - | if trunc { | 403 | + | if why != domain.OmitNone { |
| 400 | // Leave the stream at the start of the next file. | 404 | // Leave the stream at the start of the next file. | ||
| 401 | - | return nil, lines, true, p.skip(sections - done - 1) | 405 | + | return nil, lines, why, p.skip(sections - done - 1) |
| 402 | } | 406 | } | ||
| 403 | } | 407 | } | ||
| 404 | - | return hunks, lines, false, nil | 408 | + | return hunks, lines, domain.OmitNone, nil |
| 405 | } | 409 | } | ||
| 406 | 410 | ||||
| 407 | // section reads the hunks of the current section up to the next marker, which | 411 | // section reads the hunks of the current section up to the next marker, which | ||
| 408 | - | // stays pending. It gives up (truncated) after more than max changed lines. | 412 | + | // stays pending. It gives up (with a reason) after more than max changed lines. |
| 409 | - | func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncated bool, err error) { | 413 | + | func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, why domain.FileOmission, err error) { |
| 410 | var cur *domain.Hunk | 414 | var cur *domain.Hunk | ||
| 411 | var oldN, newN int | 415 | var oldN, newN int | ||
| 412 | for { | 416 | for { | ||
| 413 | line, ok, err := p.line() | 417 | line, ok, err := p.line() | ||
| 414 | if err != nil { | 418 | if err != nil { | ||
| 415 | - | return nil, 0, false, err | 419 | + | return nil, 0, domain.OmitNone, err |
| 416 | } | 420 | } | ||
| 417 | if !ok { | 421 | if !ok { | ||
| 418 | - | return hunks, lines, false, nil | 422 | + | return hunks, lines, domain.OmitNone, nil |
| 419 | } | 423 | } | ||
| 420 | if bytes.HasPrefix(line, []byte(sectionMarker)) { | 424 | if bytes.HasPrefix(line, []byte(sectionMarker)) { | ||
| 421 | p.unread(line) | 425 | p.unread(line) | ||
| 422 | - | return hunks, lines, false, nil | 426 | + | return hunks, lines, domain.OmitNone, nil |
| 423 | } | 427 | } | ||
| 424 | if p.cut { | 428 | if p.cut { | ||
| 425 | - | return nil, lines, true, nil | 429 | + | return nil, lines, domain.OmitUnavailable, nil |
| 426 | } | 430 | } | ||
| 427 | if bytes.HasPrefix(line, []byte("@@ ")) { | 431 | if bytes.HasPrefix(line, []byte("@@ ")) { | ||
| 428 | h, err := parseHunkHeader(string(line)) | 432 | h, err := parseHunkHeader(string(line)) | ||
| 429 | if err != nil { | 433 | if err != nil { | ||
| 430 | - | return nil, 0, false, err | 434 | + | return nil, 0, domain.OmitNone, err |
| 431 | } | 435 | } | ||
| 432 | hunks = append(hunks, h) | 436 | hunks = append(hunks, h) | ||
| 433 | cur = &hunks[len(hunks)-1] | 437 | cur = &hunks[len(hunks)-1] | ||
| @@ -463,9 +467,11 @@ func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncate | |||||
| 463 | if changed { | 467 | if changed { | ||
| 464 | lines++ | 468 | lines++ | ||
| 465 | } | 469 | } | ||
| 466 | - | fits := p.budget.ChargeLine(len(text), changed) | 470 | + | if why := p.budget.ChargeLine(len(text), changed); why != domain.OmitNone { |
| 467 | - | if lines > max || !fits { | 471 | + | return nil, lines, why, nil |
| 468 | - | return nil, lines, true, nil | 472 | + | } |
| 473 | + | if lines > max { | |||
| 474 | + | return nil, lines, domain.OmitLarge, nil | |||
| 469 | } | 475 | } | ||
| 470 | cur.Lines = append(cur.Lines, l) | 476 | cur.Lines = append(cur.Lines, l) | ||
| 471 | } | 477 | } | ||
M internal/adapters/gitcli/diff_test.go +35 -35
| @@ -177,7 +177,7 @@ func TestGetCommitFileMetadata(t *testing.T) { | |||
| 177 | 177 | assert.Equal(t, domain.FileModified, bin.Status) | |
| 178 | 178 | assert.True(t, bin.Binary) | |
| 179 | 179 | assert.Empty(t, bin.Hunks) | |
| 180 | - | assert.False(t, bin.Truncated) | |
| 180 | + | assert.Equal(t, domain.OmitNone, bin.Omitted) | |
| 181 | 181 | ||
| 182 | 182 | mode := files["d/f>d/f"] | |
| 183 | 183 | assert.Equal(t, domain.FileModified, mode.Status) | |
| @@ -185,7 +185,7 @@ func TestGetCommitFileMetadata(t *testing.T) { | |||
| 185 | 185 | assert.Equal(t, uint32(0o100755), mode.NewMode) | |
| 186 | 186 | assert.Empty(t, mode.Hunks) | |
| 187 | 187 | assert.False(t, mode.Binary) | |
| 188 | - | assert.False(t, mode.Truncated) | |
| 188 | + | assert.Equal(t, domain.OmitNone, mode.Omitted) | |
| 189 | 189 | ||
| 190 | 190 | del := files["README.md>"] | |
| 191 | 191 | assert.Equal(t, domain.FileDeleted, del.Status) | |
| @@ -315,15 +315,15 @@ func TestDiffLimits(t *testing.T) { | |||
| 315 | 315 | require.NoError(t, err) | |
| 316 | 316 | exact, over, small := find(d.Diff, "exact"), find(d.Diff, "over"), find(d.Diff, "small") | |
| 317 | 317 | ||
| 318 | - | assert.False(t, exact.Truncated) | |
| 318 | + | assert.Equal(t, domain.OmitNone, exact.Omitted) | |
| 319 | 319 | require.Len(t, exact.Hunks, 1) | |
| 320 | 320 | assert.Len(t, exact.Hunks[0].Lines, 2000) | |
| 321 | - | assert.True(t, over.Truncated) | |
| 321 | + | assert.NotEqual(t, domain.OmitNone, over.Omitted) | |
| 322 | 322 | assert.Empty(t, over.Hunks) | |
| 323 | 323 | assert.Equal(t, 2001, over.Additions, "counts stay accurate") | |
| 324 | - | assert.False(t, small.Truncated) | |
| 324 | + | assert.Equal(t, domain.OmitNone, small.Omitted) | |
| 325 | 325 | assert.Len(t, small.Hunks, 1) | |
| 326 | - | assert.True(t, d.Diff.Truncated) | |
| 326 | + | assert.False(t, d.Diff.Truncated) | |
| 327 | 327 | assert.Equal(t, domain.DiffStats{FilesChanged: 3, Additions: 4004}, d.Stats) | |
| 328 | 328 | }) | |
| 329 | 329 | ||
| @@ -346,13 +346,13 @@ func TestDiffLimits(t *testing.T) { | |||
| 346 | 346 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, defaults) | |
| 347 | 347 | require.NoError(t, err) | |
| 348 | 348 | big, tiny := find(d.Diff, "big"), find(d.Diff, "tiny") | |
| 349 | - | assert.False(t, big.Truncated) | |
| 349 | + | assert.Equal(t, domain.OmitNone, big.Omitted) | |
| 350 | 350 | require.Len(t, big.Hunks, 1) | |
| 351 | 351 | assert.Len(t, big.Hunks[0].Lines, 3000+429) | |
| 352 | 352 | assert.False(t, d.Diff.Truncated) | |
| 353 | 353 | assert.Equal(t, 429, big.Additions) | |
| 354 | 354 | assert.Equal(t, 429, big.Deletions) | |
| 355 | - | assert.False(t, tiny.Truncated, "the following file still fits the changed-line budget") | |
| 355 | + | assert.Equal(t, domain.OmitNone, tiny.Omitted, "the following file still fits the changed-line budget") | |
| 356 | 356 | assert.Equal(t, "x\n", strings.TrimPrefix(unified(tiny.Hunks), "@@ -0,0 +1 @@\n+")) | |
| 357 | 357 | assert.Equal(t, id, d.Commit.ID.String()) | |
| 358 | 358 | }) | |
| @@ -378,13 +378,13 @@ func TestDiffLimits(t *testing.T) { | |||
| 378 | 378 | } | |
| 379 | 379 | assert.LessOrEqual(t, total, 20000) | |
| 380 | 380 | for i := range 10 { | |
| 381 | - | assert.False(t, find(d.Diff, fmt.Sprintf("f%02d", i)).Truncated) | |
| 381 | + | assert.Equal(t, domain.OmitNone, find(d.Diff, fmt.Sprintf("f%02d", i)).Omitted) | |
| 382 | 382 | } | |
| 383 | - | assert.True(t, find(d.Diff, "f10").Truncated, "1,900 lines do not fit in the 1,000 left") | |
| 384 | - | assert.False(t, find(d.Diff, "g-small").Truncated) | |
| 385 | - | assert.True(t, find(d.Diff, "h-big").Truncated) | |
| 386 | - | assert.False(t, find(d.Diff, "i-small").Truncated) | |
| 387 | - | assert.True(t, d.Diff.Truncated) | |
| 383 | + | assert.NotEqual(t, domain.OmitNone, find(d.Diff, "f10").Omitted, "1,900 lines do not fit in the 1,000 left") | |
| 384 | + | assert.Equal(t, domain.OmitNone, find(d.Diff, "g-small").Omitted) | |
| 385 | + | assert.NotEqual(t, domain.OmitNone, find(d.Diff, "h-big").Omitted) | |
| 386 | + | assert.Equal(t, domain.OmitNone, find(d.Diff, "i-small").Omitted) | |
| 387 | + | assert.False(t, d.Diff.Truncated) | |
| 388 | 388 | assert.Equal(t, 11*1900+50+1500+10, d.Stats.Additions) | |
| 389 | 389 | assert.Equal(t, 14, d.Stats.FilesChanged) | |
| 390 | 390 | }) | |
| @@ -403,7 +403,7 @@ func TestDiffLimits(t *testing.T) { | |||
| 403 | 403 | assert.Equal(t, "many/f000", d.Diff.Files[0].NewPath) | |
| 404 | 404 | assert.Equal(t, "many/f299", d.Diff.Files[299].NewPath) | |
| 405 | 405 | for _, file := range d.Diff.Files { | |
| 406 | - | assert.False(t, file.Truncated) | |
| 406 | + | assert.Equal(t, domain.OmitNone, file.Omitted) | |
| 407 | 407 | assert.Len(t, file.Hunks, 1) | |
| 408 | 408 | } | |
| 409 | 409 | }) | |
| @@ -426,16 +426,16 @@ func TestDiffLimits(t *testing.T) { | |||
| 426 | 426 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxFiles: 2, MaxFileLines: 100, MaxTotalLines: 15, MaxTotalBytes: 1 << 20}) | |
| 427 | 427 | require.NoError(t, err) | |
| 428 | 428 | require.Len(t, d.Diff.Files, 2) | |
| 429 | - | assert.False(t, d.Diff.Files[0].Truncated) | |
| 430 | - | assert.True(t, d.Diff.Files[1].Truncated, "10 lines do not fit in the 5 left") | |
| 429 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted) | |
| 430 | + | assert.NotEqual(t, domain.OmitNone, d.Diff.Files[1].Omitted, "10 lines do not fit in the 5 left") | |
| 431 | 431 | assert.Equal(t, 3, d.Stats.FilesChanged) | |
| 432 | 432 | ||
| 433 | 433 | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxFiles: 10, MaxFileLines: 100, MaxTotalLines: 100, MaxTotalBytes: 50}) | |
| 434 | 434 | require.NoError(t, err) | |
| 435 | 435 | require.Len(t, d.Diff.Files, 3) | |
| 436 | - | assert.False(t, d.Diff.Files[0].Truncated, "30 bytes of text fit in 50") | |
| 436 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted, "30 bytes of text fit in 50") | |
| 437 | 437 | for _, file := range d.Diff.Files[1:] { | |
| 438 | - | assert.True(t, file.Truncated, "the byte budget has 20 bytes left for 30") | |
| 438 | + | assert.NotEqual(t, domain.OmitNone, file.Omitted, "the byte budget has 20 bytes left for 30") | |
| 439 | 439 | assert.Empty(t, file.Hunks) | |
| 440 | 440 | } | |
| 441 | 441 | }) | |
| @@ -446,9 +446,9 @@ func TestDiffLimits(t *testing.T) { | |||
| 446 | 446 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, defaults) | |
| 447 | 447 | require.NoError(t, err) | |
| 448 | 448 | long, short := find(d.Diff, "long"), find(d.Diff, "short") | |
| 449 | - | assert.True(t, long.Truncated) | |
| 449 | + | assert.NotEqual(t, domain.OmitNone, long.Omitted) | |
| 450 | 450 | assert.Empty(t, long.Hunks) | |
| 451 | - | assert.False(t, short.Truncated) | |
| 451 | + | assert.Equal(t, domain.OmitNone, short.Omitted) | |
| 452 | 452 | assert.Len(t, short.Hunks, 1) | |
| 453 | 453 | }) | |
| 454 | 454 | ||
| @@ -460,10 +460,10 @@ func TestDiffLimits(t *testing.T) { | |||
| 460 | 460 | id := f.Commit("v2") | |
| 461 | 461 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, defaults) | |
| 462 | 462 | require.NoError(t, err) | |
| 463 | - | assert.True(t, find(d.Diff, "a-big").Truncated) | |
| 463 | + | assert.NotEqual(t, domain.OmitNone, find(d.Diff, "a-big").Omitted) | |
| 464 | 464 | assert.True(t, find(d.Diff, "b-bin").Binary) | |
| 465 | - | assert.False(t, find(d.Diff, "b-bin").Truncated) | |
| 466 | - | assert.False(t, find(d.Diff, "c-mode").Truncated) | |
| 465 | + | assert.Equal(t, domain.OmitNone, find(d.Diff, "b-bin").Omitted) | |
| 466 | + | assert.Equal(t, domain.OmitNone, find(d.Diff, "c-mode").Omitted) | |
| 467 | 467 | assert.Len(t, find(d.Diff, "d-text").Hunks, 1) | |
| 468 | 468 | // What is kept matches git. | |
| 469 | 469 | sections := showSections(f, id) | |
| @@ -482,8 +482,8 @@ func TestGetCommitStopsGitEarly(t *testing.T) { | |||
| 482 | 482 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DefaultDiffLimits()) | |
| 483 | 483 | require.NoError(t, err) | |
| 484 | 484 | require.Len(t, d.Diff.Files, 2) | |
| 485 | - | assert.False(t, d.Diff.Files[0].Truncated) | |
| 486 | - | assert.True(t, d.Diff.Files[1].Truncated) | |
| 485 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted) | |
| 486 | + | assert.NotEqual(t, domain.OmitNone, d.Diff.Files[1].Omitted) | |
| 487 | 487 | assert.Equal(t, 100005, d.Stats.Additions) | |
| 488 | 488 | } | |
| 489 | 489 | ||
| @@ -582,23 +582,23 @@ func TestDiffLimitsWindow(t *testing.T) { | |||
| 582 | 582 | for i, file := range d.Diff.Files { | |
| 583 | 583 | switch i { | |
| 584 | 584 | case 3: | |
| 585 | - | assert.False(t, file.Truncated) | |
| 585 | + | assert.Equal(t, domain.OmitNone, file.Omitted) | |
| 586 | 586 | assert.Len(t, file.Hunks, 1, "10 lines fit in 15") | |
| 587 | 587 | case 4: | |
| 588 | - | assert.True(t, file.Truncated, "the second file of the window does not fit in the 5 lines left") | |
| 588 | + | assert.NotEqual(t, domain.OmitNone, file.Omitted, "the second file of the window does not fit in the 5 lines left") | |
| 589 | 589 | default: | |
| 590 | - | assert.False(t, file.Truncated) | |
| 590 | + | assert.Equal(t, domain.OmitNone, file.Omitted) | |
| 591 | 591 | assert.Empty(t, file.Hunks) | |
| 592 | 592 | assert.Equal(t, 10, file.Additions, "counts stay accurate") | |
| 593 | 593 | } | |
| 594 | 594 | } | |
| 595 | - | assert.True(t, d.Diff.Truncated, "a file of the window is truncated") | |
| 595 | + | assert.False(t, d.Diff.Truncated, "an omitted file does not set Diff.Truncated") | |
| 596 | 596 | ||
| 597 | 597 | limits.From, limits.To = 0, 0 | |
| 598 | 598 | limits.MaxTotalBytes = 25 // "a 0\n".."a 9\n" is 30 bytes of text per file | |
| 599 | 599 | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits) | |
| 600 | 600 | require.NoError(t, err) | |
| 601 | - | assert.True(t, d.Diff.Files[0].Truncated) | |
| 601 | + | assert.NotEqual(t, domain.OmitNone, d.Diff.Files[0].Omitted) | |
| 602 | 602 | }) | |
| 603 | 603 | ||
| 604 | 604 | t.Run("a file beyond the window is not truncated by its own size", func(t *testing.T) { | |
| @@ -610,14 +610,14 @@ func TestDiffLimitsWindow(t *testing.T) { | |||
| 610 | 610 | require.NoError(t, err) | |
| 611 | 611 | require.Len(t, d.Diff.Files, 2) | |
| 612 | 612 | assert.Len(t, d.Diff.Files[0].Hunks, 1) | |
| 613 | - | assert.False(t, d.Diff.Files[1].Truncated) | |
| 613 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[1].Omitted) | |
| 614 | 614 | assert.Empty(t, d.Diff.Files[1].Hunks) | |
| 615 | 615 | assert.False(t, d.Diff.Truncated) | |
| 616 | 616 | limits.From, limits.To = 1, 2 | |
| 617 | 617 | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits) | |
| 618 | 618 | require.NoError(t, err) | |
| 619 | - | assert.True(t, d.Diff.Files[1].Truncated, "in the window, over the per-file limit") | |
| 620 | - | assert.True(t, d.Diff.Truncated) | |
| 619 | + | assert.NotEqual(t, domain.OmitNone, d.Diff.Files[1].Omitted, "in the window, over the per-file limit") | |
| 620 | + | assert.False(t, d.Diff.Truncated) | |
| 621 | 621 | }) | |
| 622 | 622 | ||
| 623 | 623 | t.Run("MaxFiles 0 lists every file", func(t *testing.T) { | |
| @@ -652,7 +652,7 @@ func TestDiffLimitsWindow(t *testing.T) { | |||
| 652 | 652 | } else { | |
| 653 | 653 | assert.Empty(t, file.Hunks) | |
| 654 | 654 | } | |
| 655 | - | assert.False(t, file.Truncated) | |
| 655 | + | assert.Equal(t, domain.OmitNone, file.Omitted) | |
| 656 | 656 | } | |
| 657 | 657 | ||
| 658 | 658 | // The default limits still cut at 300. | |
| @@ -177,7 +177,7 @@ func TestGetCommitFileMetadata(t *testing.T) { | |||||
| 177 | assert.Equal(t, domain.FileModified, bin.Status) | 177 | assert.Equal(t, domain.FileModified, bin.Status) | ||
| 178 | assert.True(t, bin.Binary) | 178 | assert.True(t, bin.Binary) | ||
| 179 | assert.Empty(t, bin.Hunks) | 179 | assert.Empty(t, bin.Hunks) | ||
| 180 | - | assert.False(t, bin.Truncated) | 180 | + | assert.Equal(t, domain.OmitNone, bin.Omitted) |
| 181 | 181 | ||||
| 182 | mode := files["d/f>d/f"] | 182 | mode := files["d/f>d/f"] | ||
| 183 | assert.Equal(t, domain.FileModified, mode.Status) | 183 | assert.Equal(t, domain.FileModified, mode.Status) | ||
| @@ -185,7 +185,7 @@ func TestGetCommitFileMetadata(t *testing.T) { | |||||
| 185 | assert.Equal(t, uint32(0o100755), mode.NewMode) | 185 | assert.Equal(t, uint32(0o100755), mode.NewMode) | ||
| 186 | assert.Empty(t, mode.Hunks) | 186 | assert.Empty(t, mode.Hunks) | ||
| 187 | assert.False(t, mode.Binary) | 187 | assert.False(t, mode.Binary) | ||
| 188 | - | assert.False(t, mode.Truncated) | 188 | + | assert.Equal(t, domain.OmitNone, mode.Omitted) |
| 189 | 189 | ||||
| 190 | del := files["README.md>"] | 190 | del := files["README.md>"] | ||
| 191 | assert.Equal(t, domain.FileDeleted, del.Status) | 191 | assert.Equal(t, domain.FileDeleted, del.Status) | ||
| @@ -315,15 +315,15 @@ func TestDiffLimits(t *testing.T) { | |||||
| 315 | require.NoError(t, err) | 315 | require.NoError(t, err) | ||
| 316 | exact, over, small := find(d.Diff, "exact"), find(d.Diff, "over"), find(d.Diff, "small") | 316 | exact, over, small := find(d.Diff, "exact"), find(d.Diff, "over"), find(d.Diff, "small") | ||
| 317 | 317 | ||||
| 318 | - | assert.False(t, exact.Truncated) | 318 | + | assert.Equal(t, domain.OmitNone, exact.Omitted) |
| 319 | require.Len(t, exact.Hunks, 1) | 319 | require.Len(t, exact.Hunks, 1) | ||
| 320 | assert.Len(t, exact.Hunks[0].Lines, 2000) | 320 | assert.Len(t, exact.Hunks[0].Lines, 2000) | ||
| 321 | - | assert.True(t, over.Truncated) | 321 | + | assert.NotEqual(t, domain.OmitNone, over.Omitted) |
| 322 | assert.Empty(t, over.Hunks) | 322 | assert.Empty(t, over.Hunks) | ||
| 323 | assert.Equal(t, 2001, over.Additions, "counts stay accurate") | 323 | assert.Equal(t, 2001, over.Additions, "counts stay accurate") | ||
| 324 | - | assert.False(t, small.Truncated) | 324 | + | assert.Equal(t, domain.OmitNone, small.Omitted) |
| 325 | assert.Len(t, small.Hunks, 1) | 325 | assert.Len(t, small.Hunks, 1) | ||
| 326 | - | assert.True(t, d.Diff.Truncated) | 326 | + | assert.False(t, d.Diff.Truncated) |
| 327 | assert.Equal(t, domain.DiffStats{FilesChanged: 3, Additions: 4004}, d.Stats) | 327 | assert.Equal(t, domain.DiffStats{FilesChanged: 3, Additions: 4004}, d.Stats) | ||
| 328 | }) | 328 | }) | ||
| 329 | 329 | ||||
| @@ -346,13 +346,13 @@ func TestDiffLimits(t *testing.T) { | |||||
| 346 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, defaults) | 346 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, defaults) | ||
| 347 | require.NoError(t, err) | 347 | require.NoError(t, err) | ||
| 348 | big, tiny := find(d.Diff, "big"), find(d.Diff, "tiny") | 348 | big, tiny := find(d.Diff, "big"), find(d.Diff, "tiny") | ||
| 349 | - | assert.False(t, big.Truncated) | 349 | + | assert.Equal(t, domain.OmitNone, big.Omitted) |
| 350 | require.Len(t, big.Hunks, 1) | 350 | require.Len(t, big.Hunks, 1) | ||
| 351 | assert.Len(t, big.Hunks[0].Lines, 3000+429) | 351 | assert.Len(t, big.Hunks[0].Lines, 3000+429) | ||
| 352 | assert.False(t, d.Diff.Truncated) | 352 | assert.False(t, d.Diff.Truncated) | ||
| 353 | assert.Equal(t, 429, big.Additions) | 353 | assert.Equal(t, 429, big.Additions) | ||
| 354 | assert.Equal(t, 429, big.Deletions) | 354 | assert.Equal(t, 429, big.Deletions) | ||
| 355 | - | assert.False(t, tiny.Truncated, "the following file still fits the changed-line budget") | 355 | + | assert.Equal(t, domain.OmitNone, tiny.Omitted, "the following file still fits the changed-line budget") |
| 356 | assert.Equal(t, "x\n", strings.TrimPrefix(unified(tiny.Hunks), "@@ -0,0 +1 @@\n+")) | 356 | assert.Equal(t, "x\n", strings.TrimPrefix(unified(tiny.Hunks), "@@ -0,0 +1 @@\n+")) | ||
| 357 | assert.Equal(t, id, d.Commit.ID.String()) | 357 | assert.Equal(t, id, d.Commit.ID.String()) | ||
| 358 | }) | 358 | }) | ||
| @@ -378,13 +378,13 @@ func TestDiffLimits(t *testing.T) { | |||||
| 378 | } | 378 | } | ||
| 379 | assert.LessOrEqual(t, total, 20000) | 379 | assert.LessOrEqual(t, total, 20000) | ||
| 380 | for i := range 10 { | 380 | for i := range 10 { | ||
| 381 | - | assert.False(t, find(d.Diff, fmt.Sprintf("f%02d", i)).Truncated) | 381 | + | assert.Equal(t, domain.OmitNone, find(d.Diff, fmt.Sprintf("f%02d", i)).Omitted) |
| 382 | } | 382 | } | ||
| 383 | - | assert.True(t, find(d.Diff, "f10").Truncated, "1,900 lines do not fit in the 1,000 left") | 383 | + | assert.NotEqual(t, domain.OmitNone, find(d.Diff, "f10").Omitted, "1,900 lines do not fit in the 1,000 left") |
| 384 | - | assert.False(t, find(d.Diff, "g-small").Truncated) | 384 | + | assert.Equal(t, domain.OmitNone, find(d.Diff, "g-small").Omitted) |
| 385 | - | assert.True(t, find(d.Diff, "h-big").Truncated) | 385 | + | assert.NotEqual(t, domain.OmitNone, find(d.Diff, "h-big").Omitted) |
| 386 | - | assert.False(t, find(d.Diff, "i-small").Truncated) | 386 | + | assert.Equal(t, domain.OmitNone, find(d.Diff, "i-small").Omitted) |
| 387 | - | assert.True(t, d.Diff.Truncated) | 387 | + | assert.False(t, d.Diff.Truncated) |
| 388 | assert.Equal(t, 11*1900+50+1500+10, d.Stats.Additions) | 388 | assert.Equal(t, 11*1900+50+1500+10, d.Stats.Additions) | ||
| 389 | assert.Equal(t, 14, d.Stats.FilesChanged) | 389 | assert.Equal(t, 14, d.Stats.FilesChanged) | ||
| 390 | }) | 390 | }) | ||
| @@ -403,7 +403,7 @@ func TestDiffLimits(t *testing.T) { | |||||
| 403 | assert.Equal(t, "many/f000", d.Diff.Files[0].NewPath) | 403 | assert.Equal(t, "many/f000", d.Diff.Files[0].NewPath) | ||
| 404 | assert.Equal(t, "many/f299", d.Diff.Files[299].NewPath) | 404 | assert.Equal(t, "many/f299", d.Diff.Files[299].NewPath) | ||
| 405 | for _, file := range d.Diff.Files { | 405 | for _, file := range d.Diff.Files { | ||
| 406 | - | assert.False(t, file.Truncated) | 406 | + | assert.Equal(t, domain.OmitNone, file.Omitted) |
| 407 | assert.Len(t, file.Hunks, 1) | 407 | assert.Len(t, file.Hunks, 1) | ||
| 408 | } | 408 | } | ||
| 409 | }) | 409 | }) | ||
| @@ -426,16 +426,16 @@ func TestDiffLimits(t *testing.T) { | |||||
| 426 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxFiles: 2, MaxFileLines: 100, MaxTotalLines: 15, MaxTotalBytes: 1 << 20}) | 426 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxFiles: 2, MaxFileLines: 100, MaxTotalLines: 15, MaxTotalBytes: 1 << 20}) | ||
| 427 | require.NoError(t, err) | 427 | require.NoError(t, err) | ||
| 428 | require.Len(t, d.Diff.Files, 2) | 428 | require.Len(t, d.Diff.Files, 2) | ||
| 429 | - | assert.False(t, d.Diff.Files[0].Truncated) | 429 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted) |
| 430 | - | assert.True(t, d.Diff.Files[1].Truncated, "10 lines do not fit in the 5 left") | 430 | + | assert.NotEqual(t, domain.OmitNone, d.Diff.Files[1].Omitted, "10 lines do not fit in the 5 left") |
| 431 | assert.Equal(t, 3, d.Stats.FilesChanged) | 431 | assert.Equal(t, 3, d.Stats.FilesChanged) | ||
| 432 | 432 | ||||
| 433 | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxFiles: 10, MaxFileLines: 100, MaxTotalLines: 100, MaxTotalBytes: 50}) | 433 | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxFiles: 10, MaxFileLines: 100, MaxTotalLines: 100, MaxTotalBytes: 50}) | ||
| 434 | require.NoError(t, err) | 434 | require.NoError(t, err) | ||
| 435 | require.Len(t, d.Diff.Files, 3) | 435 | require.Len(t, d.Diff.Files, 3) | ||
| 436 | - | assert.False(t, d.Diff.Files[0].Truncated, "30 bytes of text fit in 50") | 436 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted, "30 bytes of text fit in 50") |
| 437 | for _, file := range d.Diff.Files[1:] { | 437 | for _, file := range d.Diff.Files[1:] { | ||
| 438 | - | assert.True(t, file.Truncated, "the byte budget has 20 bytes left for 30") | 438 | + | assert.NotEqual(t, domain.OmitNone, file.Omitted, "the byte budget has 20 bytes left for 30") |
| 439 | assert.Empty(t, file.Hunks) | 439 | assert.Empty(t, file.Hunks) | ||
| 440 | } | 440 | } | ||
| 441 | }) | 441 | }) | ||
| @@ -446,9 +446,9 @@ func TestDiffLimits(t *testing.T) { | |||||
| 446 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, defaults) | 446 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, defaults) | ||
| 447 | require.NoError(t, err) | 447 | require.NoError(t, err) | ||
| 448 | long, short := find(d.Diff, "long"), find(d.Diff, "short") | 448 | long, short := find(d.Diff, "long"), find(d.Diff, "short") | ||
| 449 | - | assert.True(t, long.Truncated) | 449 | + | assert.NotEqual(t, domain.OmitNone, long.Omitted) |
| 450 | assert.Empty(t, long.Hunks) | 450 | assert.Empty(t, long.Hunks) | ||
| 451 | - | assert.False(t, short.Truncated) | 451 | + | assert.Equal(t, domain.OmitNone, short.Omitted) |
| 452 | assert.Len(t, short.Hunks, 1) | 452 | assert.Len(t, short.Hunks, 1) | ||
| 453 | }) | 453 | }) | ||
| 454 | 454 | ||||
| @@ -460,10 +460,10 @@ func TestDiffLimits(t *testing.T) { | |||||
| 460 | id := f.Commit("v2") | 460 | id := f.Commit("v2") | ||
| 461 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, defaults) | 461 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, defaults) | ||
| 462 | require.NoError(t, err) | 462 | require.NoError(t, err) | ||
| 463 | - | assert.True(t, find(d.Diff, "a-big").Truncated) | 463 | + | assert.NotEqual(t, domain.OmitNone, find(d.Diff, "a-big").Omitted) |
| 464 | assert.True(t, find(d.Diff, "b-bin").Binary) | 464 | assert.True(t, find(d.Diff, "b-bin").Binary) | ||
| 465 | - | assert.False(t, find(d.Diff, "b-bin").Truncated) | 465 | + | assert.Equal(t, domain.OmitNone, find(d.Diff, "b-bin").Omitted) |
| 466 | - | assert.False(t, find(d.Diff, "c-mode").Truncated) | 466 | + | assert.Equal(t, domain.OmitNone, find(d.Diff, "c-mode").Omitted) |
| 467 | assert.Len(t, find(d.Diff, "d-text").Hunks, 1) | 467 | assert.Len(t, find(d.Diff, "d-text").Hunks, 1) | ||
| 468 | // What is kept matches git. | 468 | // What is kept matches git. | ||
| 469 | sections := showSections(f, id) | 469 | sections := showSections(f, id) | ||
| @@ -482,8 +482,8 @@ func TestGetCommitStopsGitEarly(t *testing.T) { | |||||
| 482 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DefaultDiffLimits()) | 482 | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DefaultDiffLimits()) | ||
| 483 | require.NoError(t, err) | 483 | require.NoError(t, err) | ||
| 484 | require.Len(t, d.Diff.Files, 2) | 484 | require.Len(t, d.Diff.Files, 2) | ||
| 485 | - | assert.False(t, d.Diff.Files[0].Truncated) | 485 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted) |
| 486 | - | assert.True(t, d.Diff.Files[1].Truncated) | 486 | + | assert.NotEqual(t, domain.OmitNone, d.Diff.Files[1].Omitted) |
| 487 | assert.Equal(t, 100005, d.Stats.Additions) | 487 | assert.Equal(t, 100005, d.Stats.Additions) | ||
| 488 | } | 488 | } | ||
| 489 | 489 | ||||
| @@ -582,23 +582,23 @@ func TestDiffLimitsWindow(t *testing.T) { | |||||
| 582 | for i, file := range d.Diff.Files { | 582 | for i, file := range d.Diff.Files { | ||
| 583 | switch i { | 583 | switch i { | ||
| 584 | case 3: | 584 | case 3: | ||
| 585 | - | assert.False(t, file.Truncated) | 585 | + | assert.Equal(t, domain.OmitNone, file.Omitted) |
| 586 | assert.Len(t, file.Hunks, 1, "10 lines fit in 15") | 586 | assert.Len(t, file.Hunks, 1, "10 lines fit in 15") | ||
| 587 | case 4: | 587 | case 4: | ||
| 588 | - | assert.True(t, file.Truncated, "the second file of the window does not fit in the 5 lines left") | 588 | + | assert.NotEqual(t, domain.OmitNone, file.Omitted, "the second file of the window does not fit in the 5 lines left") |
| 589 | default: | 589 | default: | ||
| 590 | - | assert.False(t, file.Truncated) | 590 | + | assert.Equal(t, domain.OmitNone, file.Omitted) |
| 591 | assert.Empty(t, file.Hunks) | 591 | assert.Empty(t, file.Hunks) | ||
| 592 | assert.Equal(t, 10, file.Additions, "counts stay accurate") | 592 | assert.Equal(t, 10, file.Additions, "counts stay accurate") | ||
| 593 | } | 593 | } | ||
| 594 | } | 594 | } | ||
| 595 | - | assert.True(t, d.Diff.Truncated, "a file of the window is truncated") | 595 | + | assert.False(t, d.Diff.Truncated, "an omitted file does not set Diff.Truncated") |
| 596 | 596 | ||||
| 597 | limits.From, limits.To = 0, 0 | 597 | limits.From, limits.To = 0, 0 | ||
| 598 | limits.MaxTotalBytes = 25 // "a 0\n".."a 9\n" is 30 bytes of text per file | 598 | limits.MaxTotalBytes = 25 // "a 0\n".."a 9\n" is 30 bytes of text per file | ||
| 599 | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits) | 599 | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits) | ||
| 600 | require.NoError(t, err) | 600 | require.NoError(t, err) | ||
| 601 | - | assert.True(t, d.Diff.Files[0].Truncated) | 601 | + | assert.NotEqual(t, domain.OmitNone, d.Diff.Files[0].Omitted) |
| 602 | }) | 602 | }) | ||
| 603 | 603 | ||||
| 604 | t.Run("a file beyond the window is not truncated by its own size", func(t *testing.T) { | 604 | t.Run("a file beyond the window is not truncated by its own size", func(t *testing.T) { | ||
| @@ -610,14 +610,14 @@ func TestDiffLimitsWindow(t *testing.T) { | |||||
| 610 | require.NoError(t, err) | 610 | require.NoError(t, err) | ||
| 611 | require.Len(t, d.Diff.Files, 2) | 611 | require.Len(t, d.Diff.Files, 2) | ||
| 612 | assert.Len(t, d.Diff.Files[0].Hunks, 1) | 612 | assert.Len(t, d.Diff.Files[0].Hunks, 1) | ||
| 613 | - | assert.False(t, d.Diff.Files[1].Truncated) | 613 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[1].Omitted) |
| 614 | assert.Empty(t, d.Diff.Files[1].Hunks) | 614 | assert.Empty(t, d.Diff.Files[1].Hunks) | ||
| 615 | assert.False(t, d.Diff.Truncated) | 615 | assert.False(t, d.Diff.Truncated) | ||
| 616 | limits.From, limits.To = 1, 2 | 616 | limits.From, limits.To = 1, 2 | ||
| 617 | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits) | 617 | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits) | ||
| 618 | require.NoError(t, err) | 618 | require.NoError(t, err) | ||
| 619 | - | assert.True(t, d.Diff.Files[1].Truncated, "in the window, over the per-file limit") | 619 | + | assert.NotEqual(t, domain.OmitNone, d.Diff.Files[1].Omitted, "in the window, over the per-file limit") |
| 620 | - | assert.True(t, d.Diff.Truncated) | 620 | + | assert.False(t, d.Diff.Truncated) |
| 621 | }) | 621 | }) | ||
| 622 | 622 | ||||
| 623 | t.Run("MaxFiles 0 lists every file", func(t *testing.T) { | 623 | t.Run("MaxFiles 0 lists every file", func(t *testing.T) { | ||
| @@ -652,7 +652,7 @@ func TestDiffLimitsWindow(t *testing.T) { | |||||
| 652 | } else { | 652 | } else { | ||
| 653 | assert.Empty(t, file.Hunks) | 653 | assert.Empty(t, file.Hunks) | ||
| 654 | } | 654 | } | ||
| 655 | - | assert.False(t, file.Truncated) | 655 | + | assert.Equal(t, domain.OmitNone, file.Omitted) |
| 656 | } | 656 | } | ||
| 657 | 657 | ||||
| 658 | // The default limits still cut at 300. | 658 | // The default limits still cut at 300. | ||
M internal/adapters/rpc/convert.go +17 -1
| @@ -394,7 +394,8 @@ func fileDiffToPB(f domain.FileDiff) *pb.FileDiff { | |||
| 394 | 394 | Additions: int32(f.Additions), | |
| 395 | 395 | Deletions: int32(f.Deletions), | |
| 396 | 396 | Similarity: int32(f.Similarity), | |
| 397 | - | Truncated: f.Truncated, | |
| 397 | + | Truncated: f.Omitted != domain.OmitNone, | |
| 398 | + | Omitted: fileOmissionToPB(f.Omitted), | |
| 398 | 399 | } | |
| 399 | 400 | for _, h := range f.Hunks { | |
| 400 | 401 | out.Hunks = append(out.Hunks, hunkToPB(h)) | |
| @@ -402,6 +403,20 @@ func fileDiffToPB(f domain.FileDiff) *pb.FileDiff { | |||
| 402 | 403 | return out | |
| 403 | 404 | } | |
| 404 | 405 | ||
| 406 | + | func fileOmissionToPB(o domain.FileOmission) pb.FileOmission { | |
| 407 | + | switch o { | |
| 408 | + | case domain.OmitGenerated: | |
| 409 | + | return pb.FileOmission_FILE_OMISSION_GENERATED | |
| 410 | + | case domain.OmitLarge: | |
| 411 | + | return pb.FileOmission_FILE_OMISSION_LARGE | |
| 412 | + | case domain.OmitBudget: | |
| 413 | + | return pb.FileOmission_FILE_OMISSION_BUDGET | |
| 414 | + | case domain.OmitUnavailable: | |
| 415 | + | return pb.FileOmission_FILE_OMISSION_UNAVAILABLE | |
| 416 | + | } | |
| 417 | + | return pb.FileOmission_FILE_OMISSION_UNSPECIFIED | |
| 418 | + | } | |
| 419 | + | ||
| 405 | 420 | func diffStatsToPB(s domain.DiffStats) *pb.DiffStats { | |
| 406 | 421 | return &pb.DiffStats{ | |
| 407 | 422 | FilesChanged: int32(s.FilesChanged), | |
| @@ -413,6 +428,7 @@ func diffStatsToPB(s domain.DiffStats) *pb.DiffStats { | |||
| 413 | 428 | func diffToPB(d domain.Diff) *pb.Diff { | |
| 414 | 429 | out := &pb.Diff{Truncated: d.Truncated} | |
| 415 | 430 | for _, f := range d.Files { | |
| 431 | + | out.Truncated = out.Truncated || f.Omitted != domain.OmitNone | |
| 416 | 432 | out.Files = append(out.Files, fileDiffToPB(f)) | |
| 417 | 433 | } | |
| 418 | 434 | return out | |
| @@ -394,7 +394,8 @@ func fileDiffToPB(f domain.FileDiff) *pb.FileDiff { | |||||
| 394 | Additions: int32(f.Additions), | 394 | Additions: int32(f.Additions), | ||
| 395 | Deletions: int32(f.Deletions), | 395 | Deletions: int32(f.Deletions), | ||
| 396 | Similarity: int32(f.Similarity), | 396 | Similarity: int32(f.Similarity), | ||
| 397 | - | Truncated: f.Truncated, | 397 | + | Truncated: f.Omitted != domain.OmitNone, |
| 398 | + | Omitted: fileOmissionToPB(f.Omitted), | |||
| 398 | } | 399 | } | ||
| 399 | for _, h := range f.Hunks { | 400 | for _, h := range f.Hunks { | ||
| 400 | out.Hunks = append(out.Hunks, hunkToPB(h)) | 401 | out.Hunks = append(out.Hunks, hunkToPB(h)) | ||
| @@ -402,6 +403,20 @@ func fileDiffToPB(f domain.FileDiff) *pb.FileDiff { | |||||
| 402 | return out | 403 | return out | ||
| 403 | } | 404 | } | ||
| 404 | 405 | ||||
| 406 | + | func fileOmissionToPB(o domain.FileOmission) pb.FileOmission { | |||
| 407 | + | switch o { | |||
| 408 | + | case domain.OmitGenerated: | |||
| 409 | + | return pb.FileOmission_FILE_OMISSION_GENERATED | |||
| 410 | + | case domain.OmitLarge: | |||
| 411 | + | return pb.FileOmission_FILE_OMISSION_LARGE | |||
| 412 | + | case domain.OmitBudget: | |||
| 413 | + | return pb.FileOmission_FILE_OMISSION_BUDGET | |||
| 414 | + | case domain.OmitUnavailable: | |||
| 415 | + | return pb.FileOmission_FILE_OMISSION_UNAVAILABLE | |||
| 416 | + | } | |||
| 417 | + | return pb.FileOmission_FILE_OMISSION_UNSPECIFIED | |||
| 418 | + | } | |||
| 419 | + | ||||
| 405 | func diffStatsToPB(s domain.DiffStats) *pb.DiffStats { | 420 | func diffStatsToPB(s domain.DiffStats) *pb.DiffStats { | ||
| 406 | return &pb.DiffStats{ | 421 | return &pb.DiffStats{ | ||
| 407 | FilesChanged: int32(s.FilesChanged), | 422 | FilesChanged: int32(s.FilesChanged), | ||
| @@ -413,6 +428,7 @@ func diffStatsToPB(s domain.DiffStats) *pb.DiffStats { | |||||
| 413 | func diffToPB(d domain.Diff) *pb.Diff { | 428 | func diffToPB(d domain.Diff) *pb.Diff { | ||
| 414 | out := &pb.Diff{Truncated: d.Truncated} | 429 | out := &pb.Diff{Truncated: d.Truncated} | ||
| 415 | for _, f := range d.Files { | 430 | for _, f := range d.Files { | ||
| 431 | + | out.Truncated = out.Truncated || f.Omitted != domain.OmitNone | |||
| 416 | out.Files = append(out.Files, fileDiffToPB(f)) | 432 | out.Files = append(out.Files, fileDiffToPB(f)) | ||
| 417 | } | 433 | } | ||
| 418 | return out | 434 | return out | ||
M internal/adapters/rpc/convert_test.go +18 -0
| @@ -398,3 +398,21 @@ func TestMergeRequestToPBMergeabilityOnlyWhileOpen(t *testing.T) { | |||
| 398 | 398 | assert.Nil(t, got.GetMergeRequestClosed().GetMergeRequest().GetMergeability()) | |
| 399 | 399 | } | |
| 400 | 400 | } | |
| 401 | + | ||
| 402 | + | func TestFileDiffOmissionMapping(t *testing.T) { | |
| 403 | + | for o, want := range map[domain.FileOmission]pb.FileOmission{ | |
| 404 | + | domain.OmitNone: pb.FileOmission_FILE_OMISSION_UNSPECIFIED, | |
| 405 | + | domain.OmitGenerated: pb.FileOmission_FILE_OMISSION_GENERATED, | |
| 406 | + | domain.OmitLarge: pb.FileOmission_FILE_OMISSION_LARGE, | |
| 407 | + | domain.OmitBudget: pb.FileOmission_FILE_OMISSION_BUDGET, | |
| 408 | + | domain.OmitUnavailable: pb.FileOmission_FILE_OMISSION_UNAVAILABLE, | |
| 409 | + | } { | |
| 410 | + | f := fileDiffToPB(domain.FileDiff{NewPath: "a", Omitted: o}) | |
| 411 | + | assert.Equal(t, want, f.GetOmitted()) | |
| 412 | + | assert.Equal(t, o != domain.OmitNone, f.GetTruncated()) | |
| 413 | + | d := diffToPB(domain.Diff{Files: []domain.FileDiff{{Omitted: o}}}) | |
| 414 | + | assert.Equal(t, o != domain.OmitNone, d.GetTruncated()) | |
| 415 | + | } | |
| 416 | + | assert.True(t, diffToPB(domain.Diff{Truncated: true}).GetTruncated()) | |
| 417 | + | assert.False(t, diffToPB(domain.Diff{Files: []domain.FileDiff{{}}}).GetTruncated()) | |
| 418 | + | } | |
| @@ -398,3 +398,21 @@ func TestMergeRequestToPBMergeabilityOnlyWhileOpen(t *testing.T) { | |||||
| 398 | assert.Nil(t, got.GetMergeRequestClosed().GetMergeRequest().GetMergeability()) | 398 | assert.Nil(t, got.GetMergeRequestClosed().GetMergeRequest().GetMergeability()) | ||
| 399 | } | 399 | } | ||
| 400 | } | 400 | } | ||
| 401 | + | ||||
| 402 | + | func TestFileDiffOmissionMapping(t *testing.T) { | |||
| 403 | + | for o, want := range map[domain.FileOmission]pb.FileOmission{ | |||
| 404 | + | domain.OmitNone: pb.FileOmission_FILE_OMISSION_UNSPECIFIED, | |||
| 405 | + | domain.OmitGenerated: pb.FileOmission_FILE_OMISSION_GENERATED, | |||
| 406 | + | domain.OmitLarge: pb.FileOmission_FILE_OMISSION_LARGE, | |||
| 407 | + | domain.OmitBudget: pb.FileOmission_FILE_OMISSION_BUDGET, | |||
| 408 | + | domain.OmitUnavailable: pb.FileOmission_FILE_OMISSION_UNAVAILABLE, | |||
| 409 | + | } { | |||
| 410 | + | f := fileDiffToPB(domain.FileDiff{NewPath: "a", Omitted: o}) | |||
| 411 | + | assert.Equal(t, want, f.GetOmitted()) | |||
| 412 | + | assert.Equal(t, o != domain.OmitNone, f.GetTruncated()) | |||
| 413 | + | d := diffToPB(domain.Diff{Files: []domain.FileDiff{{Omitted: o}}}) | |||
| 414 | + | assert.Equal(t, o != domain.OmitNone, d.GetTruncated()) | |||
| 415 | + | } | |||
| 416 | + | assert.True(t, diffToPB(domain.Diff{Truncated: true}).GetTruncated()) | |||
| 417 | + | assert.False(t, diffToPB(domain.Diff{Files: []domain.FileDiff{{}}}).GetTruncated()) | |||
| 418 | + | } | |||
M internal/adapters/web/commit.go +7 -7
| @@ -188,19 +188,19 @@ func commitBody(c domain.Commit) string { | |||
| 188 | 188 | ||
| 189 | 189 | // diffNote says what is missing from an incomplete diff, "" for a complete one. | |
| 190 | 190 | func diffNote(d domain.Diff, shown, total int) string { | |
| 191 | - | if !d.Truncated { | |
| 191 | + | cut := 0 | |
| 192 | + | for _, f := range d.Files { | |
| 193 | + | if f.Omitted != domain.OmitNone { | |
| 194 | + | cut++ | |
| 195 | + | } | |
| 196 | + | } | |
| 197 | + | if !d.Truncated && cut == 0 { | |
| 192 | 198 | return "" | |
| 193 | 199 | } | |
| 194 | 200 | var parts []string | |
| 195 | 201 | if shown < total { | |
| 196 | 202 | parts = append(parts, plural(total-shown, "file is", "files are")+" not listed") | |
| 197 | 203 | } | |
| 198 | - | cut := 0 | |
| 199 | - | for _, f := range d.Files { | |
| 200 | - | if f.Truncated { | |
| 201 | - | cut++ | |
| 202 | - | } | |
| 203 | - | } | |
| 204 | 204 | if cut > 0 { | |
| 205 | 205 | parts = append(parts, plural(cut, "file is", "files are")+" too large to show") | |
| 206 | 206 | } | |
| @@ -188,19 +188,19 @@ func commitBody(c domain.Commit) string { | |||||
| 188 | 188 | ||||
| 189 | // diffNote says what is missing from an incomplete diff, "" for a complete one. | 189 | // diffNote says what is missing from an incomplete diff, "" for a complete one. | ||
| 190 | func diffNote(d domain.Diff, shown, total int) string { | 190 | func diffNote(d domain.Diff, shown, total int) string { | ||
| 191 | - | if !d.Truncated { | 191 | + | cut := 0 |
| 192 | + | for _, f := range d.Files { | |||
| 193 | + | if f.Omitted != domain.OmitNone { | |||
| 194 | + | cut++ | |||
| 195 | + | } | |||
| 196 | + | } | |||
| 197 | + | if !d.Truncated && cut == 0 { | |||
| 192 | return "" | 198 | return "" | ||
| 193 | } | 199 | } | ||
| 194 | var parts []string | 200 | var parts []string | ||
| 195 | if shown < total { | 201 | if shown < total { | ||
| 196 | parts = append(parts, plural(total-shown, "file is", "files are")+" not listed") | 202 | parts = append(parts, plural(total-shown, "file is", "files are")+" not listed") | ||
| 197 | } | 203 | } | ||
| 198 | - | cut := 0 | |||
| 199 | - | for _, f := range d.Files { | |||
| 200 | - | if f.Truncated { | |||
| 201 | - | cut++ | |||
| 202 | - | } | |||
| 203 | - | } | |||
| 204 | if cut > 0 { | 204 | if cut > 0 { | ||
| 205 | parts = append(parts, plural(cut, "file is", "files are")+" too large to show") | 205 | parts = append(parts, plural(cut, "file is", "files are")+" too large to show") | ||
| 206 | } | 206 | } | ||
M internal/adapters/web/diff.go +3 -3
| @@ -163,7 +163,7 @@ const diffContextLines = 3 | |||
| 163 | 163 | // them: an added or a deleted file shows every line already. | |
| 164 | 164 | func contextGaps(out []DiffFile, files []domain.FileDiff, base string) { | |
| 165 | 165 | for i, f := range files { | |
| 166 | - | if i >= len(out) || f.Binary || f.Truncated || len(f.Hunks) == 0 || len(out[i].Hunks) != len(f.Hunks) { | |
| 166 | + | if i >= len(out) || f.Binary || f.Omitted != domain.OmitNone || len(f.Hunks) == 0 || len(out[i].Hunks) != len(f.Hunks) { | |
| 167 | 167 | continue | |
| 168 | 168 | } | |
| 169 | 169 | switch f.Status { | |
| @@ -371,7 +371,7 @@ func diffFiles(files []domain.FileDiff) []DiffFile { | |||
| 371 | 371 | Add: f.Additions, Del: f.Deletions, Bin: f.Binary, | |
| 372 | 372 | // A file that is shown in full opens; a file with nothing to read | |
| 373 | 373 | // (binary, truncated) stays closed under its note. | |
| 374 | - | Open: len(f.Hunks) > 0 && !f.Truncated, | |
| 374 | + | Open: len(f.Hunks) > 0 && f.Omitted == domain.OmitNone, | |
| 375 | 375 | } | |
| 376 | 376 | df.Key = f.NewPath | |
| 377 | 377 | switch f.Status { | |
| @@ -406,7 +406,7 @@ func fileNote(f domain.FileDiff) string { | |||
| 406 | 406 | switch { | |
| 407 | 407 | case f.Binary: | |
| 408 | 408 | parts = append(parts, "binary file: no text diff") | |
| 409 | - | case f.Truncated: | |
| 409 | + | case f.Omitted != domain.OmitNone: | |
| 410 | 410 | parts = append(parts, fmt.Sprintf("diff too large to show (%d lines changed): collapsed", f.Additions+f.Deletions)) | |
| 411 | 411 | } | |
| 412 | 412 | if f.OldMode != 0 && f.NewMode != 0 && f.OldMode != f.NewMode { | |
| @@ -163,7 +163,7 @@ const diffContextLines = 3 | |||||
| 163 | // them: an added or a deleted file shows every line already. | 163 | // them: an added or a deleted file shows every line already. | ||
| 164 | func contextGaps(out []DiffFile, files []domain.FileDiff, base string) { | 164 | func contextGaps(out []DiffFile, files []domain.FileDiff, base string) { | ||
| 165 | for i, f := range files { | 165 | for i, f := range files { | ||
| 166 | - | if i >= len(out) || f.Binary || f.Truncated || len(f.Hunks) == 0 || len(out[i].Hunks) != len(f.Hunks) { | 166 | + | if i >= len(out) || f.Binary || f.Omitted != domain.OmitNone || len(f.Hunks) == 0 || len(out[i].Hunks) != len(f.Hunks) { |
| 167 | continue | 167 | continue | ||
| 168 | } | 168 | } | ||
| 169 | switch f.Status { | 169 | switch f.Status { | ||
| @@ -371,7 +371,7 @@ func diffFiles(files []domain.FileDiff) []DiffFile { | |||||
| 371 | Add: f.Additions, Del: f.Deletions, Bin: f.Binary, | 371 | Add: f.Additions, Del: f.Deletions, Bin: f.Binary, | ||
| 372 | // A file that is shown in full opens; a file with nothing to read | 372 | // A file that is shown in full opens; a file with nothing to read | ||
| 373 | // (binary, truncated) stays closed under its note. | 373 | // (binary, truncated) stays closed under its note. | ||
| 374 | - | Open: len(f.Hunks) > 0 && !f.Truncated, | 374 | + | Open: len(f.Hunks) > 0 && f.Omitted == domain.OmitNone, |
| 375 | } | 375 | } | ||
| 376 | df.Key = f.NewPath | 376 | df.Key = f.NewPath | ||
| 377 | switch f.Status { | 377 | switch f.Status { | ||
| @@ -406,7 +406,7 @@ func fileNote(f domain.FileDiff) string { | |||||
| 406 | switch { | 406 | switch { | ||
| 407 | case f.Binary: | 407 | case f.Binary: | ||
| 408 | parts = append(parts, "binary file: no text diff") | 408 | parts = append(parts, "binary file: no text diff") | ||
| 409 | - | case f.Truncated: | 409 | + | case f.Omitted != domain.OmitNone: |
| 410 | parts = append(parts, fmt.Sprintf("diff too large to show (%d lines changed): collapsed", f.Additions+f.Deletions)) | 410 | parts = append(parts, fmt.Sprintf("diff too large to show (%d lines changed): collapsed", f.Additions+f.Deletions)) | ||
| 411 | } | 411 | } | ||
| 412 | if f.OldMode != 0 && f.NewMode != 0 && f.OldMode != f.NewMode { | 412 | if f.OldMode != 0 && f.NewMode != 0 && f.OldMode != f.NewMode { | ||
M internal/adapters/web/diff_highlight.go +1 -1
| @@ -446,7 +446,7 @@ func (c *hlCache) files(ctx context.Context, files []domain.FileDiff) []DiffFile | |||
| 446 | 446 | ||
| 447 | 447 | left := maxDiffHLTotalBytes | |
| 448 | 448 | for i, f := range files { | |
| 449 | - | if i >= len(out) || f.Binary || f.Truncated || len(f.Hunks) == 0 || len(out[i].Hunks) != len(f.Hunks) { | |
| 449 | + | if i >= len(out) || f.Binary || f.Omitted != domain.OmitNone || len(f.Hunks) == 0 || len(out[i].Hunks) != len(f.Hunks) { | |
| 450 | 450 | continue | |
| 451 | 451 | } | |
| 452 | 452 | if ctx.Err() != nil { | |
| @@ -446,7 +446,7 @@ func (c *hlCache) files(ctx context.Context, files []domain.FileDiff) []DiffFile | |||||
| 446 | 446 | ||||
| 447 | left := maxDiffHLTotalBytes | 447 | left := maxDiffHLTotalBytes | ||
| 448 | for i, f := range files { | 448 | for i, f := range files { | ||
| 449 | - | if i >= len(out) || f.Binary || f.Truncated || len(f.Hunks) == 0 || len(out[i].Hunks) != len(f.Hunks) { | 449 | + | if i >= len(out) || f.Binary || f.Omitted != domain.OmitNone || len(f.Hunks) == 0 || len(out[i].Hunks) != len(f.Hunks) { |
| 450 | continue | 450 | continue | ||
| 451 | } | 451 | } | ||
| 452 | if ctx.Err() != nil { | 452 | if ctx.Err() != nil { | ||
M internal/adapters/web/diff_highlight_test.go +1 -1
| @@ -162,7 +162,7 @@ func TestHighlightSkips(t *testing.T) { | |||
| 162 | 162 | binary := hunkOf("a.go", "+package a") | |
| 163 | 163 | binary.Binary = true | |
| 164 | 164 | truncated := hunkOf("a.go", "+package a") | |
| 165 | - | truncated.Truncated = true | |
| 165 | + | truncated.Omitted = domain.OmitLarge | |
| 166 | 166 | for name, fd := range map[string]domain.FileDiff{ | |
| 167 | 167 | "no extension": hunkOf("LICENSE.unknownext", "+package a", "+func f() {}"), | |
| 168 | 168 | "plain text": hunkOf("notes.txt", "+package a"), | |
| @@ -162,7 +162,7 @@ func TestHighlightSkips(t *testing.T) { | |||||
| 162 | binary := hunkOf("a.go", "+package a") | 162 | binary := hunkOf("a.go", "+package a") | ||
| 163 | binary.Binary = true | 163 | binary.Binary = true | ||
| 164 | truncated := hunkOf("a.go", "+package a") | 164 | truncated := hunkOf("a.go", "+package a") | ||
| 165 | - | truncated.Truncated = true | 165 | + | truncated.Omitted = domain.OmitLarge |
| 166 | for name, fd := range map[string]domain.FileDiff{ | 166 | for name, fd := range map[string]domain.FileDiff{ | ||
| 167 | "no extension": hunkOf("LICENSE.unknownext", "+package a", "+func f() {}"), | 167 | "no extension": hunkOf("LICENSE.unknownext", "+package a", "+func f() {}"), | ||
| 168 | "plain text": hunkOf("notes.txt", "+package a"), | 168 | "plain text": hunkOf("notes.txt", "+package a"), | ||
M internal/adapters/web/diff_test.go +2 -2
| @@ -99,7 +99,7 @@ func TestFileNotes(t *testing.T) { | |||
| 99 | 99 | {"rename", domain.FileDiff{Status: domain.FileRenamed, Similarity: 90, OldMode: 0o100644, NewMode: 0o100644}, "renamed, 90% similar", true}, | |
| 100 | 100 | {"copy", domain.FileDiff{Status: domain.FileCopied, Similarity: 80}, "copied, 80% similar", true}, | |
| 101 | 101 | {"type change", domain.FileDiff{Status: domain.FileTypeChanged}, "type changed", true}, | |
| 102 | - | {"truncated", domain.FileDiff{Truncated: true, Additions: 2500}, "too large", false}, | |
| 102 | + | {"truncated", domain.FileDiff{Omitted: domain.OmitLarge, Additions: 2500}, "too large", false}, | |
| 103 | 103 | } | |
| 104 | 104 | for _, tt := range tests { | |
| 105 | 105 | t.Run(tt.name, func(t *testing.T) { | |
| @@ -120,7 +120,7 @@ func TestDiffFilesPaths(t *testing.T) { | |||
| 120 | 120 | {Status: domain.FileAdded, NewPath: "a.go", Additions: 1, Hunks: []domain.Hunk{{Lines: []domain.Line{line(domain.LineAddition, 0, 1, "x")}}}}, | |
| 121 | 121 | {Status: domain.FileDeleted, OldPath: "b.go", Deletions: 1}, | |
| 122 | 122 | {Status: domain.FileRenamed, OldPath: "c\u202e.go", NewPath: "d.go"}, | |
| 123 | - | {Status: domain.FileModified, NewPath: "big.go", Truncated: true, Additions: 3000}, | |
| 123 | + | {Status: domain.FileModified, NewPath: "big.go", Omitted: domain.OmitLarge, Additions: 3000}, | |
| 124 | 124 | }) | |
| 125 | 125 | assert.Equal(t, "a.go", fs[0].Path) | |
| 126 | 126 | assert.True(t, fs[0].Open) | |
| @@ -99,7 +99,7 @@ func TestFileNotes(t *testing.T) { | |||||
| 99 | {"rename", domain.FileDiff{Status: domain.FileRenamed, Similarity: 90, OldMode: 0o100644, NewMode: 0o100644}, "renamed, 90% similar", true}, | 99 | {"rename", domain.FileDiff{Status: domain.FileRenamed, Similarity: 90, OldMode: 0o100644, NewMode: 0o100644}, "renamed, 90% similar", true}, | ||
| 100 | {"copy", domain.FileDiff{Status: domain.FileCopied, Similarity: 80}, "copied, 80% similar", true}, | 100 | {"copy", domain.FileDiff{Status: domain.FileCopied, Similarity: 80}, "copied, 80% similar", true}, | ||
| 101 | {"type change", domain.FileDiff{Status: domain.FileTypeChanged}, "type changed", true}, | 101 | {"type change", domain.FileDiff{Status: domain.FileTypeChanged}, "type changed", true}, | ||
| 102 | - | {"truncated", domain.FileDiff{Truncated: true, Additions: 2500}, "too large", false}, | 102 | + | {"truncated", domain.FileDiff{Omitted: domain.OmitLarge, Additions: 2500}, "too large", false}, |
| 103 | } | 103 | } | ||
| 104 | for _, tt := range tests { | 104 | for _, tt := range tests { | ||
| 105 | t.Run(tt.name, func(t *testing.T) { | 105 | t.Run(tt.name, func(t *testing.T) { | ||
| @@ -120,7 +120,7 @@ func TestDiffFilesPaths(t *testing.T) { | |||||
| 120 | {Status: domain.FileAdded, NewPath: "a.go", Additions: 1, Hunks: []domain.Hunk{{Lines: []domain.Line{line(domain.LineAddition, 0, 1, "x")}}}}, | 120 | {Status: domain.FileAdded, NewPath: "a.go", Additions: 1, Hunks: []domain.Hunk{{Lines: []domain.Line{line(domain.LineAddition, 0, 1, "x")}}}}, | ||
| 121 | {Status: domain.FileDeleted, OldPath: "b.go", Deletions: 1}, | 121 | {Status: domain.FileDeleted, OldPath: "b.go", Deletions: 1}, | ||
| 122 | {Status: domain.FileRenamed, OldPath: "c\u202e.go", NewPath: "d.go"}, | 122 | {Status: domain.FileRenamed, OldPath: "c\u202e.go", NewPath: "d.go"}, | ||
| 123 | - | {Status: domain.FileModified, NewPath: "big.go", Truncated: true, Additions: 3000}, | 123 | + | {Status: domain.FileModified, NewPath: "big.go", Omitted: domain.OmitLarge, Additions: 3000}, |
| 124 | }) | 124 | }) | ||
| 125 | assert.Equal(t, "a.go", fs[0].Path) | 125 | assert.Equal(t, "a.go", fs[0].Path) | ||
| 126 | assert.True(t, fs[0].Open) | 126 | assert.True(t, fs[0].Open) | ||
M internal/app/cli_mr.go +21 -10
| @@ -470,14 +470,16 @@ type jsonHunk struct { | |||
| 470 | 470 | } | |
| 471 | 471 | ||
| 472 | 472 | type jsonFile struct { | |
| 473 | - | OldPath string `json:"old_path"` | |
| 474 | - | NewPath string `json:"new_path"` | |
| 475 | - | Status string `json:"status"` | |
| 476 | - | Additions int32 `json:"additions"` | |
| 477 | - | Deletions int32 `json:"deletions"` | |
| 478 | - | Binary bool `json:"binary"` | |
| 479 | - | Truncated bool `json:"truncated"` | |
| 480 | - | Hunks []jsonHunk `json:"hunks,omitempty"` | |
| 473 | + | OldPath string `json:"old_path"` | |
| 474 | + | NewPath string `json:"new_path"` | |
| 475 | + | Status string `json:"status"` | |
| 476 | + | Additions int32 `json:"additions"` | |
| 477 | + | Deletions int32 `json:"deletions"` | |
| 478 | + | Binary bool `json:"binary"` | |
| 479 | + | Truncated bool `json:"truncated"` | |
| 480 | + | // Omitted is generated, large, budget or unavailable; absent when the hunks are not omitted. | |
| 481 | + | Omitted string `json:"omitted,omitempty"` | |
| 482 | + | Hunks []jsonHunk `json:"hunks,omitempty"` | |
| 481 | 483 | } | |
| 482 | 484 | ||
| 483 | 485 | type jsonCommit struct { | |
| @@ -516,6 +518,14 @@ func fileStatusName(s ermite.FileStatus) string { | |||
| 516 | 518 | return strings.ToLower(strings.TrimPrefix(s.String(), "FILE_STATUS_")) | |
| 517 | 519 | } | |
| 518 | 520 | ||
| 521 | + | // fileOmissionName is the lowercase reason of an omitted file, "" for none. | |
| 522 | + | func fileOmissionName(o ermite.FileOmission) string { | |
| 523 | + | if o == ermite.FileOmission_FILE_OMISSION_UNSPECIFIED { | |
| 524 | + | return "" | |
| 525 | + | } | |
| 526 | + | return strings.ToLower(strings.TrimPrefix(o.String(), "FILE_OMISSION_")) | |
| 527 | + | } | |
| 528 | + | ||
| 519 | 529 | func lineKindName(k ermite.LineKind) string { | |
| 520 | 530 | return strings.ToLower(strings.TrimPrefix(k.String(), "LINE_KIND_")) | |
| 521 | 531 | } | |
| @@ -540,6 +550,7 @@ func toJSONDetail(d *ermite.GetMergeRequestResponse, withDiff bool, baseURL stri | |||
| 540 | 550 | jf := jsonFile{ | |
| 541 | 551 | OldPath: f.GetOldPath(), NewPath: f.GetNewPath(), Status: fileStatusName(f.GetStatus()), | |
| 542 | 552 | Additions: f.GetAdditions(), Deletions: f.GetDeletions(), Binary: f.GetBinary(), Truncated: f.GetTruncated(), | |
| 553 | + | Omitted: fileOmissionName(f.GetOmitted()), | |
| 543 | 554 | } | |
| 544 | 555 | if withDiff { | |
| 545 | 556 | for _, h := range f.GetHunks() { | |
| @@ -581,8 +592,8 @@ func newMRShowCmd(env *cliEnv) *cobra.Command { | |||
| 581 | 592 | "or in conflict with the paths that conflict (rebase needed)." + jsonMRHelp + jsonMergeabilityHelp + jsonCommentHelp + "\n" + | |
| 582 | 593 | "--json adds to the merge request fields: commits (id, author, summary), commits_truncated,\n" + | |
| 583 | 594 | "diff_base_id (the commit the diff starts from), stats (files_changed, additions, deletions),\n" + | |
| 584 | - | "diff_truncated, files (old_path, new_path, status, additions, deletions, binary, truncated and,\n" + | |
| 585 | - | "with --diff, hunks of lines with kind, old_number, new_number and text), comments, last_read_id\n" + | |
| 595 | + | "diff_truncated, files (old_path, new_path, status, additions, deletions, binary, truncated, omitted (generated, large, budget or unavailable; absent when the hunks are\n" + | |
| 596 | + | "not omitted) and, with --diff, hunks of lines with kind, old_number, new_number and text), comments, last_read_id\n" + | |
| 586 | 597 | "and last_read_lost; verdicts (author, verdict, comment_id, commit_id, created_at, stale).", | |
| 587 | 598 | Args: cobra.ExactArgs(2), | |
| 588 | 599 | ValidArgsFunction: completeMRRepoThenNumber(env), | |
| @@ -470,14 +470,16 @@ type jsonHunk struct { | |||||
| 470 | } | 470 | } | ||
| 471 | 471 | ||||
| 472 | type jsonFile struct { | 472 | type jsonFile struct { | ||
| 473 | - | OldPath string `json:"old_path"` | 473 | + | OldPath string `json:"old_path"` |
| 474 | - | NewPath string `json:"new_path"` | 474 | + | NewPath string `json:"new_path"` |
| 475 | - | Status string `json:"status"` | 475 | + | Status string `json:"status"` |
| 476 | - | Additions int32 `json:"additions"` | 476 | + | Additions int32 `json:"additions"` |
| 477 | - | Deletions int32 `json:"deletions"` | 477 | + | Deletions int32 `json:"deletions"` |
| 478 | - | Binary bool `json:"binary"` | 478 | + | Binary bool `json:"binary"` |
| 479 | - | Truncated bool `json:"truncated"` | 479 | + | Truncated bool `json:"truncated"` |
| 480 | - | Hunks []jsonHunk `json:"hunks,omitempty"` | 480 | + | // Omitted is generated, large, budget or unavailable; absent when the hunks are not omitted. |
| 481 | + | Omitted string `json:"omitted,omitempty"` | |||
| 482 | + | Hunks []jsonHunk `json:"hunks,omitempty"` | |||
| 481 | } | 483 | } | ||
| 482 | 484 | ||||
| 483 | type jsonCommit struct { | 485 | type jsonCommit struct { | ||
| @@ -516,6 +518,14 @@ func fileStatusName(s ermite.FileStatus) string { | |||||
| 516 | return strings.ToLower(strings.TrimPrefix(s.String(), "FILE_STATUS_")) | 518 | return strings.ToLower(strings.TrimPrefix(s.String(), "FILE_STATUS_")) | ||
| 517 | } | 519 | } | ||
| 518 | 520 | ||||
| 521 | + | // fileOmissionName is the lowercase reason of an omitted file, "" for none. | |||
| 522 | + | func fileOmissionName(o ermite.FileOmission) string { | |||
| 523 | + | if o == ermite.FileOmission_FILE_OMISSION_UNSPECIFIED { | |||
| 524 | + | return "" | |||
| 525 | + | } | |||
| 526 | + | return strings.ToLower(strings.TrimPrefix(o.String(), "FILE_OMISSION_")) | |||
| 527 | + | } | |||
| 528 | + | ||||
| 519 | func lineKindName(k ermite.LineKind) string { | 529 | func lineKindName(k ermite.LineKind) string { | ||
| 520 | return strings.ToLower(strings.TrimPrefix(k.String(), "LINE_KIND_")) | 530 | return strings.ToLower(strings.TrimPrefix(k.String(), "LINE_KIND_")) | ||
| 521 | } | 531 | } | ||
| @@ -540,6 +550,7 @@ func toJSONDetail(d *ermite.GetMergeRequestResponse, withDiff bool, baseURL stri | |||||
| 540 | jf := jsonFile{ | 550 | jf := jsonFile{ | ||
| 541 | OldPath: f.GetOldPath(), NewPath: f.GetNewPath(), Status: fileStatusName(f.GetStatus()), | 551 | OldPath: f.GetOldPath(), NewPath: f.GetNewPath(), Status: fileStatusName(f.GetStatus()), | ||
| 542 | Additions: f.GetAdditions(), Deletions: f.GetDeletions(), Binary: f.GetBinary(), Truncated: f.GetTruncated(), | 552 | Additions: f.GetAdditions(), Deletions: f.GetDeletions(), Binary: f.GetBinary(), Truncated: f.GetTruncated(), | ||
| 553 | + | Omitted: fileOmissionName(f.GetOmitted()), | |||
| 543 | } | 554 | } | ||
| 544 | if withDiff { | 555 | if withDiff { | ||
| 545 | for _, h := range f.GetHunks() { | 556 | for _, h := range f.GetHunks() { | ||
| @@ -581,8 +592,8 @@ func newMRShowCmd(env *cliEnv) *cobra.Command { | |||||
| 581 | "or in conflict with the paths that conflict (rebase needed)." + jsonMRHelp + jsonMergeabilityHelp + jsonCommentHelp + "\n" + | 592 | "or in conflict with the paths that conflict (rebase needed)." + jsonMRHelp + jsonMergeabilityHelp + jsonCommentHelp + "\n" + | ||
| 582 | "--json adds to the merge request fields: commits (id, author, summary), commits_truncated,\n" + | 593 | "--json adds to the merge request fields: commits (id, author, summary), commits_truncated,\n" + | ||
| 583 | "diff_base_id (the commit the diff starts from), stats (files_changed, additions, deletions),\n" + | 594 | "diff_base_id (the commit the diff starts from), stats (files_changed, additions, deletions),\n" + | ||
| 584 | - | "diff_truncated, files (old_path, new_path, status, additions, deletions, binary, truncated and,\n" + | 595 | + | "diff_truncated, files (old_path, new_path, status, additions, deletions, binary, truncated, omitted (generated, large, budget or unavailable; absent when the hunks are\n" + |
| 585 | - | "with --diff, hunks of lines with kind, old_number, new_number and text), comments, last_read_id\n" + | 596 | + | "not omitted) and, with --diff, hunks of lines with kind, old_number, new_number and text), comments, last_read_id\n" + |
| 586 | "and last_read_lost; verdicts (author, verdict, comment_id, commit_id, created_at, stale).", | 597 | "and last_read_lost; verdicts (author, verdict, comment_id, commit_id, created_at, stale).", | ||
| 587 | Args: cobra.ExactArgs(2), | 598 | Args: cobra.ExactArgs(2), | ||
| 588 | ValidArgsFunction: completeMRRepoThenNumber(env), | 599 | ValidArgsFunction: completeMRRepoThenNumber(env), | ||
M internal/app/cli_mr_test.go +15 -0
| @@ -563,3 +563,18 @@ func TestMRWebURL(t *testing.T) { | |||
| 563 | 563 | t.Run(tt.name, func(t *testing.T) { assert.Equal(t, tt.want, mrWebURL(tt.base, tt.repo, 42)) }) | |
| 564 | 564 | } | |
| 565 | 565 | } | |
| 566 | + | ||
| 567 | + | func TestJSONFileOmitted(t *testing.T) { | |
| 568 | + | d := &ermite.GetMergeRequestResponse{MergeRequest: mrOne, Diff: &ermite.Diff{Files: []*ermite.FileDiff{ | |
| 569 | + | {NewPath: "a"}, | |
| 570 | + | {NewPath: "go.sum", Omitted: ermite.FileOmission_FILE_OMISSION_GENERATED, Truncated: true}, | |
| 571 | + | {NewPath: "b", Omitted: ermite.FileOmission_FILE_OMISSION_UNAVAILABLE, Truncated: true}, | |
| 572 | + | }}} | |
| 573 | + | got := toJSONDetail(d, false, "https://git.example.org") | |
| 574 | + | assert.Equal(t, []string{"", "generated", "unavailable"}, []string{got.Files[0].Omitted, got.Files[1].Omitted, got.Files[2].Omitted}) | |
| 575 | + | raw, err := json.Marshal(got.Files) | |
| 576 | + | require.NoError(t, err) | |
| 577 | + | assert.Contains(t, string(raw), `"omitted":"generated"`) | |
| 578 | + | assert.Contains(t, string(raw), `"truncated":true`) | |
| 579 | + | assert.Equal(t, 2, strings.Count(string(raw), `"omitted"`)) | |
| 580 | + | } | |
| @@ -563,3 +563,18 @@ func TestMRWebURL(t *testing.T) { | |||||
| 563 | t.Run(tt.name, func(t *testing.T) { assert.Equal(t, tt.want, mrWebURL(tt.base, tt.repo, 42)) }) | 563 | t.Run(tt.name, func(t *testing.T) { assert.Equal(t, tt.want, mrWebURL(tt.base, tt.repo, 42)) }) | ||
| 564 | } | 564 | } | ||
| 565 | } | 565 | } | ||
| 566 | + | ||||
| 567 | + | func TestJSONFileOmitted(t *testing.T) { | |||
| 568 | + | d := &ermite.GetMergeRequestResponse{MergeRequest: mrOne, Diff: &ermite.Diff{Files: []*ermite.FileDiff{ | |||
| 569 | + | {NewPath: "a"}, | |||
| 570 | + | {NewPath: "go.sum", Omitted: ermite.FileOmission_FILE_OMISSION_GENERATED, Truncated: true}, | |||
| 571 | + | {NewPath: "b", Omitted: ermite.FileOmission_FILE_OMISSION_UNAVAILABLE, Truncated: true}, | |||
| 572 | + | }}} | |||
| 573 | + | got := toJSONDetail(d, false, "https://git.example.org") | |||
| 574 | + | assert.Equal(t, []string{"", "generated", "unavailable"}, []string{got.Files[0].Omitted, got.Files[1].Omitted, got.Files[2].Omitted}) | |||
| 575 | + | raw, err := json.Marshal(got.Files) | |||
| 576 | + | require.NoError(t, err) | |||
| 577 | + | assert.Contains(t, string(raw), `"omitted":"generated"`) | |||
| 578 | + | assert.Contains(t, string(raw), `"truncated":true`) | |||
| 579 | + | assert.Equal(t, 2, strings.Count(string(raw), `"omitted"`)) | |||
| 580 | + | } | |||
M internal/domain/budget.go +45 -11
| @@ -2,7 +2,10 @@ | |||
| 2 | 2 | ||
| 3 | 3 | package domain | |
| 4 | 4 | ||
| 5 | - | import "fmt" | |
| 5 | + | import ( | |
| 6 | + | "fmt" | |
| 7 | + | "math" | |
| 8 | + | ) | |
| 6 | 9 | ||
| 7 | 10 | // ValidateCommitObjects checks a request before any repository I/O. max is the | |
| 8 | 11 | // caller's commit-count limit. | |
| @@ -45,7 +48,7 @@ func (b *CommitObjectBudget) Charge(id ObjectID, size int64) { | |||
| 45 | 48 | b.total += size | |
| 46 | 49 | } | |
| 47 | 50 | ||
| 48 | - | // DiffBudget owns file selection and retained hunk accounting. Truncated files | |
| 51 | + | // DiffBudget owns file selection and retained hunk accounting. Omitted files | |
| 49 | 52 | // consume no budget; streaming readers may use a copy while attempting a file. | |
| 50 | 53 | type DiffBudget struct { | |
| 51 | 54 | limits DiffLimits | |
| @@ -69,20 +72,51 @@ func (b DiffBudget) Window(files int) (kept, from, to int, truncated bool) { | |||
| 69 | 72 | return | |
| 70 | 73 | } | |
| 71 | 74 | ||
| 72 | - | func (b DiffBudget) ChangedLines(binary bool, n int) bool { return !binary && n > 0 } | |
| 73 | - | ||
| 74 | - | func (b DiffBudget) WantsHunks(binary bool, n int) bool { | |
| 75 | - | return b.ChangedLines(binary, n) && n <= b.limits.MaxFileLines && n <= b.linesLeft && b.bytesLeft > 0 | |
| 75 | + | // Admit decides whether the hunks of a file are read. read is false with | |
| 76 | + | // OmitNone for a binary file or one without changed lines (nothing to show). | |
| 77 | + | // Otherwise read is false with the reason the file is omitted. path is the path | |
| 78 | + | // a comment designates (see FileDiff.CommentPath). | |
| 79 | + | func (b DiffBudget) Admit(binary bool, path string, changed int) (read bool, why FileOmission) { | |
| 80 | + | switch { | |
| 81 | + | case binary || changed <= 0: | |
| 82 | + | return false, OmitNone | |
| 83 | + | case b.limits.SkipGenerated && IsGeneratedPath(path): | |
| 84 | + | return false, OmitGenerated | |
| 85 | + | case b.limits.MaxFileLines > 0 && changed > b.limits.MaxFileLines: | |
| 86 | + | return false, OmitLarge | |
| 87 | + | case b.limits.MaxTotalLines > 0 && changed > b.linesLeft, b.bytesLeft <= 0: | |
| 88 | + | return false, OmitBudget | |
| 89 | + | } | |
| 90 | + | return true, OmitNone | |
| 76 | 91 | } | |
| 77 | 92 | ||
| 78 | - | func (b DiffBudget) FileLines() int { return min(b.limits.MaxFileLines, b.linesLeft) } | |
| 93 | + | // FileLines is the most changed lines the next file may have; math.MaxInt when | |
| 94 | + | // neither MaxFileLines nor MaxTotalLines limits it. | |
| 95 | + | func (b DiffBudget) FileLines() int { | |
| 96 | + | n := math.MaxInt | |
| 97 | + | if b.limits.MaxFileLines > 0 { | |
| 98 | + | n = b.limits.MaxFileLines | |
| 99 | + | } | |
| 100 | + | if b.limits.MaxTotalLines > 0 { | |
| 101 | + | n = min(n, b.linesLeft) | |
| 102 | + | } | |
| 103 | + | return n | |
| 104 | + | } | |
| 79 | 105 | ||
| 80 | 106 | // ChargeLine accounts for decoded text bytes and, for additions or deletions, | |
| 81 | - | // one changed line. Context consumes bytes only. It reports whether the line fits. | |
| 82 | - | func (b *DiffBudget) ChargeLine(bytes int, changed bool) bool { | |
| 83 | - | if changed { | |
| 107 | + | // one changed line. Context consumes bytes only. It returns OmitNone when the | |
| 108 | + | // line fits, OmitBudget when the changed lines ran out and OmitUnavailable when | |
| 109 | + | // the bytes ran out. | |
| 110 | + | func (b *DiffBudget) ChargeLine(bytes int, changed bool) FileOmission { | |
| 111 | + | if changed && b.limits.MaxTotalLines > 0 { | |
| 84 | 112 | b.linesLeft-- | |
| 85 | 113 | } | |
| 86 | 114 | b.bytesLeft -= bytes | |
| 87 | - | return b.linesLeft >= 0 && b.bytesLeft >= 0 | |
| 115 | + | switch { | |
| 116 | + | case b.limits.MaxTotalLines > 0 && b.linesLeft < 0: | |
| 117 | + | return OmitBudget | |
| 118 | + | case b.bytesLeft < 0: | |
| 119 | + | return OmitUnavailable | |
| 120 | + | } | |
| 121 | + | return OmitNone | |
| 88 | 122 | } | |
| @@ -2,7 +2,10 @@ | |||||
| 2 | 2 | ||||
| 3 | package domain | 3 | package domain | ||
| 4 | 4 | ||||
| 5 | - | import "fmt" | 5 | + | import ( |
| 6 | + | "fmt" | |||
| 7 | + | "math" | |||
| 8 | + | ) | |||
| 6 | 9 | ||||
| 7 | // ValidateCommitObjects checks a request before any repository I/O. max is the | 10 | // ValidateCommitObjects checks a request before any repository I/O. max is the | ||
| 8 | // caller's commit-count limit. | 11 | // caller's commit-count limit. | ||
| @@ -45,7 +48,7 @@ func (b *CommitObjectBudget) Charge(id ObjectID, size int64) { | |||||
| 45 | b.total += size | 48 | b.total += size | ||
| 46 | } | 49 | } | ||
| 47 | 50 | ||||
| 48 | - | // DiffBudget owns file selection and retained hunk accounting. Truncated files | 51 | + | // DiffBudget owns file selection and retained hunk accounting. Omitted files |
| 49 | // consume no budget; streaming readers may use a copy while attempting a file. | 52 | // consume no budget; streaming readers may use a copy while attempting a file. | ||
| 50 | type DiffBudget struct { | 53 | type DiffBudget struct { | ||
| 51 | limits DiffLimits | 54 | limits DiffLimits | ||
| @@ -69,20 +72,51 @@ func (b DiffBudget) Window(files int) (kept, from, to int, truncated bool) { | |||||
| 69 | return | 72 | return | ||
| 70 | } | 73 | } | ||
| 71 | 74 | ||||
| 72 | - | func (b DiffBudget) ChangedLines(binary bool, n int) bool { return !binary && n > 0 } | 75 | + | // Admit decides whether the hunks of a file are read. read is false with |
| 73 | - | 76 | + | // OmitNone for a binary file or one without changed lines (nothing to show). | |
| 74 | - | func (b DiffBudget) WantsHunks(binary bool, n int) bool { | 77 | + | // Otherwise read is false with the reason the file is omitted. path is the path |
| 75 | - | return b.ChangedLines(binary, n) && n <= b.limits.MaxFileLines && n <= b.linesLeft && b.bytesLeft > 0 | 78 | + | // a comment designates (see FileDiff.CommentPath). |
| 79 | + | func (b DiffBudget) Admit(binary bool, path string, changed int) (read bool, why FileOmission) { | |||
| 80 | + | switch { | |||
| 81 | + | case binary || changed <= 0: | |||
| 82 | + | return false, OmitNone | |||
| 83 | + | case b.limits.SkipGenerated && IsGeneratedPath(path): | |||
| 84 | + | return false, OmitGenerated | |||
| 85 | + | case b.limits.MaxFileLines > 0 && changed > b.limits.MaxFileLines: | |||
| 86 | + | return false, OmitLarge | |||
| 87 | + | case b.limits.MaxTotalLines > 0 && changed > b.linesLeft, b.bytesLeft <= 0: | |||
| 88 | + | return false, OmitBudget | |||
| 89 | + | } | |||
| 90 | + | return true, OmitNone | |||
| 76 | } | 91 | } | ||
| 77 | 92 | ||||
| 78 | - | func (b DiffBudget) FileLines() int { return min(b.limits.MaxFileLines, b.linesLeft) } | 93 | + | // FileLines is the most changed lines the next file may have; math.MaxInt when |
| 94 | + | // neither MaxFileLines nor MaxTotalLines limits it. | |||
| 95 | + | func (b DiffBudget) FileLines() int { | |||
| 96 | + | n := math.MaxInt | |||
| 97 | + | if b.limits.MaxFileLines > 0 { | |||
| 98 | + | n = b.limits.MaxFileLines | |||
| 99 | + | } | |||
| 100 | + | if b.limits.MaxTotalLines > 0 { | |||
| 101 | + | n = min(n, b.linesLeft) | |||
| 102 | + | } | |||
| 103 | + | return n | |||
| 104 | + | } | |||
| 79 | 105 | ||||
| 80 | // ChargeLine accounts for decoded text bytes and, for additions or deletions, | 106 | // ChargeLine accounts for decoded text bytes and, for additions or deletions, | ||
| 81 | - | // one changed line. Context consumes bytes only. It reports whether the line fits. | 107 | + | // one changed line. Context consumes bytes only. It returns OmitNone when the |
| 82 | - | func (b *DiffBudget) ChargeLine(bytes int, changed bool) bool { | 108 | + | // line fits, OmitBudget when the changed lines ran out and OmitUnavailable when |
| 83 | - | if changed { | 109 | + | // the bytes ran out. |
| 110 | + | func (b *DiffBudget) ChargeLine(bytes int, changed bool) FileOmission { | |||
| 111 | + | if changed && b.limits.MaxTotalLines > 0 { | |||
| 84 | b.linesLeft-- | 112 | b.linesLeft-- | ||
| 85 | } | 113 | } | ||
| 86 | b.bytesLeft -= bytes | 114 | b.bytesLeft -= bytes | ||
| 87 | - | return b.linesLeft >= 0 && b.bytesLeft >= 0 | 115 | + | switch { |
| 116 | + | case b.limits.MaxTotalLines > 0 && b.linesLeft < 0: | |||
| 117 | + | return OmitBudget | |||
| 118 | + | case b.bytesLeft < 0: | |||
| 119 | + | return OmitUnavailable | |||
| 120 | + | } | |||
| 121 | + | return OmitNone | |||
| 88 | } | 122 | } | ||
M internal/domain/budget_test.go +73 -21
| @@ -94,21 +94,32 @@ func TestDiffBudgetWindow(t *testing.T) { | |||
| 94 | 94 | } | |
| 95 | 95 | } | |
| 96 | 96 | ||
| 97 | - | func TestDiffBudgetHunks(t *testing.T) { | |
| 97 | + | func TestDiffBudgetAdmit(t *testing.T) { | |
| 98 | 98 | for _, tt := range []struct { | |
| 99 | - | name string | |
| 100 | - | binary bool | |
| 101 | - | n, lines, bytes int | |
| 102 | - | want bool | |
| 99 | + | name string | |
| 100 | + | limits DiffLimits | |
| 101 | + | binary bool | |
| 102 | + | path string | |
| 103 | + | n int | |
| 104 | + | wantRead bool | |
| 105 | + | wantWhy FileOmission | |
| 103 | 106 | }{ | |
| 104 | - | {"fits", false, 3, 3, 1, true}, {"binary", true, 3, 3, 1, false}, | |
| 105 | - | {"no changes", false, 0, 3, 1, false}, {"file limit", false, 4, 5, 1, false}, | |
| 106 | - | {"total lines", false, 3, 2, 1, false}, {"no bytes", false, 1, 3, 0, false}, | |
| 107 | + | {"fits", DiffLimits{MaxFileLines: 3, MaxTotalLines: 3, MaxTotalBytes: 1}, false, "a.go", 3, true, OmitNone}, | |
| 108 | + | {"binary", DiffLimits{MaxTotalBytes: 1}, true, "a.bin", 3, false, OmitNone}, | |
| 109 | + | {"no changes", DiffLimits{MaxTotalBytes: 1}, false, "a.go", 0, false, OmitNone}, | |
| 110 | + | {"file limit", DiffLimits{MaxFileLines: 3, MaxTotalLines: 5, MaxTotalBytes: 1}, false, "a.go", 4, false, OmitLarge}, | |
| 111 | + | {"total lines", DiffLimits{MaxFileLines: 3, MaxTotalLines: 2, MaxTotalBytes: 1}, false, "a.go", 3, false, OmitBudget}, | |
| 112 | + | {"no bytes", DiffLimits{MaxFileLines: 3, MaxTotalLines: 3}, false, "a.go", 1, false, OmitBudget}, | |
| 113 | + | {"generated", DiffLimits{SkipGenerated: true, MaxTotalBytes: 1}, false, "go.sum", 1, false, OmitGenerated}, | |
| 114 | + | {"generated not skipped", DiffLimits{MaxTotalBytes: 1}, false, "go.sum", 1, true, OmitNone}, | |
| 115 | + | {"generated before large", DiffLimits{SkipGenerated: true, MaxFileLines: 1, MaxTotalBytes: 1}, false, "go.sum", 9, false, OmitGenerated}, | |
| 116 | + | {"unlimited lines", DiffLimits{MaxTotalBytes: 1}, false, "a.go", 1 << 20, true, OmitNone}, | |
| 107 | 117 | } { | |
| 108 | 118 | t.Run(tt.name, func(t *testing.T) { | |
| 109 | - | b := NewDiffBudget(DiffLimits{MaxFileLines: 3, MaxTotalLines: tt.lines, MaxTotalBytes: tt.bytes}) | |
| 110 | - | if got := b.WantsHunks(tt.binary, tt.n); got != tt.want { | |
| 111 | - | t.Fatalf("got %v", got) | |
| 119 | + | b := NewDiffBudget(tt.limits) | |
| 120 | + | read, why := b.Admit(tt.binary, tt.path, tt.n) | |
| 121 | + | if read != tt.wantRead || why != tt.wantWhy { | |
| 122 | + | t.Fatalf("got %v,%v", read, why) | |
| 112 | 123 | } | |
| 113 | 124 | }) | |
| 114 | 125 | } | |
| @@ -118,42 +129,83 @@ func TestDiffBudgetCharge(t *testing.T) { | |||
| 118 | 129 | for _, tt := range []struct { | |
| 119 | 130 | name string | |
| 120 | 131 | bytes int | |
| 121 | - | fits bool | |
| 122 | - | }{{"exact", 3, true}, {"over", 4, false}} { | |
| 132 | + | want FileOmission | |
| 133 | + | }{{"exact", 3, OmitNone}, {"over", 4, OmitUnavailable}} { | |
| 123 | 134 | t.Run(tt.name, func(t *testing.T) { | |
| 124 | 135 | b := NewDiffBudget(DiffLimits{MaxFileLines: 2, MaxTotalLines: 1, MaxTotalBytes: 3}) | |
| 125 | 136 | before := b | |
| 126 | 137 | if b.FileLines() != 1 { | |
| 127 | 138 | t.Fatal("wrong file limit") | |
| 128 | 139 | } | |
| 129 | - | if b.ChargeLine(tt.bytes, true) != tt.fits { | |
| 130 | - | t.Fatal("wrong charge result") | |
| 140 | + | if got := b.ChargeLine(tt.bytes, true); got != tt.want { | |
| 141 | + | t.Fatalf("wrong charge result %v", got) | |
| 131 | 142 | } | |
| 132 | - | if b.ChargeLine(0, true) { | |
| 143 | + | if b.ChargeLine(0, true) == OmitNone { | |
| 133 | 144 | t.Fatal("line budget exceeded") | |
| 134 | 145 | } | |
| 135 | 146 | b = before | |
| 136 | - | if !b.WantsHunks(false, 1) { | |
| 147 | + | if read, _ := b.Admit(false, "a", 1); !read { | |
| 137 | 148 | t.Fatal("discarded file consumed budget") | |
| 138 | 149 | } | |
| 139 | 150 | }) | |
| 140 | 151 | } | |
| 141 | 152 | } | |
| 142 | 153 | ||
| 154 | + | func TestDiffBudgetChargeReasons(t *testing.T) { | |
| 155 | + | b := NewDiffBudget(DiffLimits{MaxTotalLines: 1, MaxTotalBytes: 3}) | |
| 156 | + | if b.ChargeLine(2, false) != OmitNone || b.FileLines() != 1 { | |
| 157 | + | t.Fatal("context consumes bytes only") | |
| 158 | + | } | |
| 159 | + | if b.ChargeLine(1, true) != OmitNone || b.FileLines() != 0 { | |
| 160 | + | t.Fatal("first changed line fits") | |
| 161 | + | } | |
| 162 | + | if got := b.ChargeLine(0, true); got != OmitBudget { | |
| 163 | + | t.Fatalf("lines ran out: %v", got) | |
| 164 | + | } | |
| 165 | + | b = NewDiffBudget(DiffLimits{MaxTotalLines: 9, MaxTotalBytes: 1}) | |
| 166 | + | if got := b.ChargeLine(2, false); got != OmitUnavailable { | |
| 167 | + | t.Fatalf("bytes ran out: %v", got) | |
| 168 | + | } | |
| 169 | + | free := NewDiffBudget(DiffLimits{MaxTotalBytes: 5}) | |
| 170 | + | for range 100 { | |
| 171 | + | if free.ChargeLine(0, true) != OmitNone { | |
| 172 | + | t.Fatal("0 means no line limit") | |
| 173 | + | } | |
| 174 | + | } | |
| 175 | + | if free.FileLines() < 1<<30 { | |
| 176 | + | t.Fatal("unlimited FileLines") | |
| 177 | + | } | |
| 178 | + | } | |
| 179 | + | ||
| 180 | + | func TestFileOmissionCollapsed(t *testing.T) { | |
| 181 | + | for o, want := range map[FileOmission]bool{OmitNone: false, OmitGenerated: true, OmitLarge: true, OmitBudget: true, OmitUnavailable: false} { | |
| 182 | + | if o.Collapsed() != want { | |
| 183 | + | t.Errorf("%d: got %v", o, !want) | |
| 184 | + | } | |
| 185 | + | } | |
| 186 | + | } | |
| 187 | + | ||
| 188 | + | func TestOneFileDiffLimits(t *testing.T) { | |
| 189 | + | l := OneFileDiffLimits(3) | |
| 190 | + | if l.From != 3 || l.To != 4 || l.MaxFileLines != 0 || l.MaxTotalLines != 0 || l.MaxFiles != 0 || l.MaxTotalBytes != DefaultDiffLimits().MaxTotalBytes { | |
| 191 | + | t.Fatalf("got %+v", l) | |
| 192 | + | } | |
| 193 | + | } | |
| 194 | + | ||
| 143 | 195 | func TestDiffBudgetContext(t *testing.T) { | |
| 144 | 196 | b := NewDiffBudget(DiffLimits{MaxFileLines: 2, MaxTotalLines: 2, MaxTotalBytes: 6}) | |
| 145 | - | if !b.ChargeLine(2, false) || b.FileLines() != 2 { | |
| 197 | + | if b.ChargeLine(2, false) != OmitNone || b.FileLines() != 2 { | |
| 146 | 198 | t.Fatal("context consumed changed-line budget") | |
| 147 | 199 | } | |
| 148 | 200 | first := b.ChargeLine(1, true) | |
| 149 | 201 | second := b.ChargeLine(1, true) | |
| 150 | - | if !first || !second || b.FileLines() != 0 { | |
| 202 | + | if first != OmitNone || second != OmitNone || b.FileLines() != 0 { | |
| 151 | 203 | t.Fatal("additions and deletions must consume changed-line budget") | |
| 152 | 204 | } | |
| 153 | - | if !b.ChargeLine(2, false) { | |
| 205 | + | if b.ChargeLine(2, false) != OmitNone { | |
| 154 | 206 | t.Fatal("context at the byte boundary must fit even without changed lines left") | |
| 155 | 207 | } | |
| 156 | - | if b.ChargeLine(1, false) { | |
| 208 | + | if b.ChargeLine(1, false) != OmitUnavailable { | |
| 157 | 209 | t.Fatal("context must not exceed the byte budget") | |
| 158 | 210 | } | |
| 159 | 211 | } | |
| @@ -94,21 +94,32 @@ func TestDiffBudgetWindow(t *testing.T) { | |||||
| 94 | } | 94 | } | ||
| 95 | } | 95 | } | ||
| 96 | 96 | ||||
| 97 | - | func TestDiffBudgetHunks(t *testing.T) { | 97 | + | func TestDiffBudgetAdmit(t *testing.T) { |
| 98 | for _, tt := range []struct { | 98 | for _, tt := range []struct { | ||
| 99 | - | name string | 99 | + | name string |
| 100 | - | binary bool | 100 | + | limits DiffLimits |
| 101 | - | n, lines, bytes int | 101 | + | binary bool |
| 102 | - | want bool | 102 | + | path string |
| 103 | + | n int | |||
| 104 | + | wantRead bool | |||
| 105 | + | wantWhy FileOmission | |||
| 103 | }{ | 106 | }{ | ||
| 104 | - | {"fits", false, 3, 3, 1, true}, {"binary", true, 3, 3, 1, false}, | 107 | + | {"fits", DiffLimits{MaxFileLines: 3, MaxTotalLines: 3, MaxTotalBytes: 1}, false, "a.go", 3, true, OmitNone}, |
| 105 | - | {"no changes", false, 0, 3, 1, false}, {"file limit", false, 4, 5, 1, false}, | 108 | + | {"binary", DiffLimits{MaxTotalBytes: 1}, true, "a.bin", 3, false, OmitNone}, |
| 106 | - | {"total lines", false, 3, 2, 1, false}, {"no bytes", false, 1, 3, 0, false}, | 109 | + | {"no changes", DiffLimits{MaxTotalBytes: 1}, false, "a.go", 0, false, OmitNone}, |
| 110 | + | {"file limit", DiffLimits{MaxFileLines: 3, MaxTotalLines: 5, MaxTotalBytes: 1}, false, "a.go", 4, false, OmitLarge}, | |||
| 111 | + | {"total lines", DiffLimits{MaxFileLines: 3, MaxTotalLines: 2, MaxTotalBytes: 1}, false, "a.go", 3, false, OmitBudget}, | |||
| 112 | + | {"no bytes", DiffLimits{MaxFileLines: 3, MaxTotalLines: 3}, false, "a.go", 1, false, OmitBudget}, | |||
| 113 | + | {"generated", DiffLimits{SkipGenerated: true, MaxTotalBytes: 1}, false, "go.sum", 1, false, OmitGenerated}, | |||
| 114 | + | {"generated not skipped", DiffLimits{MaxTotalBytes: 1}, false, "go.sum", 1, true, OmitNone}, | |||
| 115 | + | {"generated before large", DiffLimits{SkipGenerated: true, MaxFileLines: 1, MaxTotalBytes: 1}, false, "go.sum", 9, false, OmitGenerated}, | |||
| 116 | + | {"unlimited lines", DiffLimits{MaxTotalBytes: 1}, false, "a.go", 1 << 20, true, OmitNone}, | |||
| 107 | } { | 117 | } { | ||
| 108 | t.Run(tt.name, func(t *testing.T) { | 118 | t.Run(tt.name, func(t *testing.T) { | ||
| 109 | - | b := NewDiffBudget(DiffLimits{MaxFileLines: 3, MaxTotalLines: tt.lines, MaxTotalBytes: tt.bytes}) | 119 | + | b := NewDiffBudget(tt.limits) |
| 110 | - | if got := b.WantsHunks(tt.binary, tt.n); got != tt.want { | 120 | + | read, why := b.Admit(tt.binary, tt.path, tt.n) |
| 111 | - | t.Fatalf("got %v", got) | 121 | + | if read != tt.wantRead || why != tt.wantWhy { |
| 122 | + | t.Fatalf("got %v,%v", read, why) | |||
| 112 | } | 123 | } | ||
| 113 | }) | 124 | }) | ||
| 114 | } | 125 | } | ||
| @@ -118,42 +129,83 @@ func TestDiffBudgetCharge(t *testing.T) { | |||||
| 118 | for _, tt := range []struct { | 129 | for _, tt := range []struct { | ||
| 119 | name string | 130 | name string | ||
| 120 | bytes int | 131 | bytes int | ||
| 121 | - | fits bool | 132 | + | want FileOmission |
| 122 | - | }{{"exact", 3, true}, {"over", 4, false}} { | 133 | + | }{{"exact", 3, OmitNone}, {"over", 4, OmitUnavailable}} { |
| 123 | t.Run(tt.name, func(t *testing.T) { | 134 | t.Run(tt.name, func(t *testing.T) { | ||
| 124 | b := NewDiffBudget(DiffLimits{MaxFileLines: 2, MaxTotalLines: 1, MaxTotalBytes: 3}) | 135 | b := NewDiffBudget(DiffLimits{MaxFileLines: 2, MaxTotalLines: 1, MaxTotalBytes: 3}) | ||
| 125 | before := b | 136 | before := b | ||
| 126 | if b.FileLines() != 1 { | 137 | if b.FileLines() != 1 { | ||
| 127 | t.Fatal("wrong file limit") | 138 | t.Fatal("wrong file limit") | ||
| 128 | } | 139 | } | ||
| 129 | - | if b.ChargeLine(tt.bytes, true) != tt.fits { | 140 | + | if got := b.ChargeLine(tt.bytes, true); got != tt.want { |
| 130 | - | t.Fatal("wrong charge result") | 141 | + | t.Fatalf("wrong charge result %v", got) |
| 131 | } | 142 | } | ||
| 132 | - | if b.ChargeLine(0, true) { | 143 | + | if b.ChargeLine(0, true) == OmitNone { |
| 133 | t.Fatal("line budget exceeded") | 144 | t.Fatal("line budget exceeded") | ||
| 134 | } | 145 | } | ||
| 135 | b = before | 146 | b = before | ||
| 136 | - | if !b.WantsHunks(false, 1) { | 147 | + | if read, _ := b.Admit(false, "a", 1); !read { |
| 137 | t.Fatal("discarded file consumed budget") | 148 | t.Fatal("discarded file consumed budget") | ||
| 138 | } | 149 | } | ||
| 139 | }) | 150 | }) | ||
| 140 | } | 151 | } | ||
| 141 | } | 152 | } | ||
| 142 | 153 | ||||
| 154 | + | func TestDiffBudgetChargeReasons(t *testing.T) { | |||
| 155 | + | b := NewDiffBudget(DiffLimits{MaxTotalLines: 1, MaxTotalBytes: 3}) | |||
| 156 | + | if b.ChargeLine(2, false) != OmitNone || b.FileLines() != 1 { | |||
| 157 | + | t.Fatal("context consumes bytes only") | |||
| 158 | + | } | |||
| 159 | + | if b.ChargeLine(1, true) != OmitNone || b.FileLines() != 0 { | |||
| 160 | + | t.Fatal("first changed line fits") | |||
| 161 | + | } | |||
| 162 | + | if got := b.ChargeLine(0, true); got != OmitBudget { | |||
| 163 | + | t.Fatalf("lines ran out: %v", got) | |||
| 164 | + | } | |||
| 165 | + | b = NewDiffBudget(DiffLimits{MaxTotalLines: 9, MaxTotalBytes: 1}) | |||
| 166 | + | if got := b.ChargeLine(2, false); got != OmitUnavailable { | |||
| 167 | + | t.Fatalf("bytes ran out: %v", got) | |||
| 168 | + | } | |||
| 169 | + | free := NewDiffBudget(DiffLimits{MaxTotalBytes: 5}) | |||
| 170 | + | for range 100 { | |||
| 171 | + | if free.ChargeLine(0, true) != OmitNone { | |||
| 172 | + | t.Fatal("0 means no line limit") | |||
| 173 | + | } | |||
| 174 | + | } | |||
| 175 | + | if free.FileLines() < 1<<30 { | |||
| 176 | + | t.Fatal("unlimited FileLines") | |||
| 177 | + | } | |||
| 178 | + | } | |||
| 179 | + | ||||
| 180 | + | func TestFileOmissionCollapsed(t *testing.T) { | |||
| 181 | + | for o, want := range map[FileOmission]bool{OmitNone: false, OmitGenerated: true, OmitLarge: true, OmitBudget: true, OmitUnavailable: false} { | |||
| 182 | + | if o.Collapsed() != want { | |||
| 183 | + | t.Errorf("%d: got %v", o, !want) | |||
| 184 | + | } | |||
| 185 | + | } | |||
| 186 | + | } | |||
| 187 | + | ||||
| 188 | + | func TestOneFileDiffLimits(t *testing.T) { | |||
| 189 | + | l := OneFileDiffLimits(3) | |||
| 190 | + | if l.From != 3 || l.To != 4 || l.MaxFileLines != 0 || l.MaxTotalLines != 0 || l.MaxFiles != 0 || l.MaxTotalBytes != DefaultDiffLimits().MaxTotalBytes { | |||
| 191 | + | t.Fatalf("got %+v", l) | |||
| 192 | + | } | |||
| 193 | + | } | |||
| 194 | + | ||||
| 143 | func TestDiffBudgetContext(t *testing.T) { | 195 | func TestDiffBudgetContext(t *testing.T) { | ||
| 144 | b := NewDiffBudget(DiffLimits{MaxFileLines: 2, MaxTotalLines: 2, MaxTotalBytes: 6}) | 196 | b := NewDiffBudget(DiffLimits{MaxFileLines: 2, MaxTotalLines: 2, MaxTotalBytes: 6}) | ||
| 145 | - | if !b.ChargeLine(2, false) || b.FileLines() != 2 { | 197 | + | if b.ChargeLine(2, false) != OmitNone || b.FileLines() != 2 { |
| 146 | t.Fatal("context consumed changed-line budget") | 198 | t.Fatal("context consumed changed-line budget") | ||
| 147 | } | 199 | } | ||
| 148 | first := b.ChargeLine(1, true) | 200 | first := b.ChargeLine(1, true) | ||
| 149 | second := b.ChargeLine(1, true) | 201 | second := b.ChargeLine(1, true) | ||
| 150 | - | if !first || !second || b.FileLines() != 0 { | 202 | + | if first != OmitNone || second != OmitNone || b.FileLines() != 0 { |
| 151 | t.Fatal("additions and deletions must consume changed-line budget") | 203 | t.Fatal("additions and deletions must consume changed-line budget") | ||
| 152 | } | 204 | } | ||
| 153 | - | if !b.ChargeLine(2, false) { | 205 | + | if b.ChargeLine(2, false) != OmitNone { |
| 154 | t.Fatal("context at the byte boundary must fit even without changed lines left") | 206 | t.Fatal("context at the byte boundary must fit even without changed lines left") | ||
| 155 | } | 207 | } | ||
| 156 | - | if b.ChargeLine(1, false) { | 208 | + | if b.ChargeLine(1, false) != OmitUnavailable { |
| 157 | t.Fatal("context must not exceed the byte budget") | 209 | t.Fatal("context must not exceed the byte budget") | ||
| 158 | } | 210 | } | ||
| 159 | } | 211 | } | ||
M internal/domain/git.go +52 -17
| @@ -505,11 +505,37 @@ type FileDiff struct { | |||
| 505 | 505 | Additions, Deletions int | |
| 506 | 506 | // Similarity is a percentage for a renamed or copied file, 0 otherwise. | |
| 507 | 507 | Similarity int | |
| 508 | - | // Truncated means the file exceeded a diff limit: Hunks is empty although | |
| 509 | - | // the file changed. Additions and Deletions stay accurate. A file outside | |
| 510 | - | // the DiffLimits window has no Hunks and is not Truncated. | |
| 511 | - | Truncated bool | |
| 512 | - | Hunks []Hunk | |
| 508 | + | // Omitted tells why the hunks were not kept: Hunks is empty although the | |
| 509 | + | // file changed; Additions and Deletions stay accurate. A file outside the | |
| 510 | + | // window is OmitNone with no Hunks. | |
| 511 | + | Omitted FileOmission | |
| 512 | + | Hunks []Hunk | |
| 513 | + | } | |
| 514 | + | ||
| 515 | + | // CommentPath is the path a comment designates for f, as Diff.HasLine reads it: | |
| 516 | + | // the old path of a deleted file, the new path otherwise. | |
| 517 | + | func (f FileDiff) CommentPath() string { | |
| 518 | + | if f.Status == FileDeleted { | |
| 519 | + | return f.OldPath | |
| 520 | + | } | |
| 521 | + | return f.NewPath | |
| 522 | + | } | |
| 523 | + | ||
| 524 | + | // FileOmission tells why a changed file has no hunks. | |
| 525 | + | type FileOmission uint8 | |
| 526 | + | ||
| 527 | + | // File omissions. | |
| 528 | + | const ( | |
| 529 | + | OmitNone FileOmission = iota | |
| 530 | + | OmitGenerated // SkipGenerated and IsGeneratedPath: hunks not read | |
| 531 | + | OmitLarge // changed lines > MaxFileLines | |
| 532 | + | OmitBudget // did not fit in the remaining MaxTotalLines | |
| 533 | + | OmitUnavailable // retained text would exceed MaxTotalBytes | |
| 534 | + | ) | |
| 535 | + | ||
| 536 | + | // Collapsed reports a file the web view folds and can load on its own. | |
| 537 | + | func (o FileOmission) Collapsed() bool { | |
| 538 | + | return o == OmitGenerated || o == OmitLarge || o == OmitBudget | |
| 513 | 539 | } | |
| 514 | 540 | ||
| 515 | 541 | // Diff is the structured difference a commit introduces against its first | |
| @@ -518,9 +544,8 @@ type Diff struct { | |||
| 518 | 544 | // Files are ordered as Git reports them (by path). Files past | |
| 519 | 545 | // DiffLimits.MaxFiles (when it is not 0) are left out. | |
| 520 | 546 | Files []FileDiff | |
| 521 | - | // Truncated is true when files were left out or a listed file is truncated. | |
| 522 | - | // Files listed without hunks because they are outside the DiffLimits window | |
| 523 | - | // do not make it true. | |
| 547 | + | // Truncated is true when files past DiffLimits.MaxFiles were left out. It | |
| 548 | + | // says nothing of the listed files: see FileDiff.Omitted. | |
| 524 | 549 | Truncated bool | |
| 525 | 550 | } | |
| 526 | 551 | ||
| @@ -537,34 +562,44 @@ type CommitDetail struct { | |||
| 537 | 562 | Diff Diff | |
| 538 | 563 | } | |
| 539 | 564 | ||
| 540 | - | // DiffLimits bound the size of a Diff. Line limits count additions plus deletions, not context. | |
| 565 | + | // DiffLimits bound the size of a Diff. Line limits count additions plus | |
| 566 | + | // deletions, not context. A file that does not fit is listed without hunks and | |
| 567 | + | // with an Omitted reason. | |
| 541 | 568 | type DiffLimits struct { | |
| 542 | 569 | // MaxFiles is the number of files kept; 0 means no limit on the number of | |
| 543 | 570 | // files (every changed file is listed). | |
| 544 | 571 | MaxFiles int | |
| 545 | 572 | // MaxFileLines is the most changed lines one file may have; a file over it is | |
| 546 | - | // truncated. | |
| 573 | + | // OmitLarge. 0 means no limit. | |
| 547 | 574 | MaxFileLines int | |
| 548 | - | // MaxTotalLines is the most changed lines of all the files. A file is truncated | |
| 549 | - | // when it does not fit in what remains; a smaller one after it may still | |
| 550 | - | // fit. | |
| 575 | + | // MaxTotalLines is the most changed lines of all the files. A file is | |
| 576 | + | // OmitBudget when it does not fit in what remains; a smaller one after it | |
| 577 | + | // may still fit. 0 means no limit. | |
| 551 | 578 | MaxTotalLines int | |
| 552 | 579 | // MaxTotalBytes is the most text, in bytes, of all the lines, including context. It bounds | |
| 553 | - | // the memory of a diff whose lines are very long; files are truncated | |
| 554 | - | // like for MaxTotalLines. | |
| 580 | + | // the memory of a diff whose lines are very long; a file whose | |
| 581 | + | // text does not fit is OmitUnavailable. | |
| 555 | 582 | MaxTotalBytes int | |
| 556 | 583 | // From and To select the files whose hunks are read: those with an index in | |
| 557 | 584 | // [From, To), counted from 0 in the order of Diff.Files. The other files | |
| 558 | 585 | // are listed (paths, status, modes, ids, Binary, Additions, Deletions, | |
| 559 | - | // Similarity) without hunks; they are not Truncated and use none of | |
| 586 | + | // Similarity) without hunks; they are OmitNone and use none of | |
| 560 | 587 | // MaxTotalLines and MaxTotalBytes. To == 0 means no end, so the zero values | |
| 561 | 588 | // select every file. MaxFileLines applies to each file of the window. | |
| 562 | 589 | From, To int | |
| 590 | + | // SkipGenerated omits the files of the window that IsGeneratedPath | |
| 591 | + | // recognizes (OmitGenerated), without reading their hunks. | |
| 592 | + | SkipGenerated bool | |
| 593 | + | } | |
| 594 | + | ||
| 595 | + | // OneFileDiffLimits reads the hunks of file i only, with no line cap and MaxTotalBytes as the sole guard. | |
| 596 | + | func OneFileDiffLimits(i int) DiffLimits { | |
| 597 | + | return DiffLimits{From: i, To: i + 1, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes} | |
| 563 | 598 | } | |
| 564 | 599 | ||
| 565 | 600 | // DefaultDiffLimits are the limits of the API contract: 300 files, 2,000 | |
| 566 | 601 | // changed lines per file, 20,000 changed lines in total, plus 8 MiB of text | |
| 567 | - | // including context. | |
| 602 | + | // including context. Generated files are not skipped. | |
| 568 | 603 | func DefaultDiffLimits() DiffLimits { | |
| 569 | 604 | return DiffLimits{MaxFiles: 300, MaxFileLines: 2000, MaxTotalLines: 20000, MaxTotalBytes: 8 << 20} | |
| 570 | 605 | } | |
| @@ -505,11 +505,37 @@ type FileDiff struct { | |||||
| 505 | Additions, Deletions int | 505 | Additions, Deletions int | ||
| 506 | // Similarity is a percentage for a renamed or copied file, 0 otherwise. | 506 | // Similarity is a percentage for a renamed or copied file, 0 otherwise. | ||
| 507 | Similarity int | 507 | Similarity int | ||
| 508 | - | // Truncated means the file exceeded a diff limit: Hunks is empty although | 508 | + | // Omitted tells why the hunks were not kept: Hunks is empty although the |
| 509 | - | // the file changed. Additions and Deletions stay accurate. A file outside | 509 | + | // file changed; Additions and Deletions stay accurate. A file outside the |
| 510 | - | // the DiffLimits window has no Hunks and is not Truncated. | 510 | + | // window is OmitNone with no Hunks. |
| 511 | - | Truncated bool | 511 | + | Omitted FileOmission |
| 512 | - | Hunks []Hunk | 512 | + | Hunks []Hunk |
| 513 | + | } | |||
| 514 | + | ||||
| 515 | + | // CommentPath is the path a comment designates for f, as Diff.HasLine reads it: | |||
| 516 | + | // the old path of a deleted file, the new path otherwise. | |||
| 517 | + | func (f FileDiff) CommentPath() string { | |||
| 518 | + | if f.Status == FileDeleted { | |||
| 519 | + | return f.OldPath | |||
| 520 | + | } | |||
| 521 | + | return f.NewPath | |||
| 522 | + | } | |||
| 523 | + | ||||
| 524 | + | // FileOmission tells why a changed file has no hunks. | |||
| 525 | + | type FileOmission uint8 | |||
| 526 | + | ||||
| 527 | + | // File omissions. | |||
| 528 | + | const ( | |||
| 529 | + | OmitNone FileOmission = iota | |||
| 530 | + | OmitGenerated // SkipGenerated and IsGeneratedPath: hunks not read | |||
| 531 | + | OmitLarge // changed lines > MaxFileLines | |||
| 532 | + | OmitBudget // did not fit in the remaining MaxTotalLines | |||
| 533 | + | OmitUnavailable // retained text would exceed MaxTotalBytes | |||
| 534 | + | ) | |||
| 535 | + | ||||
| 536 | + | // Collapsed reports a file the web view folds and can load on its own. | |||
| 537 | + | func (o FileOmission) Collapsed() bool { | |||
| 538 | + | return o == OmitGenerated || o == OmitLarge || o == OmitBudget | |||
| 513 | } | 539 | } | ||
| 514 | 540 | ||||
| 515 | // Diff is the structured difference a commit introduces against its first | 541 | // Diff is the structured difference a commit introduces against its first | ||
| @@ -518,9 +544,8 @@ type Diff struct { | |||||
| 518 | // Files are ordered as Git reports them (by path). Files past | 544 | // Files are ordered as Git reports them (by path). Files past | ||
| 519 | // DiffLimits.MaxFiles (when it is not 0) are left out. | 545 | // DiffLimits.MaxFiles (when it is not 0) are left out. | ||
| 520 | Files []FileDiff | 546 | Files []FileDiff | ||
| 521 | - | // Truncated is true when files were left out or a listed file is truncated. | 547 | + | // Truncated is true when files past DiffLimits.MaxFiles were left out. It |
| 522 | - | // Files listed without hunks because they are outside the DiffLimits window | 548 | + | // says nothing of the listed files: see FileDiff.Omitted. |
| 523 | - | // do not make it true. | |||
| 524 | Truncated bool | 549 | Truncated bool | ||
| 525 | } | 550 | } | ||
| 526 | 551 | ||||
| @@ -537,34 +562,44 @@ type CommitDetail struct { | |||||
| 537 | Diff Diff | 562 | Diff Diff | ||
| 538 | } | 563 | } | ||
| 539 | 564 | ||||
| 540 | - | // DiffLimits bound the size of a Diff. Line limits count additions plus deletions, not context. | 565 | + | // DiffLimits bound the size of a Diff. Line limits count additions plus |
| 566 | + | // deletions, not context. A file that does not fit is listed without hunks and | |||
| 567 | + | // with an Omitted reason. | |||
| 541 | type DiffLimits struct { | 568 | type DiffLimits struct { | ||
| 542 | // MaxFiles is the number of files kept; 0 means no limit on the number of | 569 | // MaxFiles is the number of files kept; 0 means no limit on the number of | ||
| 543 | // files (every changed file is listed). | 570 | // files (every changed file is listed). | ||
| 544 | MaxFiles int | 571 | MaxFiles int | ||
| 545 | // MaxFileLines is the most changed lines one file may have; a file over it is | 572 | // MaxFileLines is the most changed lines one file may have; a file over it is | ||
| 546 | - | // truncated. | 573 | + | // OmitLarge. 0 means no limit. |
| 547 | MaxFileLines int | 574 | MaxFileLines int | ||
| 548 | - | // MaxTotalLines is the most changed lines of all the files. A file is truncated | 575 | + | // MaxTotalLines is the most changed lines of all the files. A file is |
| 549 | - | // when it does not fit in what remains; a smaller one after it may still | 576 | + | // OmitBudget when it does not fit in what remains; a smaller one after it |
| 550 | - | // fit. | 577 | + | // may still fit. 0 means no limit. |
| 551 | MaxTotalLines int | 578 | MaxTotalLines int | ||
| 552 | // MaxTotalBytes is the most text, in bytes, of all the lines, including context. It bounds | 579 | // MaxTotalBytes is the most text, in bytes, of all the lines, including context. It bounds | ||
| 553 | - | // the memory of a diff whose lines are very long; files are truncated | 580 | + | // the memory of a diff whose lines are very long; a file whose |
| 554 | - | // like for MaxTotalLines. | 581 | + | // text does not fit is OmitUnavailable. |
| 555 | MaxTotalBytes int | 582 | MaxTotalBytes int | ||
| 556 | // From and To select the files whose hunks are read: those with an index in | 583 | // From and To select the files whose hunks are read: those with an index in | ||
| 557 | // [From, To), counted from 0 in the order of Diff.Files. The other files | 584 | // [From, To), counted from 0 in the order of Diff.Files. The other files | ||
| 558 | // are listed (paths, status, modes, ids, Binary, Additions, Deletions, | 585 | // are listed (paths, status, modes, ids, Binary, Additions, Deletions, | ||
| 559 | - | // Similarity) without hunks; they are not Truncated and use none of | 586 | + | // Similarity) without hunks; they are OmitNone and use none of |
| 560 | // MaxTotalLines and MaxTotalBytes. To == 0 means no end, so the zero values | 587 | // MaxTotalLines and MaxTotalBytes. To == 0 means no end, so the zero values | ||
| 561 | // select every file. MaxFileLines applies to each file of the window. | 588 | // select every file. MaxFileLines applies to each file of the window. | ||
| 562 | From, To int | 589 | From, To int | ||
| 590 | + | // SkipGenerated omits the files of the window that IsGeneratedPath | |||
| 591 | + | // recognizes (OmitGenerated), without reading their hunks. | |||
| 592 | + | SkipGenerated bool | |||
| 593 | + | } | |||
| 594 | + | ||||
| 595 | + | // OneFileDiffLimits reads the hunks of file i only, with no line cap and MaxTotalBytes as the sole guard. | |||
| 596 | + | func OneFileDiffLimits(i int) DiffLimits { | |||
| 597 | + | return DiffLimits{From: i, To: i + 1, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes} | |||
| 563 | } | 598 | } | ||
| 564 | 599 | ||||
| 565 | // DefaultDiffLimits are the limits of the API contract: 300 files, 2,000 | 600 | // DefaultDiffLimits are the limits of the API contract: 300 files, 2,000 | ||
| 566 | // changed lines per file, 20,000 changed lines in total, plus 8 MiB of text | 601 | // changed lines per file, 20,000 changed lines in total, plus 8 MiB of text | ||
| 567 | - | // including context. | 602 | + | // including context. Generated files are not skipped. |
| 568 | func DefaultDiffLimits() DiffLimits { | 603 | func DefaultDiffLimits() DiffLimits { | ||
| 569 | return DiffLimits{MaxFiles: 300, MaxFileLines: 2000, MaxTotalLines: 20000, MaxTotalBytes: 8 << 20} | 604 | return DiffLimits{MaxFiles: 300, MaxFileLines: 2000, MaxTotalLines: 20000, MaxTotalBytes: 8 << 20} | ||
| 570 | } | 605 | } | ||
M internal/domain/mergerequest_test.go +1 -1
| @@ -77,7 +77,7 @@ func hunkDiff() domain.Diff { | |||
| 77 | 77 | {OldPath: "gone.txt", Status: domain.FileDeleted, Hunks: []domain.Hunk{{ | |
| 78 | 78 | Lines: []domain.Line{{Kind: domain.LineDeletion, OldNumber: 1}}, | |
| 79 | 79 | }}}, | |
| 80 | - | {NewPath: "big.txt", Status: domain.FileAdded, Truncated: true}, | |
| 80 | + | {NewPath: "big.txt", Status: domain.FileAdded, Omitted: domain.OmitLarge}, | |
| 81 | 81 | {OldPath: "old.txt", NewPath: "new.txt", Status: domain.FileRenamed, Hunks: []domain.Hunk{{ | |
| 82 | 82 | Lines: []domain.Line{{Kind: domain.LineAddition, NewNumber: 2}}, | |
| 83 | 83 | }}}, | |
| @@ -77,7 +77,7 @@ func hunkDiff() domain.Diff { | |||||
| 77 | {OldPath: "gone.txt", Status: domain.FileDeleted, Hunks: []domain.Hunk{{ | 77 | {OldPath: "gone.txt", Status: domain.FileDeleted, Hunks: []domain.Hunk{{ | ||
| 78 | Lines: []domain.Line{{Kind: domain.LineDeletion, OldNumber: 1}}, | 78 | Lines: []domain.Line{{Kind: domain.LineDeletion, OldNumber: 1}}, | ||
| 79 | }}}, | 79 | }}}, | ||
| 80 | - | {NewPath: "big.txt", Status: domain.FileAdded, Truncated: true}, | 80 | + | {NewPath: "big.txt", Status: domain.FileAdded, Omitted: domain.OmitLarge}, |
| 81 | {OldPath: "old.txt", NewPath: "new.txt", Status: domain.FileRenamed, Hunks: []domain.Hunk{{ | 81 | {OldPath: "old.txt", NewPath: "new.txt", Status: domain.FileRenamed, Hunks: []domain.Hunk{{ | ||
| 82 | Lines: []domain.Line{{Kind: domain.LineAddition, NewNumber: 2}}, | 82 | Lines: []domain.Line{{Kind: domain.LineAddition, NewNumber: 2}}, | ||
| 83 | }}}, | 83 | }}}, | ||
M internal/readertest/conformance.go +1 -0
| @@ -32,6 +32,7 @@ func Conformance(t *testing.T, mk Maker) { | |||
| 32 | 32 | fn func(*testing.T, Maker) | |
| 33 | 33 | }{ | |
| 34 | 34 | {"DiffChangedLineLimits", DiffChangedLineLimits}, | |
| 35 | + | {"DiffOmissions", DiffOmissions}, | |
| 35 | 36 | {"ListRefs", testListRefs}, | |
| 36 | 37 | {"ListRefsEmptyAndMissing", testListRefsEmptyAndMissing}, | |
| 37 | 38 | {"DetachedHead", testDetachedHead}, | |
| @@ -32,6 +32,7 @@ func Conformance(t *testing.T, mk Maker) { | |||||
| 32 | fn func(*testing.T, Maker) | 32 | fn func(*testing.T, Maker) | ||
| 33 | }{ | 33 | }{ | ||
| 34 | {"DiffChangedLineLimits", DiffChangedLineLimits}, | 34 | {"DiffChangedLineLimits", DiffChangedLineLimits}, | ||
| 35 | + | {"DiffOmissions", DiffOmissions}, | |||
| 35 | {"ListRefs", testListRefs}, | 36 | {"ListRefs", testListRefs}, | ||
| 36 | {"ListRefsEmptyAndMissing", testListRefsEmptyAndMissing}, | 37 | {"ListRefsEmptyAndMissing", testListRefsEmptyAndMissing}, | ||
| 37 | {"DetachedHead", testDetachedHead}, | 38 | {"DetachedHead", testDetachedHead}, | ||
M internal/readertest/diff.go +83 -4
| @@ -36,7 +36,7 @@ func DiffChangedLineLimits(t *testing.T, mk Maker) { | |||
| 36 | 36 | require.NoError(t, err) | |
| 37 | 37 | require.Len(t, d.Diff.Files, 1) | |
| 38 | 38 | file := d.Diff.Files[0] | |
| 39 | - | require.False(t, file.Truncated) | |
| 39 | + | require.Equal(t, domain.OmitNone, file.Omitted) | |
| 40 | 40 | assert.False(t, d.Diff.Truncated) | |
| 41 | 41 | require.Len(t, file.Hunks, hunks) | |
| 42 | 42 | var changed, patchLines, textBytes, changedBytes int | |
| @@ -51,6 +51,10 @@ func DiffChangedLineLimits(t *testing.T, mk Maker) { | |||
| 51 | 51 | } | |
| 52 | 52 | } | |
| 53 | 53 | assert.Equal(t, hunks*2, changed) | |
| 54 | + | // Every hunk is read with no line limit too. | |
| 55 | + | free, err := g.GetCommit(context.Background(), f.Name, domain.Revision{}, domain.DiffLimits{MaxTotalBytes: 1 << 20}) | |
| 56 | + | require.NoError(t, err) | |
| 57 | + | require.Len(t, free.Diff.Files[0].Hunks, hunks) | |
| 54 | 58 | assert.Greater(t, patchLines, limits.MaxFileLines) | |
| 55 | 59 | assert.Equal(t, hunks, file.Additions) | |
| 56 | 60 | assert.Equal(t, hunks, file.Deletions) | |
| @@ -59,12 +63,87 @@ func DiffChangedLineLimits(t *testing.T, mk Maker) { | |||
| 59 | 63 | limits.MaxTotalBytes = textBytes | |
| 60 | 64 | d, err = g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits) | |
| 61 | 65 | require.NoError(t, err) | |
| 62 | - | assert.False(t, d.Diff.Files[0].Truncated) | |
| 66 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted) | |
| 63 | 67 | limits.MaxTotalBytes-- | |
| 64 | 68 | require.Greater(t, limits.MaxTotalBytes, changedBytes) | |
| 65 | 69 | d, err = g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits) | |
| 66 | 70 | require.NoError(t, err) | |
| 67 | - | assert.True(t, d.Diff.Truncated) | |
| 68 | - | assert.True(t, d.Diff.Files[0].Truncated) | |
| 71 | + | assert.False(t, d.Diff.Truncated, "only MaxFiles sets Diff.Truncated") | |
| 72 | + | assert.Equal(t, domain.OmitUnavailable, d.Diff.Files[0].Omitted) | |
| 69 | 73 | assert.Empty(t, d.Diff.Files[0].Hunks) | |
| 70 | 74 | } | |
| 75 | + | ||
| 76 | + | // DiffOmissions checks the omission reasons: generated files skipped without | |
| 77 | + | // reading their hunks, one file read without a line cap, and the MaxFiles | |
| 78 | + | // overflow being the only thing that sets Diff.Truncated. | |
| 79 | + | func DiffOmissions(t *testing.T, mk Maker) { | |
| 80 | + | g, data := mk(t) | |
| 81 | + | ctx := context.Background() | |
| 82 | + | f := NewFixture(t, data, "diff-omissions") | |
| 83 | + | f.Write("base.txt", "x\n").Commit("base") | |
| 84 | + | f.Write("a.txt", "small\n") | |
| 85 | + | // The generated file (515 bytes) would overflow the 300-byte budget below | |
| 86 | + | // if it were read: OmitGenerated, not OmitUnavailable, proves its hunks | |
| 87 | + | // are not read. | |
| 88 | + | f.Write("go.sum", strings.Repeat("h1:"+strings.Repeat("z", 100)+"\n", 5)) | |
| 89 | + | f.Lines("z-big.txt", "line", 2500) | |
| 90 | + | f.Commit("change") | |
| 91 | + | ||
| 92 | + | limits := domain.DiffLimits{MaxTotalBytes: 1 << 20, MaxFileLines: 2000, SkipGenerated: true} | |
| 93 | + | limits.MaxTotalBytes = 300 | |
| 94 | + | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, limits) | |
| 95 | + | require.NoError(t, err) | |
| 96 | + | require.Len(t, d.Diff.Files, 3) | |
| 97 | + | byPath := map[string]domain.FileDiff{} | |
| 98 | + | for _, file := range d.Diff.Files { | |
| 99 | + | byPath[file.NewPath] = file | |
| 100 | + | } | |
| 101 | + | assert.Equal(t, domain.OmitNone, byPath["a.txt"].Omitted) | |
| 102 | + | assert.Len(t, byPath["a.txt"].Hunks, 1) | |
| 103 | + | assert.Equal(t, domain.OmitGenerated, byPath["go.sum"].Omitted) | |
| 104 | + | assert.Empty(t, byPath["go.sum"].Hunks) | |
| 105 | + | assert.Equal(t, 5, byPath["go.sum"].Additions) | |
| 106 | + | assert.Equal(t, domain.OmitLarge, byPath["z-big.txt"].Omitted) | |
| 107 | + | assert.Empty(t, byPath["z-big.txt"].Hunks) | |
| 108 | + | assert.False(t, d.Diff.Truncated) | |
| 109 | + | ||
| 110 | + | limits.SkipGenerated = false | |
| 111 | + | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits) | |
| 112 | + | require.NoError(t, err) | |
| 113 | + | for _, file := range d.Diff.Files { | |
| 114 | + | if file.NewPath == "go.sum" { | |
| 115 | + | assert.Equal(t, domain.OmitUnavailable, file.Omitted, "read, it overflows the bytes") | |
| 116 | + | assert.Empty(t, file.Hunks) | |
| 117 | + | } | |
| 118 | + | } | |
| 119 | + | ||
| 120 | + | // OneFileDiffLimits reads a file over 2,000 changed lines, and only it. | |
| 121 | + | idx := -1 | |
| 122 | + | for i, file := range d.Diff.Files { | |
| 123 | + | if file.NewPath == "z-big.txt" { | |
| 124 | + | idx = i | |
| 125 | + | } | |
| 126 | + | } | |
| 127 | + | require.GreaterOrEqual(t, idx, 0) | |
| 128 | + | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.OneFileDiffLimits(idx)) | |
| 129 | + | require.NoError(t, err) | |
| 130 | + | for i, file := range d.Diff.Files { | |
| 131 | + | assert.Equal(t, domain.OmitNone, file.Omitted) | |
| 132 | + | if i == idx { | |
| 133 | + | require.Len(t, file.Hunks, 1) | |
| 134 | + | assert.Len(t, file.Hunks[0].Lines, 2500) | |
| 135 | + | } else { | |
| 136 | + | assert.Empty(t, file.Hunks) | |
| 137 | + | } | |
| 138 | + | } | |
| 139 | + | assert.False(t, d.Diff.Truncated) | |
| 140 | + | ||
| 141 | + | // MaxFiles alone sets Diff.Truncated. | |
| 142 | + | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxFiles: 2, MaxTotalBytes: 1 << 20}) | |
| 143 | + | require.NoError(t, err) | |
| 144 | + | assert.Len(t, d.Diff.Files, 2) | |
| 145 | + | assert.True(t, d.Diff.Truncated) | |
| 146 | + | for _, file := range d.Diff.Files { | |
| 147 | + | assert.Equal(t, domain.OmitNone, file.Omitted) | |
| 148 | + | } | |
| 149 | + | } | |
| @@ -36,7 +36,7 @@ func DiffChangedLineLimits(t *testing.T, mk Maker) { | |||||
| 36 | require.NoError(t, err) | 36 | require.NoError(t, err) | ||
| 37 | require.Len(t, d.Diff.Files, 1) | 37 | require.Len(t, d.Diff.Files, 1) | ||
| 38 | file := d.Diff.Files[0] | 38 | file := d.Diff.Files[0] | ||
| 39 | - | require.False(t, file.Truncated) | 39 | + | require.Equal(t, domain.OmitNone, file.Omitted) |
| 40 | assert.False(t, d.Diff.Truncated) | 40 | assert.False(t, d.Diff.Truncated) | ||
| 41 | require.Len(t, file.Hunks, hunks) | 41 | require.Len(t, file.Hunks, hunks) | ||
| 42 | var changed, patchLines, textBytes, changedBytes int | 42 | var changed, patchLines, textBytes, changedBytes int | ||
| @@ -51,6 +51,10 @@ func DiffChangedLineLimits(t *testing.T, mk Maker) { | |||||
| 51 | } | 51 | } | ||
| 52 | } | 52 | } | ||
| 53 | assert.Equal(t, hunks*2, changed) | 53 | assert.Equal(t, hunks*2, changed) | ||
| 54 | + | // Every hunk is read with no line limit too. | |||
| 55 | + | free, err := g.GetCommit(context.Background(), f.Name, domain.Revision{}, domain.DiffLimits{MaxTotalBytes: 1 << 20}) | |||
| 56 | + | require.NoError(t, err) | |||
| 57 | + | require.Len(t, free.Diff.Files[0].Hunks, hunks) | |||
| 54 | assert.Greater(t, patchLines, limits.MaxFileLines) | 58 | assert.Greater(t, patchLines, limits.MaxFileLines) | ||
| 55 | assert.Equal(t, hunks, file.Additions) | 59 | assert.Equal(t, hunks, file.Additions) | ||
| 56 | assert.Equal(t, hunks, file.Deletions) | 60 | assert.Equal(t, hunks, file.Deletions) | ||
| @@ -59,12 +63,87 @@ func DiffChangedLineLimits(t *testing.T, mk Maker) { | |||||
| 59 | limits.MaxTotalBytes = textBytes | 63 | limits.MaxTotalBytes = textBytes | ||
| 60 | d, err = g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits) | 64 | d, err = g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits) | ||
| 61 | require.NoError(t, err) | 65 | require.NoError(t, err) | ||
| 62 | - | assert.False(t, d.Diff.Files[0].Truncated) | 66 | + | assert.Equal(t, domain.OmitNone, d.Diff.Files[0].Omitted) |
| 63 | limits.MaxTotalBytes-- | 67 | limits.MaxTotalBytes-- | ||
| 64 | require.Greater(t, limits.MaxTotalBytes, changedBytes) | 68 | require.Greater(t, limits.MaxTotalBytes, changedBytes) | ||
| 65 | d, err = g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits) | 69 | d, err = g.GetCommit(context.Background(), f.Name, domain.Revision{}, limits) | ||
| 66 | require.NoError(t, err) | 70 | require.NoError(t, err) | ||
| 67 | - | assert.True(t, d.Diff.Truncated) | 71 | + | assert.False(t, d.Diff.Truncated, "only MaxFiles sets Diff.Truncated") |
| 68 | - | assert.True(t, d.Diff.Files[0].Truncated) | 72 | + | assert.Equal(t, domain.OmitUnavailable, d.Diff.Files[0].Omitted) |
| 69 | assert.Empty(t, d.Diff.Files[0].Hunks) | 73 | assert.Empty(t, d.Diff.Files[0].Hunks) | ||
| 70 | } | 74 | } | ||
| 75 | + | ||||
| 76 | + | // DiffOmissions checks the omission reasons: generated files skipped without | |||
| 77 | + | // reading their hunks, one file read without a line cap, and the MaxFiles | |||
| 78 | + | // overflow being the only thing that sets Diff.Truncated. | |||
| 79 | + | func DiffOmissions(t *testing.T, mk Maker) { | |||
| 80 | + | g, data := mk(t) | |||
| 81 | + | ctx := context.Background() | |||
| 82 | + | f := NewFixture(t, data, "diff-omissions") | |||
| 83 | + | f.Write("base.txt", "x\n").Commit("base") | |||
| 84 | + | f.Write("a.txt", "small\n") | |||
| 85 | + | // The generated file (515 bytes) would overflow the 300-byte budget below | |||
| 86 | + | // if it were read: OmitGenerated, not OmitUnavailable, proves its hunks | |||
| 87 | + | // are not read. | |||
| 88 | + | f.Write("go.sum", strings.Repeat("h1:"+strings.Repeat("z", 100)+"\n", 5)) | |||
| 89 | + | f.Lines("z-big.txt", "line", 2500) | |||
| 90 | + | f.Commit("change") | |||
| 91 | + | ||||
| 92 | + | limits := domain.DiffLimits{MaxTotalBytes: 1 << 20, MaxFileLines: 2000, SkipGenerated: true} | |||
| 93 | + | limits.MaxTotalBytes = 300 | |||
| 94 | + | d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, limits) | |||
| 95 | + | require.NoError(t, err) | |||
| 96 | + | require.Len(t, d.Diff.Files, 3) | |||
| 97 | + | byPath := map[string]domain.FileDiff{} | |||
| 98 | + | for _, file := range d.Diff.Files { | |||
| 99 | + | byPath[file.NewPath] = file | |||
| 100 | + | } | |||
| 101 | + | assert.Equal(t, domain.OmitNone, byPath["a.txt"].Omitted) | |||
| 102 | + | assert.Len(t, byPath["a.txt"].Hunks, 1) | |||
| 103 | + | assert.Equal(t, domain.OmitGenerated, byPath["go.sum"].Omitted) | |||
| 104 | + | assert.Empty(t, byPath["go.sum"].Hunks) | |||
| 105 | + | assert.Equal(t, 5, byPath["go.sum"].Additions) | |||
| 106 | + | assert.Equal(t, domain.OmitLarge, byPath["z-big.txt"].Omitted) | |||
| 107 | + | assert.Empty(t, byPath["z-big.txt"].Hunks) | |||
| 108 | + | assert.False(t, d.Diff.Truncated) | |||
| 109 | + | ||||
| 110 | + | limits.SkipGenerated = false | |||
| 111 | + | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits) | |||
| 112 | + | require.NoError(t, err) | |||
| 113 | + | for _, file := range d.Diff.Files { | |||
| 114 | + | if file.NewPath == "go.sum" { | |||
| 115 | + | assert.Equal(t, domain.OmitUnavailable, file.Omitted, "read, it overflows the bytes") | |||
| 116 | + | assert.Empty(t, file.Hunks) | |||
| 117 | + | } | |||
| 118 | + | } | |||
| 119 | + | ||||
| 120 | + | // OneFileDiffLimits reads a file over 2,000 changed lines, and only it. | |||
| 121 | + | idx := -1 | |||
| 122 | + | for i, file := range d.Diff.Files { | |||
| 123 | + | if file.NewPath == "z-big.txt" { | |||
| 124 | + | idx = i | |||
| 125 | + | } | |||
| 126 | + | } | |||
| 127 | + | require.GreaterOrEqual(t, idx, 0) | |||
| 128 | + | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.OneFileDiffLimits(idx)) | |||
| 129 | + | require.NoError(t, err) | |||
| 130 | + | for i, file := range d.Diff.Files { | |||
| 131 | + | assert.Equal(t, domain.OmitNone, file.Omitted) | |||
| 132 | + | if i == idx { | |||
| 133 | + | require.Len(t, file.Hunks, 1) | |||
| 134 | + | assert.Len(t, file.Hunks[0].Lines, 2500) | |||
| 135 | + | } else { | |||
| 136 | + | assert.Empty(t, file.Hunks) | |||
| 137 | + | } | |||
| 138 | + | } | |||
| 139 | + | assert.False(t, d.Diff.Truncated) | |||
| 140 | + | ||||
| 141 | + | // MaxFiles alone sets Diff.Truncated. | |||
| 142 | + | d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, domain.DiffLimits{MaxFiles: 2, MaxTotalBytes: 1 << 20}) | |||
| 143 | + | require.NoError(t, err) | |||
| 144 | + | assert.Len(t, d.Diff.Files, 2) | |||
| 145 | + | assert.True(t, d.Diff.Truncated) | |||
| 146 | + | for _, file := range d.Diff.Files { | |||
| 147 | + | assert.Equal(t, domain.OmitNone, file.Omitted) | |||
| 148 | + | } | |||
| 149 | + | } | |||
M internal/usecase/ci_pull_test.go +1 -1
| @@ -107,7 +107,7 @@ func TestFilePatch(t *testing.T) { | |||
| 107 | 107 | assert.Equal(t, "@@ -1,2 +1,2 @@ func x()\n a\n-b\n+c\n\\ No newline at end of file\n@@ -9 +9 @@\n+z", got) | |
| 108 | 108 | _, ok = usecase.FilePatch(domain.FileDiff{Binary: true, Hunks: f.Hunks}) | |
| 109 | 109 | assert.False(t, ok, "binary") | |
| 110 | - | _, ok = usecase.FilePatch(domain.FileDiff{Truncated: true}) | |
| 110 | + | _, ok = usecase.FilePatch(domain.FileDiff{Omitted: domain.OmitLarge}) | |
| 111 | 111 | assert.False(t, ok, "truncated") | |
| 112 | 112 | _, ok = usecase.FilePatch(domain.FileDiff{Status: domain.FileRenamed}) | |
| 113 | 113 | assert.False(t, ok, "a pure rename has no hunk") | |
| @@ -107,7 +107,7 @@ func TestFilePatch(t *testing.T) { | |||||
| 107 | assert.Equal(t, "@@ -1,2 +1,2 @@ func x()\n a\n-b\n+c\n\\ No newline at end of file\n@@ -9 +9 @@\n+z", got) | 107 | assert.Equal(t, "@@ -1,2 +1,2 @@ func x()\n a\n-b\n+c\n\\ No newline at end of file\n@@ -9 +9 @@\n+z", got) | ||
| 108 | _, ok = usecase.FilePatch(domain.FileDiff{Binary: true, Hunks: f.Hunks}) | 108 | _, ok = usecase.FilePatch(domain.FileDiff{Binary: true, Hunks: f.Hunks}) | ||
| 109 | assert.False(t, ok, "binary") | 109 | assert.False(t, ok, "binary") | ||
| 110 | - | _, ok = usecase.FilePatch(domain.FileDiff{Truncated: true}) | 110 | + | _, ok = usecase.FilePatch(domain.FileDiff{Omitted: domain.OmitLarge}) |
| 111 | assert.False(t, ok, "truncated") | 111 | assert.False(t, ok, "truncated") | ||
| 112 | _, ok = usecase.FilePatch(domain.FileDiff{Status: domain.FileRenamed}) | 112 | _, ok = usecase.FilePatch(domain.FileDiff{Status: domain.FileRenamed}) | ||
| 113 | assert.False(t, ok, "a pure rename has no hunk") | 113 | assert.False(t, ok, "a pure rename has no hunk") | ||
M internal/usecase/ci_pullfiles.go +1 -1
| @@ -212,7 +212,7 @@ func escapeURLPath(p string) string { | |||
| 212 | 212 | // (hunk headers and lines, no file headers). Absent for a binary file, a pure | |
| 213 | 213 | // rename and a file too large for the diff limits, as on GitHub. | |
| 214 | 214 | func filePatch(f domain.FileDiff) (string, bool) { | |
| 215 | - | if f.Binary || f.Truncated || len(f.Hunks) == 0 { | |
| 215 | + | if f.Binary || f.Omitted != domain.OmitNone || len(f.Hunks) == 0 { | |
| 216 | 216 | return "", false | |
| 217 | 217 | } | |
| 218 | 218 | var b strings.Builder | |
| @@ -212,7 +212,7 @@ func escapeURLPath(p string) string { | |||||
| 212 | // (hunk headers and lines, no file headers). Absent for a binary file, a pure | 212 | // (hunk headers and lines, no file headers). Absent for a binary file, a pure | ||
| 213 | // rename and a file too large for the diff limits, as on GitHub. | 213 | // rename and a file too large for the diff limits, as on GitHub. | ||
| 214 | func filePatch(f domain.FileDiff) (string, bool) { | 214 | func filePatch(f domain.FileDiff) (string, bool) { | ||
| 215 | - | if f.Binary || f.Truncated || len(f.Hunks) == 0 { | 215 | + | if f.Binary || f.Omitted != domain.OmitNone || len(f.Hunks) == 0 { |
| 216 | return "", false | 216 | return "", false | ||
| 217 | } | 217 | } | ||
| 218 | var b strings.Builder | 218 | var b strings.Builder | ||
M internal/usecase/mergerequests.go +1 -9
| @@ -771,7 +771,7 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p | |||
| 771 | 771 | if err != nil { | |
| 772 | 772 | return err | |
| 773 | 773 | } | |
| 774 | - | idx := slices.IndexFunc(diff.Files, func(f domain.FileDiff) bool { return diffFileName(f) == params.Path }) | |
| 774 | + | idx := slices.IndexFunc(diff.Files, func(f domain.FileDiff) bool { return f.CommentPath() == params.Path }) | |
| 775 | 775 | if idx >= 0 { | |
| 776 | 776 | one := domain.DefaultDiffLimits() | |
| 777 | 777 | one.MaxFiles, one.From, one.To = 0, idx, idx+1 | |
| @@ -788,14 +788,6 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p | |||
| 788 | 788 | return v.Err() | |
| 789 | 789 | } | |
| 790 | 790 | ||
| 791 | - | // diffFileName is the path a comment designates for f, as Diff.HasLine reads it. | |
| 792 | - | func diffFileName(f domain.FileDiff) string { | |
| 793 | - | if f.Status == domain.FileDeleted { | |
| 794 | - | return f.OldPath | |
| 795 | - | } | |
| 796 | - | return f.NewPath | |
| 797 | - | } | |
| 798 | - | ||
| 799 | 791 | // ResolveComment marks a comment resolved or unresolved. The principal needs | |
| 800 | 792 | // mr:write. An unknown comment gives domain.ErrNotFound and a request that is | |
| 801 | 793 | // not open domain.ErrPreconditionFailed. Setting the state the comment already | |
| @@ -771,7 +771,7 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p | |||||
| 771 | if err != nil { | 771 | if err != nil { | ||
| 772 | return err | 772 | return err | ||
| 773 | } | 773 | } | ||
| 774 | - | idx := slices.IndexFunc(diff.Files, func(f domain.FileDiff) bool { return diffFileName(f) == params.Path }) | 774 | + | idx := slices.IndexFunc(diff.Files, func(f domain.FileDiff) bool { return f.CommentPath() == params.Path }) |
| 775 | if idx >= 0 { | 775 | if idx >= 0 { | ||
| 776 | one := domain.DefaultDiffLimits() | 776 | one := domain.DefaultDiffLimits() | ||
| 777 | one.MaxFiles, one.From, one.To = 0, idx, idx+1 | 777 | one.MaxFiles, one.From, one.To = 0, idx, idx+1 | ||
| @@ -788,14 +788,6 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p | |||||
| 788 | return v.Err() | 788 | return v.Err() | ||
| 789 | } | 789 | } | ||
| 790 | 790 | ||||
| 791 | - | // diffFileName is the path a comment designates for f, as Diff.HasLine reads it. | |||
| 792 | - | func diffFileName(f domain.FileDiff) string { | |||
| 793 | - | if f.Status == domain.FileDeleted { | |||
| 794 | - | return f.OldPath | |||
| 795 | - | } | |||
| 796 | - | return f.NewPath | |||
| 797 | - | } | |||
| 798 | - | ||||
| 799 | // ResolveComment marks a comment resolved or unresolved. The principal needs | 791 | // ResolveComment marks a comment resolved or unresolved. The principal needs | ||
| 800 | // mr:write. An unknown comment gives domain.ErrNotFound and a request that is | 792 | // mr:write. An unknown comment gives domain.ErrNotFound and a request that is | ||
| 801 | // not open domain.ErrPreconditionFailed. Setting the state the comment already | 793 | // not open domain.ErrPreconditionFailed. Setting the state the comment already | ||
M internal/usecase/mergerequests_files_test.go +1 -1
| @@ -63,7 +63,7 @@ func TestGetMergeRequestFilesWindow(t *testing.T) { | |||
| 63 | 63 | for i, f := range d.Diff.Files { | |
| 64 | 64 | assert.Equal(t, fmt.Sprintf("m/f%03d", i), f.NewPath) | |
| 65 | 65 | assert.Equal(t, 1, f.Additions) | |
| 66 | - | assert.False(t, f.Truncated) | |
| 66 | + | assert.Equal(t, domain.OmitNone, f.Omitted) | |
| 67 | 67 | if i >= 305 && i < 310 { | |
| 68 | 68 | require.Len(t, f.Hunks, 1, f.NewPath) | |
| 69 | 69 | assert.Equal(t, fmt.Sprintf("line %d", i), f.Hunks[0].Lines[0].Text) | |
| @@ -63,7 +63,7 @@ func TestGetMergeRequestFilesWindow(t *testing.T) { | |||||
| 63 | for i, f := range d.Diff.Files { | 63 | for i, f := range d.Diff.Files { | ||
| 64 | assert.Equal(t, fmt.Sprintf("m/f%03d", i), f.NewPath) | 64 | assert.Equal(t, fmt.Sprintf("m/f%03d", i), f.NewPath) | ||
| 65 | assert.Equal(t, 1, f.Additions) | 65 | assert.Equal(t, 1, f.Additions) | ||
| 66 | - | assert.False(t, f.Truncated) | 66 | + | assert.Equal(t, domain.OmitNone, f.Omitted) |
| 67 | if i >= 305 && i < 310 { | 67 | if i >= 305 && i < 310 { | ||
| 68 | require.Len(t, f.Hunks, 1, f.NewPath) | 68 | require.Len(t, f.Hunks, 1, f.NewPath) | ||
| 69 | assert.Equal(t, fmt.Sprintf("line %d", i), f.Hunks[0].Lines[0].Text) | 69 | assert.Equal(t, fmt.Sprintf("line %d", i), f.Hunks[0].Lines[0].Text) | ||