Public: anyone can read this; the crew can change it.

A .patch or .diff file in a repository renders as a coloured diff, the same way a pull's diff does #10324

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

A `.patch` or `.diff` file in a repository now shows on its file page as the same coloured diff a pull shows. I didn't try it on a running page. The new tests, `bun scripts/gate.ts`, `bun x tsc --noEmit` and the render and diff test files pass; the design lint printed its pass line but I did not check its exit status.

shipped production is serving it deploy f21b126

opened this

A .patch or .diff file in a repository now shows on its file page as the same coloured diff a pull shows. I didn't try it on a running page. The new tests, bun scripts/gate.ts, bun x tsc --noEmit and the render and diff test files pass; the design lint printed its pass line but I did not check its exit status.

  • Kinds: patch is a new kind in src/web/render/kinds.ts, with a 1 MB inline limit like text. .patch and .diff map to it by extension, case-insensitively.
  • Diff markup: the new src/web/render/patch.ts runs the existing parseUnifiedDiff and renderDiff, so nothing was copied. File headers, hunks, and added and removed lines come from there.
  • Mail header: for a git format-patch file, the From, Subject and Date lines show above the diff as plain escaped lines. Subject and the other fields are cut to 500 characters.
  • Fallback: text with no diff --git file in it keeps the text view.
  • Tests: test/render-patch.test.ts uses a two-file patch fixture. It checks that a line containing <img onerror> arrives escaped as &lt;img. It also covers the mail header, the fallback, and that only .patch and .diff pick the renderer. test/render-kinds.test.ts has the two new samples.

I chose not to handle plain diff -u or svn patches that lack diff --git lines. The parser the pull page uses only reads git format, and these fall back to text. I also didn't add syntax highlighting, inline comments, or a README entry.

The first push was blocked by a pre-push hook that wants a "Try:" commit message, so there is an empty commit saying how to try it. To try it, open any .patch or .diff file's blob page in a repository.

Closes #10286

commented

A .patch or .diff file in a repository now shows on its file page as the same coloured diff a pull shows. I didn't try it on a running page. The new tests, bun scripts/gate.ts, bun x tsc --noEmit and the render and diff test files pass; the design lint printed its pass line but I did not check its exit status.

  • Kinds: patch is a new kind in src/web/render/kinds.ts, with a 1 MB inline limit like text. .patch and .diff map to it by extension, case-insensitively.
  • Diff markup: the new src/web/render/patch.ts runs the existing parseUnifiedDiff and renderDiff, so nothing was copied. File headers, hunks, and added and removed lines come from there.
  • Mail header: for a git format-patch file, the From, Subject and Date lines show above the diff as plain escaped lines. Subject and the other fields are cut to 500 characters.
  • Fallback: text with no diff --git file in it keeps the text view.
  • Tests: test/render-patch.test.ts uses a two-file patch fixture. It checks that a line containing <img onerror> arrives escaped as &lt;img. It also covers the mail header, the fallback, and that only .patch and .diff pick the renderer. test/render-kinds.test.ts has the two new samples.

I chose not to handle plain diff -u or svn patches that lack diff --git lines. The parser the pull page uses only reads git format, and these fall back to text. I also didn't add syntax highlighting, inline comments, or a README entry.

The first push was blocked by a pre-push hook that wants a "Try:" commit message, so there is an empty commit saying how to try it. To try it, open any .patch or .diff file's blob page in a repository.

Changes requested — before the latest push

Reuse of parseUnifiedDiff/renderDiff is right and the tests cover the boxes. Changes needed:

  1. Design (principles 6 and 7): the mail header is a card box and patch-mail has no style anywhere in src. Render From/Subject/Date as a dl of rows (muted labels), no box.
  2. Both doors: the terminal's file view (src/tui/screens/workspace.ts) still shows a .patch as plain code. Give it the diff drawing the TUI already uses for pulls (common.ts), or say why not.
  3. Docs: docs/features/file-media.md lists the kinds (csv | model3d | config | markdown | text | binary) and the text-kinds rule; add patch and how it falls back. No README entry.
  4. The preview was not shown with a real .patch file in it; the author says it was never tried on a running page. Try one, at 360px, with JS off.
  5. Mail header is only found when line 1 matches From <sha> ; a mail whose headers are folded or that has no From line is silently dropped. Fine to keep, but add a test for a header with no diff after it? Optional.
commented

Four of the five review points are done and pushed. Item 4 is not: I did not try a real .patch at 360px with JS off. My shell has no browser, and the preview would need a .patch committed to the branch just to look at it. Someone should open a .patch blob on the #10324 preview before merge.

The gate, tsc, the design lint, and the render and workspace tests pass.

  1. Mail header: the card patch-mail box is gone. From, Subject and Date are now rows in a dl.status-rows list, using classes that already exist, so no new CSS. The labels are muted, and the values are escaped. The test checks the markup and that patch-mail no longer appears.
  2. Terminal: the file view in src/tui/screens/workspace.ts now draws a .patch or .diff with the TUI's pull diff (renderDiff in tui/ui/diff.ts), reusing the same parse as the web page. v flips to the source, and a file that doesn't parse stays as plain code. A new case in test/tui-workspace.test.ts covers the diff view, v, and a non-diff .diff file.
  3. Docs: docs/features/file-media.md now lists patch among the kinds and in the text-kinds rule. It also says how a patch falls back and that the terminal draws the same parse. There is no README entry.
  4. Header with no diff: a mail header with no diff after it now has a test; it falls back to text.

A mail without a From <sha> first line, or with folded headers, still shows no header. I kept that, as you allowed.

Changes requested — before the latest push

Reuse of parseUnifiedDiff/renderDiff is right, the escaping holds, and the terminal door, docs and tests are in. Two things to fix: (1) the mail-header value cap is wrong (slice end is an index, not a length), (2) the header markup is a hand-built string passed through raw() rather than an html template. Also: nobody has opened a real .patch on the preview (360px, JS off); please commit a small .patch fixture to the branch or show it, and say what you saw. Box 4 is met; box 1-3 are met in code but box 1 needs the preview check before I tick it.

commented

Opening a .patch or .diff file in a repository now draws it as a coloured diff, the same way a pull's diff is drawn: it goes through parseUnifiedDiff and renderDiff. A file from git format-patch also shows its From, Subject and Date lines as rows above the diff. The terminal draws such a file as a pull diff too.

This push fixes the two review points:

  • Length limit on header values: the limit was applied at position 500 of the whole line, so a Subject lost the length of its Subject: label from its 500-character limit. It is now 500 characters of the value itself (VALUE_CAP). A new test with a 600-character Subject checks that exactly the first 500 characters are kept; it fails against the old code.
  • Header markup: the header rows are now built with the html template and a row helper, the same way fact works in pages/maintenance.ts. escapeHtml and raw are no longer imported in src/web/render/patch.ts.

There is now a sample file, test/fixtures/format-patch.patch: a two-file patch with a mail header and a <b> tag in one line. Two tests use it:

  • test/render-patch.test.ts renders the fixture directly.
  • test/blob-patch-page.test.ts starts a real server and commits the fixture to a public repository as fix.patch. It then fetches /alice/p/blob/main/fix.patch without signing in, as plain HTML with no scripts run. The page answers 200 with the Subject and From rows (escaped), both files, added and removed lines marked, and the tag shown as escaped text.

Not checked on the preview: the preview runs its own empty instance, with only preview-admin/hello, and I have no account there, so I could not put a .patch into it. I also could not take a 360px screenshot from this sandbox. The end-to-end test above is the closest check I could run: it covers the same page with JS off. The 360px layout is the pull page's existing diff layout, unchanged. To see it in a browser: push the fixture to any repository on the preview and open its blob page.

Gate, type check and design lint pass, and so do test/render-patch.test.ts, test/blob-patch-page.test.ts, test/render-kinds.test.ts and test/tui-workspace.test.ts.

🤖 Generated with Claude Code

commented

A .patch or .diff file in a repository now opens on its blob page as the same coloured diff the pull page draws. When the file is a git format-patch mail, its From, Date and Subject header shows as plain rows above the diff, and each header value is cut at 500 characters. The terminal's file view draws these files the same way. A small .patch file is committed as a test fixture, and a test opens it over HTTP on its blob page.

This push merges main, which had added an archive renderer. The only conflict was the RENDERERS table in src/web/render/renderers.ts: it now lists both patch and archive, and file-kind detection handles both.

Checks run: bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts all pass. The fast test picker named 1551 files after the merge, too many to run here, so I ran the 88 tests that cover patch rendering, file-kind detection, archives and blob pages; all pass.

Not done: patch files over 1 MB are not drawn as a diff.

To try it: open https://lab--10324--forge-trevato-apps.open-platform.sh and go to any .patch file in a repository, at /<owner>/<repo>/blob/<branch>/<path>.patch.

🤖 Generated with Claude Code

commented

A .patch or .diff file in a repository now opens on its blob page as the same coloured diff the pull page draws. When the file is a git format-patch mail, its From, Date and Subject header shows as plain rows above the diff, and each header value is cut at 500 characters. In the terminal's file view, a .patch file shows as that diff and v switches to the raw text. A .diff file that isn't a git diff stays plain text. A small .patch file is committed as a test fixture, and a test opens it over HTTP on its blob page.

This push merges main again, which had added subtitle files (.srt, .vtt) next to archives. Both features added entries in the same places, so the merge keeps both:

  • File kinds: in src/web/render/kinds.ts, patch and subtitles both have their own kind, size limit and file extensions.
  • Renderers: src/web/render/renderers.ts lists patch, subtitles and archive.
  • Tests: test/render-kinds.test.ts keeps both sets of samples. In test/tui-workspace.test.ts the two clashing tests are now separate tests: the .patch one, then the .srt one.

The terminal workspace (src/tui/screens/workspace.ts) merged on its own, and its patch and subtitle code don't touch each other.

Checks run: bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts pass. I ran the 75 tests covering patch rendering, file kinds, archives, blob size limits and the terminal workspace; all pass. I did not run the whole suite here; the forge runs it on this push.

Not done: patch files over 1 MB are not drawn as a diff.

To try it: open https://lab--10324--forge-trevato-apps.open-platform.sh and go to any .patch file in a repository, at /<owner>/<repo>/blob/<branch>/<path>.patch.

🤖 Generated with Claude Code

commented

A .patch or .diff file in a repository now renders as a diff on its blob page, drawn with the same parse and markup the pull page uses (parseUnifiedDiff into renderDiff). For a git format-patch file, its From, Subject and Date show above the diff as plain rows, each value cut at 500 characters. The terminal's file view draws the same diff, and v shows the source. A file with no diff --git section in it, such as a plain diff -u or notes, falls back to the text view with a line saying why. The kind is called patch in render/kinds.ts, and docs/features/file-media.md describes it.

Budget raised: this push also raises the first-view budget in docs/design/attention-budget.json for the repository and app pages, at both widths: words 189 → 197, blocks 28 → 29. Main fails each page stays inside its first-view budget on its own. #10331 measured the budgets before #10311 added the onboarding line to a new app's head card, and the test seeds exactly that kind of app (no deploy yet). The line accounts for the extra 8 words and 1 block. The test's rule is that a raised budget needs a note in the pull; this is that note. No other page's budget changed.

Not done: a plain diff -u with no diff --git headers is not parsed as a diff; it stays text.

To try it: open test/fixtures/format-patch.patch on its blob page. test/render-patch.test.ts and test/blob-patch-page.test.ts cover the renderer and the page. Locally the gate, bun x tsc --noEmit, and those two tests plus render-kinds, tui-workspace and attention all pass. I did not run the full suite: after the merge with main, scripts/fast.ts named 1556 test files.

🤖 Generated with Claude Code

Approved — before the latest push

Earlier asks: (1) mail header is now a dl of rows, no card: met. (2) terminal draws .patch with its pull diff, v shows source, non-diff stays text, tested: met. (3) docs/features/file-media.md lists patch and the fallback, no README entry: met. (4) a real .patch on the preview was not shown, since the preview has no repository to hold one; the end-to-end blob page test over HTTP with no script covers it. Non-blocking: look at a .patch blob at 360px once it is on main. (5) test for a header with no diff after it: met. The first-view budget raise is explained in the pull and matches the onboarding line from #10311. Reuse of parseUnifiedDiff/renderDiff, escaping and the 500-char cap hold.

commented

A .patch or .diff file in a repository now shows as a diff when you open it, the same way a commit's changes are shown, instead of as plain source text. The kind is patch in src/web/render/kinds.ts, and its renderer is src/web/render/patch.ts, registered in src/web/render/renderers.ts.

This update brings in the latest main, which added a log kind (.log, .ansi) in the same places this pull adds patch. The conflicts in kinds.ts, renderers.ts and test/render-kinds.test.ts are resolved by keeping both kinds side by side: both are in the FileKind union, the size caps, the extension table, the renderer registry and the test samples. The log rule from main still stands: a log with no escape codes keeps the plain source view.

Checks:

  • bun x tsc --noEmit, bun scripts/gate.ts and bun scripts/design-lint.ts pass.
  • scripts/fast.ts named 1561 test files, far over its 5-minute budget, so I picked the suites that cover the merged files instead: test/render-.test.ts and test/blob-.test.ts (144 tests in 14 files, including render-patch, render-ansi, render-kinds and blob-patch-page). All pass.
  • The full suite was not run here; the forge's check runs it on this push.

To try it: open any .patch or .diff file in a repository on the preview; it should show as a diff. A .log file with color codes should still show in color.

🤖 Generated with Claude Code

Approved

Earlier asks are met. The mail header is now a dl of muted rows with no box. The terminal draws .patch with the TUI pull diff, and v shows the source. docs/features/file-media.md lists the patch kind and its fallback, with nothing added to the README. A header with no diff after it has a test, and the 500-character cap applies to the value. Parsing and markup reuse parseUnifiedDiff and renderDiff, and every header value and diff line is escaped. The blob-page test over HTTP covers the JS-off page. The preview answers 200, but it holds no .patch to open. I could not diff against 85f6013e2e2a because the clone does not have that commit, so I read the change as shown. Non-blocking: the 360px layout was never viewed in a browser.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs