Skip to content

feat(diff): add a button to expand all hidden lines of a diff file - #38675

Open
lunny wants to merge 4 commits into
go-gitea:mainfrom
lunny:lunny/expand-full-file
Open

feat(diff): add a button to expand all hidden lines of a diff file#38675
lunny wants to merge 4 commits into
go-gitea:mainfrom
lunny:lunny/expand-full-file

Conversation

@lunny

@lunny lunny commented Jul 28, 2026

Copy link
Copy Markdown
Member

Adds a button to each diff file header that expands all of that file's hidden (unchanged) lines in one click, and collapses them again on a second click.

This is a re-implementation of #36663, which was rejected for duplicating backend code, for OOM/DoS risk, and for fragile frontend state. The approach here is deliberately different.

Expanding: one request per gap, no new backend logic

A file's hidden lines live in "gaps" between hunks, so a file has hunks + 1 gaps — typically 1-10, not the 200 chunked requests the earlier attempt made for a 4000-line file.

BuildBlobExcerptDiffSection already had a branch that fills an entire gap in one response; only up/down do the 20-line chunking. So direction=all needs no new logic, no new option field, and no batch endpoint — it just shares that existing branch with single.

The one backend change this needs is that the offset in that branch is now derived from the hunk sizes (leftHunkSize <= 0 && rightHunkSize <= 0, i.e. "this gap runs to the end of the file") instead of from direction == "down". This is equivalent for every request the UI can emit — down is only ever offered for gaps with both hunk sizes 0, and up/single only for gaps with at least one hunk size above 0 — and it makes "how much to expand" orthogonal to "which direction the button points".

Collapsing: re-render, don't snapshot

Collapsing re-fetches the file through the existing ?file-only=true URL rather than restoring saved innerHTML. That keeps it correct when the user had already expanded some gaps by hand, and there is no DOM snapshot to go stale. The "fetch a page, take its file body" logic is now a shared helper with the existing .diff-load-button path.

Memory safety

direction=all is the only path that can materialise an unbounded number of lines, so it rejects files at or above MaxDisplayFileSize. The excerpt itself still streams through bufio.Scanner — there is no io.ReadAll of the blob anywhere.

Included bug fix

The first commit fixes a pre-existing bug: prepareDiffRenderDetail wrote the file line counts into local variables instead of the DiffRenderDetail fields that addTailSection reads, so the tail section was always built with zero indices and GetExpandDirection never reported a direction for it. The "expand down to the end of the file" button therefore never rendered for any diff. Without this, "expand all lines" could never reach a file's tail. It is a separate commit so it can be backported on its own.

Notes

  • The button uses data-global-click / registerGlobalEventFunc, so it keeps working for files brought in by "load more files".
  • It unfolds the file first if it is folded (vendored/generated/already-viewed), otherwise the expansion would happen invisibly — a known bug of the earlier attempt.
  • It is not offered for files whose source diff is hidden behind a rendered view (CSV/TSV, SVG), for the same reason.
  • The collapse URL pins both style and whitespace, since neither is persisted for anonymous visitors.

Tests

  • Unit: direction=all for a leading gap, an interior gap, and an end-of-file gap, asserting exact line ranges, that no further expander row is emitted, and that highlighting still runs; plus a regression test for the tail-section fix that asserts GetExpandDirection() == "down" (verified red before the fix).
  • Integration: drives a real data-expand-all-url for both the leading and the end-of-file gap, asserting the full line range through the file's last line inclusive and that no expander button is left behind; plus the 413 guard and that the guard does not affect chunked requests.
  • E2E: expand, collapse back to the original row count, then fold and expand again to cover the unfold behaviour.

Screenshots

image image image

Generated by Codet

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 28, 2026
@github-actions github-actions Bot added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Jul 28, 2026
@lunny
lunny requested a review from Copilot July 28, 2026 02:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a per-diff-file “Expand all lines” toggle that expands every collapsed gap in that file (using one request per gap via direction=all) and collapses back by re-fetching the pristine ?file-only=true render, with backend safeguards and tests to avoid the earlier OOM/state issues.

Changes:

  • Frontend: add a diff header button that expands/collapses all hidden lines in a file; reuse shared HTML-fetch parsing for “load diff” and collapse.
  • Backend: support direction=all by reusing the existing full-gap fill path and add a size guard (413) for large blobs; fix tail-section line counts so “expand down” renders for tail gaps.
  • Tests: unit/integration/e2e coverage for full-gap expansion (leading/middle/tail), size guard behavior, and expand/collapse/unfold UX.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
web_src/js/svg.ts Adds new icon imports/registrations used by the expand-all toggle.
web_src/js/features/repo-diff.ts Implements expand/collapse-all behavior and registers the global click handler.
templates/repo/diff/box.tmpl Adds the expand-all toggle button to each diff file header (conditionally).
services/gitdiff/gitdiff.go Adds data-expand-all-url to gap elements; fixes tail-section line count propagation.
services/gitdiff/gitdiff_excerpt.go Adjusts full-gap excerpt fill offset semantics for direction=all/single vs end-of-file gaps.
routers/web/repo/compare.go Rejects direction=all expansions for blobs at/above MaxDisplayFileSize (413).
services/gitdiff/gitdiff_test.go Adds regression coverage for tail-section direction and verifies expand-all URL emission rules.
services/gitdiff/gitdiff_excerpt_test.go Adds unit tests for direction=all on leading/middle/end-of-file gaps.
tests/integration/compare_test.go Integration coverage for direction=all requests and the 413 guard behavior.
tests/e2e/utils.ts Adds an API helper to update a file (used by the new e2e test).
tests/e2e/diff-expand-all.test.ts E2E test covering expand, collapse, and unfold-then-expand behavior.
options/locale/locale_en-US.json Adds user-facing strings for expand/collapse-all tooltip/aria labels.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread web_src/js/svg.ts
Comment thread tests/e2e/utils.ts Outdated

@bircni bircni left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow the contribution guidelines and add a screenshot

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 28, 2026
lunny added 4 commits July 29, 2026 00:11
prepareDiffRenderDetail computed the file line counts into local
variables instead of the DiffRenderDetail fields that addTailSection
reads, so the tail section was always built with zero indices and
GetExpandDirection never reported a direction for it. As a result the
"expand down to the end of the file" button was never rendered.

Assisted-by: Codet
Each file header in a diff gets a button that fills in every hidden
(unchanged) line of that file; clicking it again restores the file to
its pristine rendering.

Expanding reuses the existing blob_excerpt endpoint with a new
"direction=all", which means "fill this whole gap in one response,
without the usual 20-line chunking". That direction shares the code
path "single" already used, so a file needs one request per gap
(hunks + 1) rather than one request per chunk. The offset derivation
in BuildBlobExcerptDiffSection now comes from the hunk sizes instead
of the direction, which is equivalent for every request the UI emits
and makes "how much to expand" orthogonal to the expand direction.

"direction=all" is the only path that materialises an unbounded number
of lines, so it refuses files at or above MaxDisplayFileSize.

Collapsing re-fetches the file with the existing "file-only" URL
instead of restoring a snapshot of the DOM, so it is correct even when
the user had already expanded some gaps by hand.

Assisted-by: Codet
The update API needs the current blob sha, but the GET fetching it was
neither retried nor status-checked: a transient 5xx or a 404 surfaced as
a confusing JSON parse error instead of the retry helper's message.
Wrap it like apiCreatePR does, reading the sha only when the response
is ok.

Assisted-by: Codet
…n test

OpenRepositoryLocal gained a cat-file batch context parameter in go-gitea#38684,
which broke the build of this package's tests after rebasing onto main.

Assisted-by: Codet
@lunny
lunny force-pushed the lunny/expand-full-file branch from 6aead8c to bdea3a6 Compare July 29, 2026 07:13
@lunny

lunny commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

Please follow the contribution guidelines and add a screenshot

Updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged type/feature Completely new functionality. Can only be merged if feature freeze is not active.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants