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
Brings in the final revision of longbridge#3040 by 胡飞 (feigeCode), which was closed unmerged. It is the shared base the review asked for: one annotation kind, a tracked geometric range, with gutter lanes and inline widgets left out. - Add `RangeDecorationCollection` with `set` / `append` / `clear` / `dispose` / `get_ranges`. Each collection owns only its own entries, dropping a handle does not clear it, and there are no public IDs. - `RangeDecoration::new(range)` with `with_style(Fill | Frame)` and `with_color`. `Fill` paints behind the text, `Frame` outlines it with a one-pixel stroke. `RangeDecorationStyle` is `#[non_exhaustive]`. - Text and geometric decorations share UTF-8 normalization and edit tracking through `TrackedDecoration`: insertion at an edge does not grow the range, insertion inside does, replacement clips, deletion removes empty ranges. Undo, redo, `set_value` and `replace_all` apply the same transforms; decorations are not undo history. - Ranges are clipped to the visible byte range and projected through the shaped lines, so a range crossing the viewport edge, a soft wrap, a CRLF or a fold paints its visible part instead of disappearing. - An interval index is queried once per visible, non-folded buffer span; edits update the affected entries linearly without re-sorting. - Beyond the PR: `set` with entries equal to the current ones changes nothing and does not notify, so an outline that follows the cursor can be refreshed from an observer of the editor state without re-entering it; `append` of nothing is likewise a no-op. - Update the English and Chinese docs and the Editor story. Co-authored-by: 胡飞 <1835698775@qq.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015apbabjAqXKDxR4gfNGeF2
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: