Skip to content

feat: export static diff renderer - #686

Open
privatenumber wants to merge 3 commits into
modem-dev:mainfrom
privatenumber:feat/static-diff-export
Open

feat: export static diff renderer#686
privatenumber wants to merge 3 commits into
modem-dev:mainfrom
privatenumber:feat/static-diff-export

Conversation

@privatenumber

Copy link
Copy Markdown

Problem

Hunk can render unified patches as terminal-friendly ANSI text for constrained pager hosts, but that renderer is only available inside the CLI. An integration that already has a patch must either start the interactive application or reimplement the presentation layer.

For example, a terminal host can receive git diff output and need a colored stack or split view without taking over input or the alternate screen.

Changes

  • Publish hunkdiff/static with renderStaticDiff(patch, options), returning Hunk's sanitized ANSI output without starting an interactive review.
  • Keep the static API and pager fallback on one patch parsing and rendering path, including filename-based syntax detection.
  • Package the Node-targeted entry and declarations, verify the built export during the pack check, and document its options.

@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

PR author is not in the allowed authors list.

@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

@privatenumber is attempting to deploy a commit to the Modem Team on Vercel.

A member of the Team first needs to authorize it.

@benvinegar

Copy link
Copy Markdown
Member

Makes sense!

@benvinegar benvinegar 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.

Requesting changes for two runtime regressions: the published entry does not run on the package's supported Node versions, and routing pager input through the OpenTUI parser drops loader-owned metadata such as moved-line classification and agent-sidecar ordering.

Validation on a synthetic merge with current main: typecheck, focused tests, package check, lint, formatting, the repository test script, and Node 24 smoke all pass; direct Node 18/20 and colored-patch probes reproduce the issues inline.

This comment was generated by Pi using gpt-5.6-sol

Comment thread scripts/build-npm.ts
"--format",
"esm",
"--external",
"@pierre/diffs",

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.

[P1] Keep the new entry compatible with the declared Node ≥18 runtime. The built hunkdiff/static entry cannot be imported on supported Node versions. With the packed output, Node 18.20.8 fails while parsing bundled string-width@8 (Invalid regular expression flags from its /v regex; that dependency itself requires Node ≥20), and Node 20.20.0 fails while loading the external root @pierre/diffs entry because its browser CodeView dereferences navigator.userAgent. The same smoke succeeds on Node 22/24 and Bun, which is why the current pack check misses this: it imports the artifact under Bun. Please make this Node-18/server-safe (or intentionally change the package runtime contract) and run the package smoke with the minimum supported Node version.

This comment was generated by Pi using gpt-5.6-sol

Comment thread src/ui/staticDiffPager.ts Outdated
theme: AppTheme,
width: number,
) {
const files = createHunkDiffFilesFromPatch(text, "static").map(toInternalDiffFile);

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.

[P2] Preserve the loader metadata when unifying the parse path. loadAppBootstrap previously captured Git's magenta/cyan --color-moved markers into DiffFile.lineMoveKinds; createHunkDiffFilesFromPatch(...).map(toInternalDiffFile) strips the ANSI during normalization but never carries those classifications, so captured pager output now uses ordinary added/removed backgrounds instead of the moved-line palette. A focused colored-patch probe confirms both rows had the moved background before this change and lose it here. This path also skips loadAgentContext/orderDiffFiles, so pager invocations with an agent sidecar revert from the sidecar's intentional file order to patch order. Please share/extract the full loader normalization rather than only the lower-level OpenTUI parser, and add regression coverage for these pager behaviors.

This comment was generated by Pi using gpt-5.6-sol

@benvinegar benvinegar 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.

The follow-up fully fixes the two previous findings: packed rendering now works under Node 18/20, and pager output again preserves moved-line metadata and agent-sidecar order. One new terminal-geometry regression remains from the package-wide string-width downgrade.

This comment was generated by Pi using gpt-5.6-sol

Comment thread package.json
"get-east-asian-width": "^1.5.0",
"shell-quote": "1.9.0",
"string-width": "^8.2.1",
"string-width": "^7.2.0",

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.

[P2] Avoid regressing halfwidth Katakana geometry while supporting Node 18. Downgrading the package-wide width engine changes Hunk's existing terminal measurements: with 8.2.1, measureTextWidth("ガ") is 2 cells, matching the two halfwidth code points and Python wcwidth; with this 7.2.0 dependency it is 1. That makes wrapping, clipping, horizontal placement, and copy-selection columns one cell short anywhere this cluster appears. The loop in src/ui/lib/ui-lib.test.ts:252-258 masks the change because it derives the expected value from whichever dependency version is installed. Please retain/correct the terminal width behavior while making the static bundle Node-18-compatible, and pin this case to an explicit expected width of 2.

This comment was generated by Pi using gpt-5.6-sol

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants