Repository navigation
editor: Support mirrored gutters for side-by-side diffs - #3417
Conversation
48a3393 to
fb4861c
Compare
fb4861c to
f7844c9
Compare
f7844c9 to
a38e740
Compare
huacnlee
left a comment
There was a problem hiding this comment.
This PR adds right-side editor gutters and configurable column ordering for mirrored diff panes. The requirement and shared-geometry approach are reasonable. I reviewed only the three commits after #3416's 14ef7932c, at head a38e74079.
Please address the following before merging:
-
P2 — Reserve the effective scrollbar width when keeping text clear. In
crates/base/src/input/base/element.rs:2779–2782, clearance is fixed atRIGHT_MARGIN(10 px), while the default scrollbar track is 16 px wide and theme configuration can make it wider. A Base editor with zero padding, a right gutter, and a left scrollbar starts text at x = 10: the track intercepts clicks over the first 6 px of text, and the active thumb overlaps the first 2 px. The current test supplies 6 px of left padding, masking this case. Compute clearance from the effective scrollbar geometry and editor padding, and apply it consistently to text origin, width, wrapping, and scrolling. Please cover zero padding and a wider themed track. -
Qualify the marker placement documentation.
website/component/editor.md:389andcrates/base/src/input/editor/line_decorations.rs:212describe markers as always outside the line numbers. That is only the default order;gutter_ordercan move them inward. Update the Rust documentation and both website locales accordingly.
I also found that the Public API inventory omitted InputPresentation::gutter_side(&self) -> Side; I have included it while updating the English title and description.
Validation: source/diff review and git diff --check passed. Tests and real-window validation were not run. No other concrete regressions were identified in mirrored columns, hit testing, or IME coordinate handling.
a38e740 to
f617384
Compare
|
Thanks, both addressed (rebased onto #3416's
|
## Description The vertical scrollbar is hard-coded to the right edge and the horizontal one to the bottom. In a side-by-side diff the left pane's scrollbar belongs on its outer (left) edge, so the two panes mirror each other. This adds `ScrollbarPlacement`, one value for both axes, set with one call — `Scrollbar::placement`, and `scrollbar_placement` / `set_scrollbar_placement` on the editor state: | | vertical | horizontal | | --- | --- | --- | | `BottomRight` (default, unchanged) | right | bottom | | `BottomLeft` | left | bottom | | `TopRight` | right | top | | `TopLeft` | left | top | Each axis follows its half of the placement: the track sits on that edge, the thumb and its hit area are anchored to it, and the bar slides in from that edge. When both bars show, the vertical one keeps its full height and the horizontal one stops short of it — at its start when the vertical bar is on the left, at its end when it is on the right. A single-axis scrollbar uses only its own half. The editor reserves no room for its horizontal bar at the bottom today, so at the top it overlays the first line the same way. Nothing changes by default. Targets `next`: it is the first half of a mirrored side-by-side diff whose second half, #3417, builds on #3359's gutter markers, which are on `next`. ## Screenshot | Before | After | | ------ | ----- | | <img width="800" alt="before" src="https://raw.githubusercontent.com/GigLaboCom/gpui-component/pr-assets/screenshots/before.png" /> | <img width="800" alt="scrollbar-left" src="https://raw.githubusercontent.com/GigLaboCom/gpui-component/pr-assets/screenshots/scrollbar-left.png" /> | The four placements (`ScrollbarMode::Always`); the story switches Dataset and Placement from one Options menu (`d8057f92`): <img width="1000" alt="scrollbar placements, light" src="https://raw.githubusercontent.com/GigLaboCom/gpui-component/pr-assets/screenshots/scrollbar-placement-light.png" /> <img width="1000" alt="scrollbar placements, dark" src="https://raw.githubusercontent.com/GigLaboCom/gpui-component/pr-assets/screenshots/scrollbar-placement-dark.png" /> ## Public API `gpui_base` (re-exported by `gpui_component`, also from `gpui_component::scroll`): - `ScrollbarPlacement { BottomRight, BottomLeft, TopRight, TopLeft }` — default `BottomRight`; `is_left()` (vertical bar on the left), `is_top()` (horizontal bar at the top). - `Scrollbar::placement(mut self, placement: ScrollbarPlacement) -> Self`. - `InputBaseState::scrollbar_placement(mut self, placement: ScrollbarPlacement) -> Self` — builder for the editor's scrollbars. - `InputBaseState::set_scrollbar_placement(&mut self, placement: ScrollbarPlacement, cx: &mut Context<Self>)` — replaces the whole placement at runtime. ## How to Test - `cargo test -p gpui-base --lib scrollbar`: the `Scrollbar` harness clicks and drags on each placement — the vertical track and thumb on the left, the horizontal track and thumb at the top, the horizontal track starting after a left bar and ending before a right one with both bars at the top, a thumb drag and a track click moving the offset the right way — and `visibility_translation_moves_toward_the_nearest_edge` covers all four placements on both axes; an editor layout test for `TopLeft`. Each fails with its code reverted. - `cargo run -p gpui-component-story -- scrollbar`, then Options → Placement. ## Checklist - [x] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document and followed the guidelines. - [x] Reviewed the changes in this PR and confirmed AI generated code (If any) is accurate. - [x] Passed `cargo run` for story tests related to the changes. - [x] Tested macOS, Windows and Linux platforms performance (if the change is platform-specific) --------- Co-authored-by: Jason Lee <huacnlee@gmail.com>
f617384 to
36d1e43
Compare
|
Rebased onto |
|
Please cherry-pick this follow-up commit into the PR: git fetch https://github.com/longbridge/gpui-kit.git pr-3417-diff-story
git cherry-pick 70b271adThe useful case here is a side-by-side diff with both gutters facing the center, where corresponding line numbers can be compared directly. Please frame the feature around that use case rather than presenting right-side line numbers as a general Editor preference or RTL support. This commit adds an Editor Diff story showing original and modified source with change markers. Its Options menu switches between both gutters on the left and center-facing gutters, and optionally places markers before line numbers. The ordinary Editor story remains unchanged. It also synchronizes vertical scrolling before either pane is laid out. The initial observer-only approach missed scrollbar-thumb dragging; the regression test now checks real wheel input and dragging either scrollbar, including corresponding row positions on the same frame, as well as source preservation when switching layouts. Validation: the targeted Story regression test, build, formatting, and diff checks passed. Launch with: cargo run -p gpui-component-story -- 'Editor Diff'This follow-up was implemented with AI assistance and tested locally. |
AI-assisted implementation; verified layout switching, wheel scrolling, and same-frame alignment when dragging either scrollbar.
|
Thanks — cherry-picked as Reframed around that case in |
|
CI failed on |
|
One more: |
|
Please cherry-pick this additional fix from git fetch https://github.com/longbridge/gpui-kit.git pr-3417-diff-story
git cherry-pick e30f26152f31f91cbdefdadc42810a4baf997bdbIf the previous diff story commit ( The horizontal scrollbar now excludes the center-facing gutter in the original pane, mirroring the modified pane. The vertical scrollbars remain on the outer edges, and horizontal scrolling remains independent. This also prevents the horizontal scrollbar from intercepting clicks in the right gutter. No public API changes. Validation: horizontal scrollbar, scrollbar, editor element, and diff scrolling regression tests passed; formatting and Base Clippy checks passed. Attached screenshot of the side-by-side diff (the two panes have independent horizontal scroll offsets): |
AI-assisted implementation; verified fixed-gutter click handling, unchanged scrolling ranges, scrollbar regressions, and synchronized diff scrolling.
|
Cherry-picked as |

Description
Builds on #3416 (
ScrollbarPlacement, merged intonext) and #3359's gutter markers.In a side-by-side diff, both gutters can face the center, so the line numbers of corresponding lines sit next to each other across the divider and can be compared directly. The editor's gutter is always on the left of the text today, so the left pane cannot do that.
This adds
gutter_side(Side)/set_gutter_sideto the editor state for the left pane of such a diff, defaultSide::Left(unchanged). On the right the gutter is mirrored: the same columns, in the same order counted from the text, with the line numbers aligned toward it. The text and gutter x offsets are computed once per layout and used for painting, hit testing, IME bounds and scroll-into-view. A scrollbar on the gutter's side stays outermost. With the gutter on the right and the scrollbar on the left — the left pane's outer edge — the text keeps clear of the scrollbar's whole track: its effective width, the active thumb included, less the editor's left padding, at least the usual 10 px margin; that one reservation sets the text origin, width, wrap width and scroll size.Line numbers are placed by their shaped width rather than padded with spaces: in a proportional font a space is about a third of a digit, so the default left gutter left right-aligned numbers ragged and 6–11 px short of the text; they now line up against it on either side.
Editorputs its narrow padding on the gutter's side.gutter_order([GutterColumn])orders the gutter's columns from the text outward, on either side: the default is[FoldIcons, LineNumbers, Markers](unchanged);[FoldIcons, Markers, LineNumbers]puts a diff's change markers between the text and the line numbers, as IntelliJ's diff does. A column left out follows the listed ones in its default order.The Editor Diff story (
70b271ad, @huacnlee) shows original and modified source with change markers; its Options menu switches between both gutters on the left and center-facing gutters, and optionally puts the markers before the line numbers. The two panes scroll together, by wheel and by dragging either scrollbar. The ordinary Editor story is unchanged.Screenshot
A side-by-side diff with changed lines marked; the left pane uses
.scrollbar_placement(ScrollbarPlacement::BottomLeft).gutter_side(Side::Right), mirrored:The columns in IntelliJ's order,
.gutter_order([GutterColumn::FoldIcons, GutterColumn::Markers, GutterColumn::LineNumbers])on both panes — without and with folding (an empty fold column here):Public API
gpui_base(re-exported bygpui_component):InputBaseState::gutter_side(mut self, side: Side) -> Self— builder: which side of the text the gutter is drawn on; defaultSide::Left,Side::Rightfor the left pane of a side-by-side diff.InputBaseState::set_gutter_side(&mut self, side: Side, cx: &mut Context<Self>)— the same, at runtime.InputPresentation::gutter_side(&self) -> Side— the side the editor's gutter sits on.GutterColumn { FoldIcons, LineNumbers, Markers }— a column of the gutter.InputBaseState::gutter_order(mut self, columns: impl IntoIterator<Item = GutterColumn>) -> Self/set_gutter_order(&mut self, columns, cx: &mut Context<Self>)— the columns from the text outward; default[FoldIcons, LineNumbers, Markers]; a column left out follows in its default order, a repeat is ignored.How to Test
cargo test -p gpui-base --lib: new tests for the text and gutter origins with an unchanged wrap width, the mirrored fold icons and columns, the scrollbar staying outermost (left/left, right/left, right/right), clicks in the text and in the gutter on both sides, caret / IME /range_to_bounds/ selection-path x including a long line scrolled clear of the gutter, a right-side case in the existing gutter-bounds test, every gutter column — markers included — mirrored to 0.01 px for all six orders with folding on and off, the order normalised and counted from the text on both sides, clicks on the fold icon and the line number in every side/order combination,Editorkeeping its gutter-side padding, and the text clear of the whole track of a left scrollbar with zero padding and with a wider themed track. Each fails with its code reverted.cargo test -p gpui-component-story --lib editor_diff: layout switching keeps both sources, and the panes stay aligned on wheel input and while either scrollbar is dragged.cargo run -p gpui-component-story -- 'Editor Diff', then Options.Checklist
cargo runfor story tests related to the changes.