render-grid: export bounded rows after viewport - #107
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds bounded bidirectional render-grid JSON export and active-row traversal. Rendering now covers scrollback, viewport, and bounded forward rows, with region spans, row counts, cursor location metadata, and compressed-page reuse. ChangesTerminal rendering exports
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CClient
participant ghostty_surface_render_grid_json_bounded
participant buildRenderGridJson
participant RenderGridRowIterator
participant RenderGridPageReader
CClient->>ghostty_surface_render_grid_json_bounded: Provide before and after row caps
ghostty_surface_render_grid_json_bounded->>buildRenderGridJson: Build bounded JSON frame
buildRenderGridJson->>RenderGridRowIterator: Traverse bounded terminal regions
RenderGridRowIterator->>RenderGridPageReader: Decode rows from page storage
RenderGridPageReader-->>RenderGridRowIterator: Return row data
RenderGridRowIterator-->>buildRenderGridJson: Return region and output-row mappings
buildRenderGridJson-->>CClient: Return render-grid JSON
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds a bounded rows-after-viewport window to render-grid exports. The main changes are:
Confidence Score: 4/5The unchanged C symbol needs an ABI-compatible extension before merging.
include/ghostty.h and the matching exported function in src/apprt/embedded.zig Important Files Changed
Reviews (1): Last reviewed commit: "render-grid: export bidirectional viewpo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| uintptr_t, | ||
| uintptr_t); |
There was a problem hiding this comment.
An existing binary compiled against the five-argument API can still resolve this unchanged symbol, but the new implementation reads a sixth argument that the caller never supplied. That value becomes scrollforward_lines, so mixed-version deployments can export an arbitrary number of rows after the viewport; use an ABI-compatible entry point or a new symbol for the extended signature.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/terminal/PageList.zig`:
- Around line 7979-8000: Resolve the missing PageList.activeRowsAfter reference
in the test "PageList active rows after bounded historical end": either
implement the method on PageList with the expected iterator/null behavior, or
revise the test to use the existing API that provides equivalent active-row
traversal. Ensure the test compiles and preserves its assertions for bounded
historical ends and active_bottom.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7438ba34-7408-4cf0-9824-c59a036859af
📒 Files selected for processing (2)
src/apprt/embedded.zigsrc/terminal/PageList.zig
1 participant
Adds an independent bounded rows-after-viewport window to the embedded render-grid export. The producer traverses one bounded range, preserves compressed pages, and leaves existing scrollback fields unchanged.
Verification:
zig build test -Dtest-filter="render grid" -Demit-macos-app=falsezig build -Demit-macos-app=falseParent: manaflow-ai/cmux#7304
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a bounded “rows after viewport” window to render‑grid JSON and a mode‑aware exact row scroll API for mobile. Exports now settle any pending resize first to reflect the current geometry.
New Features
scrollforward_rows/scrollforward_spansafter the viewport; emitsprimary_active_rows/primary_active_spanswhen the forward window stops before the active screen (capped to one viewport).location(viewport,above_viewport,below_viewport) andactive_row;visiblenow reflects cursor mode even when off‑screen.ghostty_surface_render_grid_json_bounded(...)andghostty_surface_mouse_scroll_with_viewport_rows(...); newer rows are excluded from the legacy entrypoint.Migration
ghostty_surface_render_grid_json(...)is unchanged and excludes rows after the viewport. Useghostty_surface_render_grid_json_bounded(surface, surface_id, len, state_seq, scrollback_lines, scrollforward_lines)to include newer rows; pass0to omit.ghostty_surface_mouse_scroll_with_viewport_rows(surface, x, y, rows, mods); positiverowsscroll upward.scrollforward_*,primary_active_*,cursor.location, andcursor.active_row. Free returned strings withghostty_string_free.Written for commit 4f4f385. Summary will update on new commits.
Summary by CodeRabbit