Skip to content

feat: export static diff renderer - #686

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

feat: export static diff renderer#686
privatenumber wants to merge 10 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
Comment thread src/ui/staticDiffPager.ts Outdated

@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 Outdated
@privatenumber

privatenumber commented Aug 10, 2026

Copy link
Copy Markdown
Author

Addressed in 7b43449.

I considered substituting string-width@7 in Node builds, raising the Node floor, and transpiling string-width@8. Those options respectively create renderer differences, break Node 18 support, or add a large/slower generated regex while retaining runtime Unicode drift.

The best balance was one Node 18-compatible width profile shared by both renderers, preserving string-width@8 behavior. Differential probes found zero mismatches, and package checks pass under Node 18 and 20.

@privatenumber

Copy link
Copy Markdown
Author

@benvinegar I rebased onto the latest main and resolved the review threads. Could you take another look when you have a moment?

@benvinegar
benvinegar force-pushed the feat/static-diff-export branch from f22cdc9 to 70b54d5 Compare August 30, 2026 21:17
@benvinegar

Copy link
Copy Markdown
Member

@privatenumber Yep bringing it home

@benvinegar
benvinegar enabled auto-merge (squash) August 30, 2026 21:31
auto-merge was automatically disabled August 31, 2026 04:14

Head branch was pushed to by a user without write access

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