Repository navigation
input: Add line decorations with row backgrounds and gutter markers - #3359
Conversation
The patch rebuilt on current gpui-kit main in #3040's collection shape, branch heretic/line-decorations-on-upstream on the org fork, and opened upstream as draft longbridge/gpui-kit#3359. The porting notes — API, old-to-new mapping, how compare::Marks migrates, test results — are kept beside the dossier. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
huacnlee
left a comment
There was a problem hiding this comment.
Please fix the two rendering issues below. This PR is targeted for the 0.8.0 milestone, where the InputEditorStyle API break can be handled.
For the API change, please also mark InputEditorStyle #[non_exhaustive], migrate the cross-crate struct literals in Component and the Base showcase examples, and update the Breaking Changes section. External struct literals, including those using ..Default::default(), will no longer compile once the attribute is added; document construction via Default followed by field assignments or a supported construction API.
Security: PASS for the affected scope based on source inspection. Existing CI checks passed for this commit; the rendering findings are source-derived and were not visually reproduced locally.
Please address the findings above, then request my review again.
| .then(|| state.editor_style.gutter_marker_renderer.clone()) | ||
| .flatten(); | ||
| // Start in the gutter's left padding, clear of numbers under three digits. | ||
| let marker_x = origin_x - state.editor_paddings.left; |
There was a problem hiding this comment.
[P2] Reserve space for gutter markers so they do not overlap line numbers. The marker starts at origin_x - editor_paddings.left and is 12px wide, but the styled code editor's left padding is at most 6px. Three-digit line numbers (for example row 100 in a 100+ line document) have no leading padding in the reserved three-digit number column, so the marker overlaps their first digit. Markers are painted after the numbers and can obscure that digit. Please allocate a dedicated marker slot and adjust the number position/gutter width accordingly. Verify marked rows with three-digit and longer line numbers, including horizontal scrolling.
There was a problem hiding this comment.
Fixed in 3f9da90. The gutter now reserves a slot of its own at the left of the numbers, the 12px marker plus a 4px gap, and the numbers and text move right by it. The slot is reserved while line numbers are shown, a marker renderer is set and a collection has a provider, so the gutter does not change width as marked rows scroll; editors without a collection are unchanged.
gutter_markers_have_a_slot_left_of_the_line_numbers covers three- and four-digit columns with the styled editor's 6px padding, before and after a horizontal scroll, and goes red without the slot. Screenshots of both columns and of the scrolled view are in the description.
| let line_height = last_layout.line_height; | ||
| let height = last_layout.lines.get(ix)?.size(line_height).height; | ||
| let top = last_layout.visible_top | ||
| + last_layout.lines[..ix] |
There was a problem hiding this comment.
[P2] Include multiline inline-completion displacement in row_bounds. This calculation sums only real buffer row heights. Text painting and layout_line_decorations add ghost_lines_height after the cursor row, so when a multiline completion is visible, row_bounds for each subsequent row returns a y coordinate above its actual rendered position by that height. For example, two extra ghost lines on row 0 make row_bounds(1) point two line heights above row 1, breaking overlays or diff connectors positioned through this new API. Please return the actual laid-out row geometry, including ghost displacement, and add regression coverage for a following row while a multiline completion is displayed.
There was a problem hiding this comment.
Fixed in 3f9da90. The ghost lines' row and height are now part of the stored layout, and row_bounds and the line decoration layout share one row walk, LastLayout::row_extents, so both place a row where its text is painted.
rows_below_a_multiline_inline_completion_are_bounded_where_painted shows two ghost lines after row 0, then checks that rows 1–3 move down by two line heights and that their backgrounds agree. It goes red with either the stored ghost lines or row_bounds' use of them taken out.
InputBaseState gains one reader beside range_to_bounds: - pub fn row_bounds(&self, row: usize) -> Option<Bounds<Pixels>> returns the window-space band a zero-based buffer row occupies in the last layout: from the input's left edge, gutter included, to its right edge, as tall as all of the row's soft-wrapped lines. It is None before the first layout and for a row that is scrolled out of view or folded away. range_to_bounds answers for a byte range at the glyphs, one visual line high; a caller that needs to line a row up with something else — two editors side by side, a popup against a row, a hitbox over it — needs the whole row instead, and the walk over the layout's visible lines is internal to the element. The band is also exactly what a line-decoration background covers (next commit), and its tests use this reader to check that. This is the fork's visible_line_bounds(row: u32), renamed to the upstream vocabulary (`row`, as in visible_row_range; `usize`, as every row elsewhere) and anchored to the unscrolled input bounds so that x and width do not move with horizontal scrolling. Like line_and_position_for_offset, it does not account for the ghost lines of an inline completion. The fork's line_number_hitbox accessor is not ported: exposing a Hitbox from the previous frame invites is_hovered checks against a stale frame. Tests: - input::element::tests::row_bounds_cover_soft_wrapped_rows_that_are_laid_out covers an unpainted state, a soft-wrapped row, the row after it, a folded row, the row after the fold, and a row scrolled out of view. Patch P4 of Sub-epic E (line decorations), ported onto upstream main. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
An editor can now paint whole rows: a background band across a row, a
marker in its gutter, or both. The entries come from a provider that is
asked about the visible buffer rows on every frame, owned through a
collection handle in the shape of TextDecorationCollection and
RangeDecorationCollection, so independent features cannot replace one
another's decorations.
Surface (gpui-base, re-exported from gpui_component::input):
- trait LineDecorationProvider with one method,
line_decorations(&self, rows: Range<usize>, cx: &App)
-> Vec<LineDecoration>.
- struct LineDecoration: new(row), with_background(Hsla),
with_marker(GutterMarker), and the readers row(), background(),
marker(). Fields are private, as on RangeDecoration.
- #[non_exhaustive] enum GutterMarker: DiffAdded, DiffRemoved,
DiffChanged, Conflict, Bookmark, Breakpoint, and
Custom { icon: SharedString, color: Hsla }, where icon is an SVG asset
path. Base holds the meaning; the presentation layer draws it.
- EditorState::create_line_decorations_collection(
Rc<dyn LineDecorationProvider>, cx) -> LineDecorationCollection, with
set_provider, clear, dispose and has_provider on the handle. Clones
address one collection; dispose invalidates all of them; calls on a
disposed collection or a dropped editor are no-ops; later collections
paint over earlier ones.
- InputExtras::line_decorations(rows, cx), defaulted to empty, is how the
mode-generic renderer reaches the editor's collections.
- InputEditorStyle::gutter_marker_renderer:
Option<GutterMarkerRenderer>, with
type GutterMarkerRenderer = Rc<dyn Fn(&GutterMarker) -> AnyElement>,
beside fold_icon_renderer. gpui-component's Input projects one that
draws Plus, Minus, Asterisk, TriangleAlert, StarFill and CircleX in
the theme's success, danger, warning, warning, info and danger colors,
and a Custom icon path as given.
Nothing is tracked across edits or cleared by set_value: the provider is
asked again every frame and answers for the document as it is, which is
also what lets it read colors from the theme when asked. Decorations
outside the asked rows are dropped. With no collection, nothing is asked
and nothing is painted.
Render integration in TextElement:
- layout_line_decorations() runs in prepaint after the fold icons. It
asks for first..last+1 of the visible buffer rows, places each row the
way the text is painted (soft wraps, and inline-completion ghost lines
after the cursor row), skips rows folded away, and prepaints a 12px
marker at the left edge of the gutter, vertically centred on the row's
first line. A line-number column wider than three digits runs under
it, as in JetBrains editors. Markers need a projected renderer and
line numbers; backgrounds need neither.
- paint draws each band across the row before the active line, so it
sits under the active line, glyph backgrounds, indent guides,
decorations, selections and text, and again across the gutter after
the opaque gutter background and before the line numbers. Bands dim
with a disabled editor like the active line. Markers paint after the
fold icons.
Story and docs: the Editor story's Decorations tab marks its last three
rows from a provider that reads the text when asked; the English and
Chinese Editor pages document the provider contract and paint order.
Tests:
- input::line_decorations::tests::line_decoration_collection_round_trips
(the port of set_line_decoration_provider_round_trips: create, clear,
set through a clone, dispose, no-op after dispose),
collections_are_independent_and_asked_in_creation_order,
a_dropped_editor_makes_its_collections_no_ops,
a_decoration_is_built_from_its_parts.
- input::element::tests::line_decorations_are_asked_for_the_visible_rows_on_every_frame,
gutter_markers_need_a_renderer_and_the_line_numbers,
line_backgrounds_cover_the_bounds_of_their_rows.
- gpui-component input::editor::tests::line_decorations_paint_every_marker_kind
draws every marker kind through the styled Editor.
Changes from the fork's LineDecorationProvider (c319bac): the one
provider slot on the code-editor mode became owned collections; u32 rows
became usize; decorations_for became line_decorations;
LineDecorationItem became LineDecoration with private fields and
builders, and loses the tooltip that was never painted;
LineDecorationGlyph became GutterMarker, non-exhaustive, with Custom
carrying an icon path because gpui-base has no IconName; the hard-coded
glyph colors became theme colors chosen in gpui-component; the provider
is Rc and need not be Send + Sync, as the LSP providers; bands no longer
need line numbers, and continue across the gutter; the tint paints under
the active line, as the fork's comment intended.
Patch P1 of Sub-epic E (line decorations), ported onto upstream main.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The input sets an I-beam over its whole frame, so the line-number gutter read as part of the text it numbers, and over a gutter marker or a fold chevron the pointer still promised a caret. Whenever line numbers are shown, TextElement::paint now sets gpui::CursorStyle::Arrow over the line-number hitbox that layout_fold_icons already inserts on every frame (folding on or off). The request is made during the element's paint, after the root's I-beam, so it wins while the pointer is in the gutter and nowhere else. Gated on line numbers being shown, so single-line inputs, textareas and editors without line numbers are untouched. Not covered by an automated test: gpui's test platform records the cursor style it is asked for but exposes no reader for it. Patch P3 of Sub-epic E (line decorations), ported onto upstream main. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The marker started at the text element's left edge, after the editor's left padding, so a 12px marker ran into the second digit cell and covered the tens digit of two-digit line numbers. The gutter background already covers that padding, so the marker now starts there: numbers under three digits stay clear, and only the hundreds digit of a three-digit column runs under it, which is the overlap the fork documented for wide columns. Found in a live check of the Editor story's Decorations tab. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Match the surrounding style: short field and type docs like the fold icon layout beside them, the behaviour described once on create_line_decorations_collection as create_range_decorations_collection does, and "line decoration" spelled like "range decoration". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review of longbridge#3359. Markers were painted in the gutter's left padding, which the styled code editor caps at 6px, so a 12px marker covered the first digit of a full line number column: three digits, or more past row 999. The gutter now reserves a slot of its own at the left of the numbers, the marker size and a 4px gap, and the numbers and text move right by it. It is reserved while line numbers are shown, a marker renderer is set and a collection has a provider (`InputExtras::has_line_decorations`), so the gutter keeps its width as marked rows scroll in and out of view, and editors without a collection are unchanged. `row_bounds` summed the heights of the rows above, while text and line decorations are painted lower past a multi-line inline completion. The ghost lines' row and height are now part of `LastLayout`, and `LastLayout::row_extents` is the one row walk `row_bounds` and the line decoration layout share. Tests: markers stay in the slot left of three- and four-digit numbers, and in place while the text scrolls horizontally; the rows below two ghost lines are bounded and banded where they are painted. Each goes red with its fix taken out. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review of longbridge#3359: the style gains a field in this PR, and marking it non_exhaustive lets later fields land without breaking callers. Outside gpui-base it is built from `Default` and assigned; the styled input and the Base showcase are migrated, and the showcase's repeated copies of one style become one function. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
3fba497 to
d7d678e
Compare
|
Thanks for the review. Since 3fba497 the branch is rebased onto main at 8d8cc67, which brings in #3358, and has two new commits:
Checked locally: |
The maintainer requested changes on longbridge/gpui-kit#3359 (marker over a digit, row_bounds past inline completion ghost lines, InputEditorStyle non_exhaustive); all three are answered at d7d678e9, rebased onto upstream 8d8cc671. The dossier records what changed and that the pinned fork keeps its glyph over the leftmost digit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
I've implemented and squashed the remaining review fixes into one commit:
Targeted Rust tests, 9 documentation tests, compilation, Clippy and formatting checks passed. The updated native Gallery was built and launched for testing. This is an AI-assisted implementation. I cannot directly update this PR branch. Maintainer edits are enabled, but the fork belongs to the GigLaboCom organization. GitHub documents that the maintainer-edit permission applies to personal forks, not organization-owned forks (fork permissions). Both push attempts were rejected with HTTP 403 for my account, Please cherry-pick the single squashed fix onto your PR branch: git fetch https://github.com/longbridge/gpui-kit.git fix/3359-gutter-marker-scaling
git cherry-pick 8aa3bcbcPlease also update the Public API section to list the new builder/reader signatures. The styled editor sizes markers at 90% of the effective editor font size, with a gap of 30%; the slot is no longer fixed at 16px. For direct Base usage, marker size and gap default to zero and must be supplied by the presentation layer together with the renderer. |
Make new gutter presentation fields private and configure the renderer, size and gap through InputEditorStyle builders and readers. Base uses application-supplied metrics for gutter reservation and marker centering. Project matching icon and gutter dimensions from the input's effective font after size defaults and caller overrides. Cover rem scaling, editor-specific font changes, three- and four-digit line numbers and horizontal scrolling. Update both documentation locales. AI-assisted implementation.
|
Thanks for doing this. 8aa3bcb is on the PR branch as-is: it sat directly on d7d678e, so this was a fast-forward, with no rebase or force-push. The description now covers it:
Checked locally on 8aa3bcb: |
Description
Editors need whole-row annotations that text styles and range decorations
cannot express: diff rows, conflicts, bookmarks, breakpoints. This PR adds
LineDecorationCollection, which is owned likeTextDecorationCollectionand
RangeDecorationCollection. Each collection's entries come from aLineDecorationProvider, and the editor asks it about the visible bufferrows on every frame it paints.
A decoration paints a background band across its row, a marker in the
line-number gutter, or both.
GutterMarkeris a semantic,#[non_exhaustive]enum:DiffAdded,DiffRemoved,DiffChanged,Conflict,Bookmark,BreakpointandCustom.renderer, next to
fold_icon_renderer, that paints a theme-colored iconsized from the editor's font.
guides, selections and text, and continue across the gutter.
marker renderer and size are set and any collection has a provider, the
gutter reserves a slot at the left of the numbers: the marker size plus
a gap. The styled editor sizes the marker at 90% of its effective font
size and the gap at 30%, so the slot follows interface zoom and an
editor's
.text_size(...). A marker never covers a digit, and thegutter keeps its width as marked rows scroll in and out of view.
Because the provider is asked again on every frame, it owns its rows and
can read colors from the theme when asked. Rows are not edit-tracked and
are not cleared by
set_value. This suits annotations computed from anexternal model, such as a diff or a debugger session.
As in #3040, each owner holds its own collection, so independent features
cannot replace one another's decorations. Gutter markers were out of
scope there; this PR does not add gutter interaction, lanes or an
InputEvent.Also in this PR:
row_bounds(row): the window-space band a laid-out buffer row occupies,where it is painted, including below a multi-line inline completion.
text I-beam.
Public API
gpui-base(gpui_base::input)#[non_exhaustive] pub enum GutterMarker { DiffAdded, DiffRemoved, DiffChanged, Conflict, Bookmark, Breakpoint, Custom { icon: SharedString, color: Hsla } }: the meaning of a gutter marker;Customis an SVG asset path and color painted as given.pub struct LineDecoration: a background and/or marker for one zero-based buffer row.LineDecoration::new(row: usize) -> Self: an empty decoration for a row.LineDecoration::with_background(self, color: Hsla) -> Self: paint a band across the row.LineDecoration::with_marker(self, marker: GutterMarker) -> Self: paint a marker in its gutter.LineDecoration::row(&self) -> usize,background(&self) -> Option<Hsla>,marker(&self) -> Option<&GutterMarker>: readers.pub trait LineDecorationProvider { fn line_decorations(&self, rows: Range<usize>, cx: &App) -> Vec<LineDecoration>; }: supplies the decorations of the visible rows, asked every frame.pub struct LineDecorationCollection: the handle for one owner's provider.set_provider(&self, provider: Rc<dyn LineDecorationProvider>, cx: &mut App): replace the provider.clear(&self, cx: &mut App): stop painting and keep the handle.dispose(&self, cx: &mut App): release the collection and invalidate its clones.has_provider(&self, cx: &App) -> bool: whether there is a provider to ask.EditorState::create_line_decorations_collection(&mut self, provider: Rc<dyn LineDecorationProvider>, cx: &mut Context<Self>) -> LineDecorationCollection: create an independent collection.InputBaseState::row_bounds(&self, row: usize) -> Option<Bounds<Pixels>>: the band a laid-out buffer row occupies, gutter to right edge, across its soft wraps, moved down past inline completion ghost lines above it.pub type GutterMarkerRenderer = Rc<dyn Fn(&GutterMarker) -> AnyElement>: renders a marker within the size the style supplies.InputEditorStyle::with_gutter_marker_renderer(self, renderer: Option<GutterMarkerRenderer>) -> SelfandInputEditorStyle::gutter_marker_renderer(&self) -> Option<&GutterMarkerRenderer>: the presentation seam for markers; withNone, markers are not painted.InputEditorStyle::with_gutter_marker_size(self, size: impl Into<AbsoluteLength>) -> SelfandInputEditorStyle::gutter_marker_size(&self) -> AbsoluteLength: the square marker size, resolved against the window's rem at each layout and shared by the icon, the slot and vertical centering; defaults to zero, which paints no marker and reserves no slot.InputEditorStyle::with_gutter_marker_gap(self, gap: impl Into<AbsoluteLength>) -> SelfandInputEditorStyle::gutter_marker_gap(&self) -> AbsoluteLength: the space between a marker and the line numbers; defaults to zero.InputExtras::line_decorations(&self, rows: Range<usize>, cx: &App) -> Vec<LineDecoration>: defaulted; how the renderer reaches the editor's collections.InputExtras::has_line_decorations(&self) -> bool: defaulted tofalse; whether the gutter reserves the marker slot.#[non_exhaustive]onInputEditorStyle, whose three gutter marker fields are private: see Breaking Changes.gpui-component(gpui_component::input)pub use gpui_base::input::{GutterMarker, LineDecoration, LineDecorationCollection, LineDecorationProvider};.Input/Editorproject a marker renderer that uses the theme's success, danger, warning and info colors, with the marker at 90% of the editor's effective font size and the gap at 30%.Breaking Changes
InputEditorStyleis now#[non_exhaustive], and its new gutter markersettings are private fields behind builders. Outside
gpui-base, structliterals no longer compile, including those that end in
..Default::default(). Start fromDefaultand assign the existingfields to change:
Reading and assigning the existing fields is unchanged. Gutter markers are
configured through the builders; size and gap default to zero, so a Base
user supplies all three:
gpui-componentand the Base showcase are migrated in this PR.While a line decoration collection has a provider and line numbers are
shown, the styled editor's gutter is wider by 120% of its font size (the
marker and the gap), and the text starts that much further right. Editors
without a collection are unchanged.
InputExtrasgains two defaulted methods, so existing implementationscompile unchanged.
How to Test
cargo test -p gpui-base --locked input::cargo test -p gpui-component --locked input::cargo test -p gpui-kit --features test-support --test input --lockedcargo clippy --workspace --exclude gpui-shell --locked -- --deny warningscargo run, then Editor › Decorations: the last three rows show agreen band with
+, a red band with−, and a bookmark star, in aslot left of the line numbers. Edit above them and the marks follow
the text. Change Options → Font size and the markers and their slot
follow the font. Hover the gutter and the pointer is an arrow.
Checklist
cargo runfor story tests related to the changes. (Editor › Decorations on macOS. The gutter cursor was not checked visually.)The line backgrounds, gutter markers,
row_boundsand the gutter cursorwere first written as a fork of this repository for a merge editor and a
diff view; this PR is that work rebuilt on current
mainin the shape of#3040's decoration collections. Happy to split
row_boundsand thegutter cursor into their own PRs if that is easier to review.
🤖 Generated with Claude Code