input: collection-owned tracked range decorations - #3040
Conversation
huacnlee
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I found several blockers in the new annotation APIs:
-
EditorAnnotations::adjust_for_editupdates range decorations and inline widgets but not gutter markers. Inserting or deleting lines before a marker leaves it on the old logical row, contrary to the promised document anchoring. Please update marker anchors for edits and add newline insertion/deletion regressions. -
Both
layout_match_rangeandlayout_range_cornersreject a range unless it is wholly insidevisible_range_offset. A range crossing the top or bottom viewport boundary therefore disappears while its middle is visible. Please clip to the visible byte range and test both boundaries, wrapping, and folds. -
Inline widgets are painted after document layout without reserving space. A widget before suffix text overpaints that text, and the API accepts only text rather than the described non-textual element. Please either implement an actual layout-participating element contract or narrow the API/name/documentation to the behavior that can be supported, with UI integration coverage.
-
Gutter marker activation is pointer-only (
on_mouse_down). Please provide a focus/keyboard action path and appropriate accessibility semantics so breakpoint-style commands do not depend on a mouse. -
Adding
GutterMarkerMouseDownto the shared publicInputEventbreaks downstream exhaustive matches (the unrelated match updates in this PR demonstrate this). Please use an editor-specific event/callback or document and deliberately handle the compatibility strategy. -
The paint path scans every stored annotation each frame; gutter layout additionally clones/sorts visible markers and repeatedly searches/sums visible lines. Please index/cull this work or provide measurements showing it remains acceptable for realistic diagnostic/search-sized collections.
Please add UI integration tests for marker activation and keyboard access, partially visible decorations, inline layout, wrapping/folding, and representative large collections. The existing range-adjustment unit tests and passing CI do not exercise these paths.
Security: PASS for the affected in-process rendering/event boundary.
Please address the findings above, then request my review again.
huacnlee
left a comment
There was a problem hiding this comment.
补充产品范围与公共 API 审查:
底层需求本身成立,但这三个能力目前并不具备相同的成熟度,不适合一起进入核心 Editor API。
- **Gutter marker:**适合作为 breakpoint、diagnostic、diff 和 bookmark 共用的核心能力,但抽象应该是通用的 gutter/lane,而不是固定为单个 22px icon lane。
- **Geometric range decoration:**需求合理,可以表达现有
TextDecoration无法覆盖的几何效果;但应复用现有 collection ownership 和 tracked range 模型,而不是再增加一套全局替换 API。 - **Inline widget:**建议从本 PR 移除并推迟。当前实现只是覆盖绘制文字,不是参与布局的 widget。真正的 API 需要定义测量、空间占用、换行、hit testing、focus、键盘操作和 accessibility。
当前 API 还有以下结构性问题:
set_gutter_markers、set_range_decorations和set_inline_widgets都会替换一个全局Vec,多个独立 feature 会互相覆盖。请延续现有TextDecorationCollection模型,让每个 feature 持有生命周期独立的 collection/handle。RangeDecoration和InlineWidget都要求 ID,但 ID 不能用于 keyed reconciliation、定向更新/删除、bounds 查询或事件处理。要么让 ID 真正承担稳定 identity,要么不要在整体替换的数据中强制要求 ID。- logical row、byte range 和 byte offset 三种 anchor 的编辑语义不一致。建议统一为 tracked anchor/range,并明确插入边界 affinity/stickiness,以及删除、整篇替换、undo/redo、fold 和 formatting 时的行为。
- styled
Editor::render会把 gutter renderer 写入 retainedEditorState。这混合了 presentation 与 document state,也会在 render 阶段修改状态;同一个 state 被多个 view 使用时,不同 renderer 还会互相覆盖。Renderer 应保留在当前 element/skin,通过 presentation seam 传入 Base。 GutterMarker一方面包含 icon token 和 tooltip,另一方面又要求 application renderer,造成 presentation ownership 重复,并在 Base 中固化了 marker 必须是 icon 的假设。Base 应只持有语义 identity、anchor 和 state,具体表现由 component 层负责。- 请使用 editor-specific event/callback,不要扩展所有 Input 共用的
InputEvent。对未来可能增加 variant 的RangeDecorationStyle等公共 enum,也应考虑#[non_exhaustive]。 - 公共模型不应过早固定为每行一个 marker、只有一个 lane。多个 owner 共存时会需要 lane identity、顺序/冲突策略,以及随 zoom/density 缩放的 geometry。
产品建议:接受 gutter marker 和 geometric range decoration 的需求方向;推迟 inline widget;先建立统一的、collection-owned、可跟踪编辑的 annotation 基础模型,再拆成较小的独立改动。产品范围仍需 maintainer 明确决定,不能仅凭实现完整度默认接受全部三个能力。
Range decorations become an independently owned collection instead of a global annotation list, and gutter markers and inline widgets are split out of this change. - Add `RangeDecorationCollection` with `set` / `append` / `clear` / `dispose` / `get_ranges`. Each collection owns only its own entries, so separate extensions cannot overwrite one another, and dropping a handle does not clear it. Public decoration IDs are gone; the handle owns lifetime. - Share UTF-8 normalization and edit tracking between text and geometric decorations through a `TrackedDecoration` abstraction. - Clip and project ranges through the shaped lines, so soft wraps, continuation indents, CRLF and folds no longer make a partly visible range disappear. - Add an interval index queried once per visible, non-folded buffer span; edits update affected entries linearly without re-sorting. - Remove gutter markers, gutter lane reservation, inline widgets, the `InputEvent::GutterMarkerMouseDown` variant and the `Editor` gutter renderer projection, plus their story/shell exhaustive-match arms. - Update the English and Chinese docs and the Editor story. Co-Authored-By: Claude <noreply@anthropic.com>
0fae175 to
b74d954
Compare
Problem
Features that need editor affordances have nowhere to draw. There is a gutter rendered in
element.rs, but nothing anchors a highlight to a byte range, so current-line emphasis, conflict regions, review hunks, or search-hit backgrounds end up needing a fork or a hack layered outside ofInput.This adds one annotation kind — a tracked geometric range — as a shared base, and deliberately leaves gutter lanes and inline widgets out for follow-up PRs. Those were part of the earlier revision of this PR; the review asked to do the shared base first and split the feature work, so they are removed here.
1.
RangeDecorationCollection— collection-owned tracked rangesEditorState::create_range_decorations_collection(decorations, cx)returns an independent handle withset/append/clear/dispose/get_ranges. Each collection owns only its own entries, so separate extensions cannot overwrite one another, and dropping the handle does not clear it.RangeDecoration::new(range)with.with_style(RangeDecorationStyle::{Fill, Frame})and.with_color(color).Fillpaints behind the text,Frameoutlines it — useful for current-line emphasis, conflict regions, or search-hit background.TrackedDecorationabstraction: insertion at either edge does not expand, insertion inside does, replacement clips overlapping anchors, and deletion removes empty ranges. Undo/redo,set_value,replace_alland formatting apply the same transforms. Decorations are not snapshots in undo history — a deleted decoration is not resurrected by undo.2. Viewport clipping and folds
Ranges are clipped and projected through the actual shaped lines, so soft-wrapped continuations, wrap indents, CRLF, and folds are handled by clipping rather than disappearing when only part of a range is visible. Hidden buffer spans are skipped.
3. Indexing
An interval index (subtree max-end) is consulted once per visible, non-folded buffer span, so one document-spanning decoration does not force a full scan every frame. Setting/appending rebuilds that collection's index; edits update affected entries linearly without re-sorting.
Note on the
crates/shelland story changesThe
InputEvent::GutterMarkerMouseDownvariant and its exhaustive-match arms are removed along with gutter markers;crates/shelland the stories only drop those arms. Nothing else in those files changes.Follow-ups (not in this PR)
Validation
cargo test -p gpui-base --locked— 935 unit + 1 integration passcargo test -p gpui-component input:: --locked— 32 passcargo test -p gpui-kit --features test-support --test input --locked— 6 passcargo check -p gpui-component-story --lockedcargo clippy -p gpui-base --all-targets --locked -- -D warningscargo fmt --all -- --check,git diff --checkFive new window-layout tests cover scroll clipping, soft wrap/indent, CRLF, folds, and edit/collection lifetime, plus a 100,001-decoration index culling test. These are automated layout tests; no desktop manual pass.
Release Notes: