Skip to content

feat: shared wrap helpers — GPUI cleanup + SwiftUI SBS wrap rendering#54

Merged
hewigovens merged 3 commits into
perf/jj-diff-context-thresholdfrom
feat/core-wrap-helpers-uniffi
May 22, 2026
Merged

feat: shared wrap helpers — GPUI cleanup + SwiftUI SBS wrap rendering#54
hewigovens merged 3 commits into
perf/jj-diff-context-thresholdfrom
feat/core-wrap-helpers-uniffi

Conversation

@hewigovens
Copy link
Copy Markdown
Owner

@hewigovens hewigovens commented May 22, 2026

Summary

Stacked on top of #53. Three commits land here, all built around the same core idea: move the diff-wrap algorithm into jj-diff once, expose it via uniffi, then have both shells consume it.

1. feat(core): expose wrap helpers via uniffi

Moves the diff-wrap algorithm into jj-diff and exposes it across uniffi.

  • New crates/jj-diff/src/wrap.rs (~330 LOC incl. tests):
    • Records WrappedDiffLine, WrappedSbsRow (u32 fields for clean uniffi crossing).
    • Pure functions wrap_cols_for_width(width: f32, advance: f32) -> u32, wrap_diff_lines, wrap_sbs_rows, sbs_line_to_row, visual_index_for_line, visual_index_for_sbs_row.
    • wrap_sbs_rows produces aligned visual rows for SBS: each SideBySideRow expands to max(old_chunks, new_chunks) rows with empty padding on the shorter side.
  • crates/jayjay-uniffi/src/types.rs: #[uniffi::remote(Record)] for the two new records.
  • crates/jayjay-uniffi/src/diff.rs: 6 #[uniffi::export] free functions; Swift bindings regenerate as wrapColsForWidth, wrapDiffLines, wrapSbsRows, sbsLineToRow, visualIndexForLine, visualIndexForSbsRow.
  • 8 unit tests cover wrap layout, SBS pairing, visual-index lookup, and wrap_cols_for_width clamping/defaulting.

2. refactor(gpui): use shared wrap helpers from jj-diff

Drops the GPUI shell's duplicate of the wrap algorithm — shell/gpui/src/diff/wrap.rs is now a 70-line thin wrapper that re-exports jj_diff::wrap::* plus the two GPUI-specific helpers that legitimately belong here:

  • wrap_cols_from_bounds(Bounds<Pixels>, Pixels) -> u32 — pulls the width out of a gpui Bounds slot.
  • selection_cols_in_fragment(Range<usize>, usize, usize) -> Option<Range<usize>> — UI helper that maps a logical selection column range onto a wrapped visual fragment. Uses Range<usize> because the GPUI selection module speaks in usize.

The selection-fragment helper kept its .then(|| v) fix from #53 (the crash-fix for eager subtraction overflow on continuation fragments). Two regression tests carry along.

Consumers updated with surgical as usize casts at the boundary between the shared u32-flavored wrap records and GPUI's usize-flavored selection geometry: sbs_body.rs, unified_body.rs, log/find.rs. attach_selection_handlers and s.col_range_for(...) already take usize so the casts stay close to the wrap-record field reads.

3. feat(swiftui): wrap SBS diff with shared wrap_sbs_rows

Drops the "SBS wrap is disabled in AppKit" workaround that #53 had to ship.

  • SideBySideDiffCoordinator now owns the rendering state (diff, font, theme) and the rebuild path:
    • Wrap measurement. Reads each pane's scrollView.contentSize.width and the font's M advance, then asks Rust for the column count via wrapColsForWidth. Calls wrapSbsRows to flatten paired SBS rows into per-visual-row records and consumes .row directly (line numbers already cleared on continuation segments).
    • Resize handling. Wires DiffTextContainerView.onContentLayoutChanged to renderIfNeeded(force: false), which short-circuits when the cached lastOldCols/lastNewCols haven't changed. SwiftUI state changes still call renderIfNeeded(force: true) from updateNSView.
  • The text-attribute helpers (appendTextLine, appendGutterLine, lineBg, tokenColor) move from extension SideBySideRepresentable to internal free functions so the Coordinator can call them. No behavior change — they never used self.
  • DiffTextContainerView.wrapsText = false stays — we pre-wrap in Rust so each visual row is shorter than the pane's column count; the NSTextContainer should not re-wrap that pre-wrapped row.

Why SBS alignment now holds

The old code fed one logical SBS row to two independent NSTextViews; long lines wrapped per-side, so 3 visual lines on the left and 1 on the right left the panes out of vertical alignment. Now we ask Rust to pad the shorter side with blank continuation rows up front, so both NSTextViews receive the same visual-row count and the gutters line up automatically.

Test plan

  • cargo test -p jj-diff wrap:: — 8 unit tests.
  • cargo clippy -p jj-diff -p jayjay-core -p jayjay-uniffi -p jayjay-gpui --all-targets -- -D warnings.
  • just build, just test-app, just test-gpui — all green; uniffi regenerates the Swift bindings.
  • Manual: open a diff with very long lines, view in SBS mode in both shells. Drag the splitter and resize the window — both panes should wrap to the same number of visual rows with aligned gutters. Find-next/prev should jump to the correct wrapped row in unified and SBS modes.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces visual wrapping logic for unified and side-by-side diffs, including new UniFFI exports and a core wrap module with comprehensive tests. Feedback focuses on optimizing the UniFFI boundary by avoiding redundant data transfers for single lookups, improving lookup performance using binary search for sorted collections, and addressing the quadratic complexity in character-based span splitting.

Comment thread crates/jayjay-uniffi/src/diff.rs Outdated
Comment thread crates/jayjay-uniffi/src/diff.rs Outdated
Comment thread crates/jj-diff/src/wrap.rs
Comment thread crates/jj-diff/src/wrap.rs
Comment thread crates/jj-diff/src/wrap.rs Outdated
@hewigovens hewigovens force-pushed the feat/core-wrap-helpers-uniffi branch from dd17242 to aa9b2c5 Compare May 22, 2026 00:49
@hewigovens hewigovens changed the title feat(core): expose wrap helpers via uniffi for shared SBS wrapping feat: shared wrap helpers + SwiftUI SBS wrap rendering May 22, 2026
@hewigovens hewigovens changed the base branch from main to perf/jj-diff-context-threshold May 22, 2026 00:49
@hewigovens hewigovens force-pushed the feat/core-wrap-helpers-uniffi branch from aa9b2c5 to d9789fc Compare May 22, 2026 02:07
@hewigovens hewigovens changed the title feat: shared wrap helpers + SwiftUI SBS wrap rendering feat: shared wrap helpers — GPUI cleanup + SwiftUI SBS wrap rendering May 22, 2026
@hewigovens hewigovens force-pushed the feat/core-wrap-helpers-uniffi branch from d9789fc to 91485f3 Compare May 22, 2026 09:52
@hewigovens hewigovens merged commit 49436fc into perf/jj-diff-context-threshold May 22, 2026
4 checks passed
@hewigovens hewigovens deleted the feat/core-wrap-helpers-uniffi branch May 22, 2026 13:13
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.

1 participant