Add caching of parsed completion documentation markdown to reduce flicker when selecting (#31546)

Related to #31460 and #28635.

Release Notes:

- Fixed redraw delay of documentation from language server completions
and added caching to reduce flicker when using arrow keys to change
selection.
This commit is contained in:
Michael Sloan 2025-05-27 17:12:38 -06:00 committed by GitHub
parent 31d908fc74
commit 506beafe10
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 300 additions and 95 deletions

1
Cargo.lock generated
View file

@ -9567,6 +9567,7 @@ dependencies = [
"assets",
"base64 0.22.1",
"env_logger 0.11.8",
"futures 0.3.31",
"gpui",
"language",
"languages",

View file

@ -4,8 +4,9 @@ use gpui::{
Size, StrikethroughStyle, StyledText, UniformListScrollHandle, div, px, uniform_list,
};
use gpui::{AsyncWindowContext, WeakEntity};
use language::Buffer;
use itertools::Itertools;
use language::CodeLabel;
use language::{Buffer, LanguageName, LanguageRegistry};
use markdown::{Markdown, MarkdownElement};
use multi_buffer::{Anchor, ExcerptId};
use ordered_float::OrderedFloat;
@ -15,6 +16,8 @@ use project::{CodeAction, Completion, TaskSourceKind};
use task::DebugScenario;
use task::TaskContext;
use std::collections::VecDeque;
use std::sync::Arc;
use std::{
cell::RefCell,
cmp::{Reverse, min},
@ -41,6 +44,25 @@ pub const MENU_ASIDE_X_PADDING: Pixels = px(16.);
pub const MENU_ASIDE_MIN_WIDTH: Pixels = px(260.);
pub const MENU_ASIDE_MAX_WIDTH: Pixels = px(500.);
// Constants for the markdown cache. The purpose of this cache is to reduce flickering due to
// documentation not yet being parsed.
//
// The size of the cache is set to the number of items fetched around the current selection plus one
// for the current selection and another to avoid cases where and adjacent selection exits the
// cache. The only current benefit of a larger cache would be doing less markdown parsing when the
// selection revisits items.
//
// One future benefit of a larger cache would be reducing flicker on backspace. This would require
// not recreating the menu on every change, by not re-querying the language server when
// `is_incomplete = false`.
const MARKDOWN_CACHE_MAX_SIZE: usize = MARKDOWN_CACHE_BEFORE_ITEMS + MARKDOWN_CACHE_AFTER_ITEMS + 2;
const MARKDOWN_CACHE_BEFORE_ITEMS: usize = 2;
const MARKDOWN_CACHE_AFTER_ITEMS: usize = 2;
// Number of items beyond the visible items to resolve documentation.
const RESOLVE_BEFORE_ITEMS: usize = 4;
const RESOLVE_AFTER_ITEMS: usize = 4;
pub enum CodeContextMenu {
Completions(CompletionsMenu),
CodeActions(CodeActionsMenu),
@ -148,13 +170,12 @@ impl CodeContextMenu {
pub fn render_aside(
&mut self,
editor: &Editor,
max_size: Size<Pixels>,
window: &mut Window,
cx: &mut Context<Editor>,
) -> Option<AnyElement> {
match self {
CodeContextMenu::Completions(menu) => menu.render_aside(editor, max_size, window, cx),
CodeContextMenu::Completions(menu) => menu.render_aside(max_size, window, cx),
CodeContextMenu::CodeActions(_) => None,
}
}
@ -162,7 +183,7 @@ impl CodeContextMenu {
pub fn focused(&self, window: &mut Window, cx: &mut Context<Editor>) -> bool {
match self {
CodeContextMenu::Completions(completions_menu) => completions_menu
.markdown_element
.get_or_create_entry_markdown(completions_menu.selected_item, cx)
.as_ref()
.is_some_and(|markdown| markdown.focus_handle(cx).contains_focused(window, cx)),
CodeContextMenu::CodeActions(_) => false,
@ -176,7 +197,7 @@ pub enum ContextMenuOrigin {
QuickActionBar,
}
#[derive(Clone, Debug)]
#[derive(Clone)]
pub struct CompletionsMenu {
pub id: CompletionId,
sort_completions: bool,
@ -191,7 +212,9 @@ pub struct CompletionsMenu {
show_completion_documentation: bool,
pub(super) ignore_completion_provider: bool,
last_rendered_range: Rc<RefCell<Option<Range<usize>>>>,
markdown_element: Option<Entity<Markdown>>,
markdown_cache: Rc<RefCell<VecDeque<(usize, Entity<Markdown>)>>>,
language_registry: Option<Arc<LanguageRegistry>>,
language: Option<LanguageName>,
snippet_sort_order: SnippetSortOrder,
}
@ -205,6 +228,9 @@ impl CompletionsMenu {
buffer: Entity<Buffer>,
completions: Box<[Completion]>,
snippet_sort_order: SnippetSortOrder,
language_registry: Option<Arc<LanguageRegistry>>,
language: Option<LanguageName>,
cx: &mut Context<Editor>,
) -> Self {
let match_candidates = completions
.iter()
@ -212,7 +238,7 @@ impl CompletionsMenu {
.map(|(id, completion)| StringMatchCandidate::new(id, &completion.label.filter_text()))
.collect();
Self {
let completions_menu = Self {
id,
sort_completions,
initial_position,
@ -226,9 +252,15 @@ impl CompletionsMenu {
scroll_handle: UniformListScrollHandle::new(),
resolve_completions: true,
last_rendered_range: RefCell::new(None).into(),
markdown_element: None,
markdown_cache: RefCell::new(VecDeque::with_capacity(MARKDOWN_CACHE_MAX_SIZE)).into(),
language_registry,
language,
snippet_sort_order,
}
};
completions_menu.start_markdown_parse_for_nearby_entries(cx);
completions_menu
}
pub fn new_snippet_choices(
@ -286,7 +318,9 @@ impl CompletionsMenu {
show_completion_documentation: false,
ignore_completion_provider: false,
last_rendered_range: RefCell::new(None).into(),
markdown_element: None,
markdown_cache: RefCell::new(VecDeque::new()).into(),
language_registry: None,
language: None,
snippet_sort_order,
}
}
@ -359,6 +393,7 @@ impl CompletionsMenu {
self.scroll_handle
.scroll_to_item(self.selected_item, ScrollStrategy::Top);
self.resolve_visible_completions(provider, cx);
self.start_markdown_parse_for_nearby_entries(cx);
if let Some(provider) = provider {
self.handle_selection_changed(provider, window, cx);
}
@ -433,11 +468,10 @@ impl CompletionsMenu {
// Expand the range to resolve more completions than are predicted to be visible, to reduce
// jank on navigation.
const EXTRA_TO_RESOLVE: usize = 4;
let entry_indices = util::iterate_expanded_and_wrapped_usize_range(
let entry_indices = util::expanded_and_wrapped_usize_range(
entry_range.clone(),
EXTRA_TO_RESOLVE,
EXTRA_TO_RESOLVE,
RESOLVE_BEFORE_ITEMS,
RESOLVE_AFTER_ITEMS,
entries.len(),
);
@ -467,14 +501,120 @@ impl CompletionsMenu {
cx,
);
let completion_id = self.id;
cx.spawn(async move |editor, cx| {
if let Some(true) = resolve_task.await.log_err() {
editor.update(cx, |_, cx| cx.notify()).ok();
editor
.update(cx, |editor, cx| {
// `resolve_completions` modified state affecting display.
cx.notify();
editor.with_completions_menu_matching_id(
completion_id,
|| (),
|this| this.start_markdown_parse_for_nearby_entries(cx),
);
})
.ok();
}
})
.detach();
}
fn start_markdown_parse_for_nearby_entries(&self, cx: &mut Context<Editor>) {
// Enqueue parse tasks of nearer items first.
//
// TODO: This means that the nearer items will actually be further back in the cache, which
// is not ideal. In practice this is fine because `get_or_create_markdown` moves the current
// selection to the front (when `is_render = true`).
let entry_indices = util::wrapped_usize_outward_from(
self.selected_item,
MARKDOWN_CACHE_BEFORE_ITEMS,
MARKDOWN_CACHE_AFTER_ITEMS,
self.entries.borrow().len(),
);
for index in entry_indices {
self.get_or_create_entry_markdown(index, cx);
}
}
fn get_or_create_entry_markdown(
&self,
index: usize,
cx: &mut Context<Editor>,
) -> Option<Entity<Markdown>> {
let entries = self.entries.borrow();
if index >= entries.len() {
return None;
}
let candidate_id = entries[index].candidate_id;
match &self.completions.borrow()[candidate_id].documentation {
Some(CompletionDocumentation::MultiLineMarkdown(source)) if !source.is_empty() => Some(
self.get_or_create_markdown(candidate_id, source.clone(), false, cx)
.1,
),
Some(_) => None,
_ => None,
}
}
fn get_or_create_markdown(
&self,
candidate_id: usize,
source: SharedString,
is_render: bool,
cx: &mut Context<Editor>,
) -> (bool, Entity<Markdown>) {
let mut markdown_cache = self.markdown_cache.borrow_mut();
if let Some((cache_index, (_, markdown))) = markdown_cache
.iter()
.find_position(|(id, _)| *id == candidate_id)
{
let markdown = if is_render && cache_index != 0 {
// Move the current selection's cache entry to the front.
markdown_cache.rotate_right(1);
let cache_len = markdown_cache.len();
markdown_cache.swap(0, (cache_index + 1) % cache_len);
&markdown_cache[0].1
} else {
markdown
};
let is_parsing = markdown.update(cx, |markdown, cx| {
// `reset` is called as it's possible for documentation to change due to resolve
// requests. It does nothing if `source` is unchanged.
markdown.reset(source, cx);
markdown.is_parsing()
});
return (is_parsing, markdown.clone());
}
if markdown_cache.len() < MARKDOWN_CACHE_MAX_SIZE {
let markdown = cx.new(|cx| {
Markdown::new(
source,
self.language_registry.clone(),
self.language.clone(),
cx,
)
});
// Handles redraw when the markdown is done parsing. The current render is for a
// deferred draw, and so without this did not redraw when `markdown` notified.
cx.observe(&markdown, |_, _, cx| cx.notify()).detach();
markdown_cache.push_front((candidate_id, markdown.clone()));
(true, markdown)
} else {
debug_assert_eq!(markdown_cache.capacity(), MARKDOWN_CACHE_MAX_SIZE);
// Moves the last cache entry to the start. The ring buffer is full, so this does no
// copying and just shifts indexes.
markdown_cache.rotate_right(1);
markdown_cache[0].0 = candidate_id;
let markdown = &markdown_cache[0].1;
markdown.update(cx, |markdown, cx| markdown.reset(source, cx));
(true, markdown.clone())
}
}
pub fn visible(&self) -> bool {
!self.entries.borrow().is_empty()
}
@ -625,7 +765,6 @@ impl CompletionsMenu {
fn render_aside(
&mut self,
editor: &Editor,
max_size: Size<Pixels>,
window: &mut Window,
cx: &mut Context<Editor>,
@ -644,33 +783,14 @@ impl CompletionsMenu {
plain_text: Some(text),
..
} => div().child(text.clone()),
CompletionDocumentation::MultiLineMarkdown(parsed) if !parsed.is_empty() => {
let markdown = self.markdown_element.get_or_insert_with(|| {
let markdown = cx.new(|cx| {
let languages = editor
.workspace
.as_ref()
.and_then(|(workspace, _)| workspace.upgrade())
.map(|workspace| workspace.read(cx).app_state().languages.clone());
let language = editor
.language_at(self.initial_position, cx)
.map(|l| l.name().to_proto());
Markdown::new(SharedString::default(), languages, language, cx)
});
// Handles redraw when the markdown is done parsing. The current render is for a
// deferred draw and so was not getting redrawn when `markdown` notified.
cx.observe(&markdown, |_, _, cx| cx.notify()).detach();
markdown
});
let is_parsing = markdown.update(cx, |markdown, cx| {
markdown.reset(parsed.clone(), cx);
markdown.is_parsing()
});
CompletionDocumentation::MultiLineMarkdown(source) if !source.is_empty() => {
let (is_parsing, markdown) =
self.get_or_create_markdown(mat.candidate_id, source.clone(), true, cx);
if is_parsing {
return None;
}
div().child(
MarkdownElement::new(markdown.clone(), hover_markdown_style(window, cx))
MarkdownElement::new(markdown, hover_markdown_style(window, cx))
.code_block_renderer(markdown::CodeBlockRenderer::Default {
copy_button: false,
copy_button_on_hover: false,
@ -882,13 +1002,7 @@ impl CompletionsMenu {
// another opened. `provider.selection_changed` should not be called in this case.
let this_menu_still_active = editor
.read_with(cx, |editor, _cx| {
if let Some(CodeContextMenu::Completions(completions_menu)) =
editor.context_menu.borrow().as_ref()
{
completions_menu.id == self.id
} else {
false
}
editor.with_completions_menu_matching_id(self.id, || false, |_| true)
})
.unwrap_or(false);
if this_menu_still_active {

View file

@ -4987,14 +4987,12 @@ impl Editor {
(buffer_position..buffer_position, None)
};
let completion_settings = language_settings(
buffer_snapshot
.language_at(buffer_position)
.map(|language| language.name()),
buffer_snapshot.file(),
cx,
)
.completions;
let language = buffer_snapshot
.language_at(buffer_position)
.map(|language| language.name());
let completion_settings =
language_settings(language.clone(), buffer_snapshot.file(), cx).completions;
// The document can be large, so stay in reasonable bounds when searching for words,
// otherwise completion pop-up might be slow to appear.
@ -5106,16 +5104,26 @@ impl Editor {
let menu = if completions.is_empty() {
None
} else {
let mut menu = CompletionsMenu::new(
id,
sort_completions,
show_completion_documentation,
ignore_completion_provider,
position,
buffer.clone(),
completions.into(),
snippet_sort_order,
);
let mut menu = editor.update(cx, |editor, cx| {
let languages = editor
.workspace
.as_ref()
.and_then(|(workspace, _)| workspace.upgrade())
.map(|workspace| workspace.read(cx).app_state().languages.clone());
CompletionsMenu::new(
id,
sort_completions,
show_completion_documentation,
ignore_completion_provider,
position,
buffer.clone(),
completions.into(),
snippet_sort_order,
languages,
language,
cx,
)
})?;
menu.filter(
if filter_completions {
@ -5190,6 +5198,22 @@ impl Editor {
}
}
pub fn with_completions_menu_matching_id<R>(
&self,
id: CompletionId,
on_absent: impl FnOnce() -> R,
on_match: impl FnOnce(&mut CompletionsMenu) -> R,
) -> R {
let mut context_menu = self.context_menu.borrow_mut();
let Some(CodeContextMenu::Completions(completions_menu)) = &mut *context_menu else {
return on_absent();
};
if completions_menu.id != id {
return on_absent();
}
on_match(completions_menu)
}
pub fn confirm_completion(
&mut self,
action: &ConfirmCompletion,
@ -8686,7 +8710,7 @@ impl Editor {
) -> Option<AnyElement> {
self.context_menu.borrow_mut().as_mut().and_then(|menu| {
if menu.visible() {
menu.render_aside(self, max_size, window, cx)
menu.render_aside(max_size, window, cx)
} else {
None
}

View file

@ -583,13 +583,6 @@ async fn parse_blocks(
language: Option<Arc<Language>>,
cx: &mut AsyncWindowContext,
) -> Option<Entity<Markdown>> {
let fallback_language_name = if let Some(ref l) = language {
let l = Arc::clone(l);
Some(l.lsp_id().clone())
} else {
None
};
let combined_text = blocks
.iter()
.map(|block| match &block.kind {
@ -607,7 +600,7 @@ async fn parse_blocks(
Markdown::new(
combined_text.into(),
Some(language_registry.clone()),
fallback_language_name,
language.map(|language| language.name()),
cx,
)
})

View file

@ -20,6 +20,7 @@ test-support = [
[dependencies]
base64.workspace = true
futures.workspace = true
gpui.workspace = true
language.workspace = true
linkify.workspace = true

View file

@ -67,14 +67,8 @@ struct MarkdownExample {
impl MarkdownExample {
pub fn new(text: SharedString, language_registry: Arc<LanguageRegistry>, cx: &mut App) -> Self {
let markdown = cx.new(|cx| {
Markdown::new(
text,
Some(language_registry),
Some("TypeScript".to_string()),
cx,
)
});
let markdown = cx
.new(|cx| Markdown::new(text, Some(language_registry), Some("TypeScript".into()), cx));
Self { markdown }
}
}

View file

@ -2,6 +2,8 @@ pub mod parser;
mod path_range;
use base64::Engine as _;
use futures::FutureExt as _;
use language::LanguageName;
use log::Level;
pub use path_range::{LineCol, PathWithRange};
@ -101,7 +103,7 @@ pub struct Markdown {
pending_parse: Option<Task<()>>,
focus_handle: FocusHandle,
language_registry: Option<Arc<LanguageRegistry>>,
fallback_code_block_language: Option<String>,
fallback_code_block_language: Option<LanguageName>,
options: Options,
copied_code_blocks: HashSet<ElementId>,
}
@ -144,7 +146,7 @@ impl Markdown {
pub fn new(
source: SharedString,
language_registry: Option<Arc<LanguageRegistry>>,
fallback_code_block_language: Option<String>,
fallback_code_block_language: Option<LanguageName>,
cx: &mut Context<Self>,
) -> Self {
let focus_handle = cx.focus_handle();
@ -310,9 +312,9 @@ impl Markdown {
if let Some(registry) = language_registry.as_ref() {
for name in language_names {
let language = if !name.is_empty() {
registry.language_for_name_or_extension(&name)
registry.language_for_name_or_extension(&name).left_future()
} else if let Some(fallback) = &fallback {
registry.language_for_name_or_extension(fallback)
registry.language_for_name(fallback.as_ref()).right_future()
} else {
continue;
};

View file

@ -670,7 +670,7 @@ pub fn measure<R>(label: &str, f: impl FnOnce() -> R) -> R {
}
}
pub fn iterate_expanded_and_wrapped_usize_range(
pub fn expanded_and_wrapped_usize_range(
range: Range<usize>,
additional_before: usize,
additional_after: usize,
@ -699,6 +699,43 @@ pub fn iterate_expanded_and_wrapped_usize_range(
}
}
/// Yields `[i, i + 1, i - 1, i + 2, ..]`, each modulo `wrap_length` and bounded by
/// `additional_before` and `additional_after`. If the wrapping causes overlap, duplicates are not
/// emitted. If wrap_length is 0, nothing is yielded.
pub fn wrapped_usize_outward_from(
start: usize,
additional_before: usize,
additional_after: usize,
wrap_length: usize,
) -> impl Iterator<Item = usize> {
let mut count = 0;
let mut after_offset = 1;
let mut before_offset = 1;
std::iter::from_fn(move || {
count += 1;
if count > wrap_length {
None
} else if count == 1 {
Some(start % wrap_length)
} else if after_offset <= additional_after && after_offset <= before_offset {
let value = (start + after_offset) % wrap_length;
after_offset += 1;
Some(value)
} else if before_offset <= additional_before {
let value = (start + wrap_length - before_offset) % wrap_length;
before_offset += 1;
Some(value)
} else if after_offset <= additional_after {
let value = (start + after_offset) % wrap_length;
after_offset += 1;
Some(value)
} else {
None
}
})
}
#[cfg(target_os = "windows")]
pub fn get_windows_system_shell() -> String {
use std::path::PathBuf;
@ -1462,49 +1499,88 @@ Line 3"#
}
#[test]
fn test_iterate_expanded_and_wrapped_usize_range() {
fn test_expanded_and_wrapped_usize_range() {
// Neither wrap
assert_eq!(
iterate_expanded_and_wrapped_usize_range(2..4, 1, 1, 8).collect::<Vec<usize>>(),
expanded_and_wrapped_usize_range(2..4, 1, 1, 8).collect::<Vec<usize>>(),
(1..5).collect::<Vec<usize>>()
);
// Start wraps
assert_eq!(
iterate_expanded_and_wrapped_usize_range(2..4, 3, 1, 8).collect::<Vec<usize>>(),
expanded_and_wrapped_usize_range(2..4, 3, 1, 8).collect::<Vec<usize>>(),
((0..5).chain(7..8)).collect::<Vec<usize>>()
);
// Start wraps all the way around
assert_eq!(
iterate_expanded_and_wrapped_usize_range(2..4, 5, 1, 8).collect::<Vec<usize>>(),
expanded_and_wrapped_usize_range(2..4, 5, 1, 8).collect::<Vec<usize>>(),
(0..8).collect::<Vec<usize>>()
);
// Start wraps all the way around and past 0
assert_eq!(
iterate_expanded_and_wrapped_usize_range(2..4, 10, 1, 8).collect::<Vec<usize>>(),
expanded_and_wrapped_usize_range(2..4, 10, 1, 8).collect::<Vec<usize>>(),
(0..8).collect::<Vec<usize>>()
);
// End wraps
assert_eq!(
iterate_expanded_and_wrapped_usize_range(3..5, 1, 4, 8).collect::<Vec<usize>>(),
expanded_and_wrapped_usize_range(3..5, 1, 4, 8).collect::<Vec<usize>>(),
(0..1).chain(2..8).collect::<Vec<usize>>()
);
// End wraps all the way around
assert_eq!(
iterate_expanded_and_wrapped_usize_range(3..5, 1, 5, 8).collect::<Vec<usize>>(),
expanded_and_wrapped_usize_range(3..5, 1, 5, 8).collect::<Vec<usize>>(),
(0..8).collect::<Vec<usize>>()
);
// End wraps all the way around and past the end
assert_eq!(
iterate_expanded_and_wrapped_usize_range(3..5, 1, 10, 8).collect::<Vec<usize>>(),
expanded_and_wrapped_usize_range(3..5, 1, 10, 8).collect::<Vec<usize>>(),
(0..8).collect::<Vec<usize>>()
);
// Both start and end wrap
assert_eq!(
iterate_expanded_and_wrapped_usize_range(3..5, 4, 4, 8).collect::<Vec<usize>>(),
expanded_and_wrapped_usize_range(3..5, 4, 4, 8).collect::<Vec<usize>>(),
(0..8).collect::<Vec<usize>>()
);
}
#[test]
fn test_wrapped_usize_outward_from() {
// No wrapping
assert_eq!(
wrapped_usize_outward_from(4, 2, 2, 10).collect::<Vec<usize>>(),
vec![4, 5, 3, 6, 2]
);
// Wrapping at end
assert_eq!(
wrapped_usize_outward_from(8, 2, 3, 10).collect::<Vec<usize>>(),
vec![8, 9, 7, 0, 6, 1]
);
// Wrapping at start
assert_eq!(
wrapped_usize_outward_from(1, 3, 2, 10).collect::<Vec<usize>>(),
vec![1, 2, 0, 3, 9, 8]
);
// All values wrap around
assert_eq!(
wrapped_usize_outward_from(5, 10, 10, 8).collect::<Vec<usize>>(),
vec![5, 6, 4, 7, 3, 0, 2, 1]
);
// None before / after
assert_eq!(
wrapped_usize_outward_from(3, 0, 0, 8).collect::<Vec<usize>>(),
vec![3]
);
// Starting point already wrapped
assert_eq!(
wrapped_usize_outward_from(15, 2, 2, 10).collect::<Vec<usize>>(),
vec![5, 6, 4, 7, 3]
);
// wrap_length of 0
assert_eq!(
wrapped_usize_outward_from(4, 2, 2, 0).collect::<Vec<usize>>(),
Vec::<usize>::new()
);
}
#[test]
fn test_truncate_lines_to_byte_limit() {
let text = "Line 1\nLine 2\nLine 3\nLine 4";