Repository navigation
Conversation
The menu always painted a fixed prefix of the label. Hosts can now supply the ranges, including contiguous and character-by-character matches.
|
This PR makes completion-label highlighting host-configurable while retaining the existing prefix matcher by default. That is a reasonable scope for the requested customization. One concrete documentation issue in the Breaking Changes migration: let mut options = CompletionMenuOptions::default();
options.max_width = px(480.);Please replace that migration example, and ideally validate it from an external-crate example or doctest so module-local access cannot mask this issue. Verified against the exact source at |
huacnlee
left a comment
There was a problem hiding this comment.
This PR lets hosts customize completion-label highlighting, adds contiguous and character-by-character matchers, and demonstrates them in the Editor story. The requirement and callback approach are reasonable, but two issues need to be addressed before merging.
-
[P1] Preserve compatibility of
CompletionMenuOptions(crates/base/src/input/editor/lsp/completions.rs:117–125). RemovingCopyand adding a private field break existing consumers that copy the options or construct them with a struct literal, contrary to the repository's backward-compatibility requirement. The migration example in the description using..CompletionMenuOptions::default()also fails with E0451 because the newhighlightfield is private. Please keep the existing options type compatible and expose highlighting through a separate configuration entry point. -
[P2] Normalize custom highlight ranges before passing them to
StyledText(crates/component/src/input/popovers/completion_menu.rs:20–32). The current validator accepts out-of-order and overlapping ranges. GPUI's text-run construction assumes ordered, non-overlapping ranges: for label"flag",[2..3, 0..1]produces 7 bytes of text runs for a 4-byte label, and[0..3, 2..4]produces 5. This can cause an out-of-bounds slice in the Linux font-fallback shaping path. Please sort and merge overlapping ranges and add focused regression coverage for these inputs.
Validation: reviewed the complete diff, related implementations, and GPUI text rendering; confirmed the private-field migration error with a minimal Rust compiler probe and the range-length mismatch with a focused probe using the PR's validator. git diff --check passes and the reported CI checks pass. I did not run the full local test suite or Story interaction checks.
Summary
The completion menu always painted a fixed prefix of each label, starting at the first byte and lasting as long as the typed word. Hosts can now supply the byte ranges to paint. Contiguous and character-by-character matchers are included, and the Editor story's Options menu shows both.
Public API
gpui-base
Re-exported from
gpui-componentthroughinput.pub type CompletionHighlighter = dyn Fn(&str, &CompletionItem) -> Vec<Range<usize>>— byte ranges of a completion label to paint as the match.pub fn highlight_prefix(query: &str, item: &CompletionItem) -> Vec<Range<usize>>— paints a prefix of the label, usingfilter_text's length when the item sets one.pub fn highlight_contains(query: &str, item: &CompletionItem) -> Vec<Range<usize>>— paints the first case-insensitive contiguous occurrence of the query.pub fn highlight_characters(query: &str, item: &CompletionItem) -> Vec<Range<usize>>— paints each query character in order, allowing gaps, and merges adjacent hits.pub fn CompletionMenuOptions::with_highlight(self, highlight: impl Fn(&str, &CompletionItem) -> Vec<Range<usize>> + 'static) -> Self— installs the highlighter the menu calls per row.pub fn CompletionMenuOptions::highlight(&self) -> &Rc<CompletionHighlighter>— returns the highlighter currently installed.Breaking Changes
CompletionMenuOptionsis no longerCopy, because it now holds the highlighter.A struct literal that names only
max_widthno longer compiles. The new field is private, so fill it fromDefault:The default highlighter is still
highlight_prefix, so an untouched menu paints the same prefix as before. That prefix is now shortened to a character boundary instead of splitting a multibyte character.Test plan
cargo test -p gpui-base --lib completions::testsflandlogDrafted with an AI coding agent.
Made with Cursor