Skip to content

input: Let hosts choose how completion labels are highlighted - #3496

Open
xhofe wants to merge 2 commits into
longbridge:mainfrom
xhofe:completion-highlight
Open

xhofe wants to merge 2 commits into
longbridge:mainfrom
xhofe:completion-highlight

Conversation

@xhofe

@xhofe xhofe commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

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-component through input.

  • 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, using filter_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

CompletionMenuOptions is no longer Copy, because it now holds the highlighter.

- use_options(options);
- use_options(options);
+ use_options(options.clone());
+ use_options(options);

A struct literal that names only max_width no longer compiles. The new field is private, so fill it from Default:

- let options = CompletionMenuOptions { max_width: px(480.) };
+ let options = CompletionMenuOptions {
+     max_width: px(480.),
+     ..CompletionMenuOptions::default()
+ };

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::tests
  • In the Editor story, open Completions and type fl and log
  • Switch Options → Completion highlight between Prefix, Contains, and Characters, including while the menu is open
  • Confirm Prefix keeps only labels that start with the query, Contains keeps a contiguous hit, and Characters keeps an in-order match with gaps

Drafted with an AI coding agent.

Made with Cursor

The menu always painted a fixed prefix of the label. Hosts can now supply the ranges, including contiguous and character-by-character matches.
@huacnlee huacnlee added input Breaking Changes Contains breaking changes; excluded from automatic cherry-picks to stable. labels Oct 14, 2026 — with ChatGPT Codex Connector

Copy link
Copy Markdown
Member

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: CompletionMenuOptions { max_width: px(480.), ..CompletionMenuOptions::default() } is not valid for downstream crates after adding the private highlight field (current head). Rust's struct-update syntax still requires visibility of the fields supplied by ... With the API in this patch, a valid migration is:

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 b109b8cb341e33e2f7b94ef632b49d50ce24a16f; this is a targeted source review, not a full PR review or a fresh build.

@huacnlee huacnlee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

  1. [P1] Preserve compatibility of CompletionMenuOptions (crates/base/src/input/editor/lsp/completions.rs:117–125). Removing Copy and 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 new highlight field is private. Please keep the existing options type compatible and expose highlighting through a separate configuration entry point.

  2. [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.

@huacnlee
huacnlee enabled auto-merge (squash) October 14, 2026 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking Changes Contains breaking changes; excluded from automatic cherry-picks to stable. input

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants