Fix some completion docs render delays (#31486)

Michael Sloan created 6 months ago

Closes #31460

While this is now much better than it was, the documentation still
flickers when changing selection. Hoping to fix that, but it will be a
much more involved change. So leaving release notes as "N/A" for now, in
anticipation of the full fix.

Release Notes:

- N/A

Change summary

Cargo.lock                              |  1 
crates/editor/src/code_context_menus.rs | 14 +++++-
crates/markdown/Cargo.toml              |  1 
crates/markdown/src/markdown.rs         | 57 ++++++++++++++------------
4 files changed, 42 insertions(+), 31 deletions(-)

Detailed changes

Cargo.lock

@@ -9563,7 +9563,6 @@ checksum = "3e2e65a1a2e43cfcb47a895c4c8b10d1f4a61097f9f254f183aee60cad9c651d"
 name = "markdown"
 version = "0.1.0"
 dependencies = [
- "anyhow",
  "assets",
  "base64 0.22.1",
  "env_logger 0.11.8",

crates/editor/src/code_context_menus.rs

@@ -646,7 +646,7 @@ impl CompletionsMenu {
             } => div().child(text.clone()),
             CompletionDocumentation::MultiLineMarkdown(parsed) if !parsed.is_empty() => {
                 let markdown = self.markdown_element.get_or_insert_with(|| {
-                    cx.new(|cx| {
+                    let markdown = cx.new(|cx| {
                         let languages = editor
                             .workspace
                             .as_ref()
@@ -656,11 +656,19 @@ impl CompletionsMenu {
                             .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
                 });
-                markdown.update(cx, |markdown, cx| {
+                let is_parsing = markdown.update(cx, |markdown, cx| {
                     markdown.reset(parsed.clone(), cx);
+                    markdown.is_parsing()
                 });
+                if is_parsing {
+                    return None;
+                }
                 div().child(
                     MarkdownElement::new(markdown.clone(), hover_markdown_style(window, cx))
                         .code_block_renderer(markdown::CodeBlockRenderer::Default {

crates/markdown/Cargo.toml

@@ -19,7 +19,6 @@ test-support = [
 ]
 
 [dependencies]
-anyhow.workspace = true
 base64.workspace = true
 gpui.workspace = true
 language.workspace = true

crates/markdown/src/markdown.rs

@@ -30,7 +30,7 @@ use pulldown_cmark::Alignment;
 use sum_tree::TreeMap;
 use theme::SyntaxTheme;
 use ui::{Tooltip, prelude::*};
-use util::{ResultExt, TryFutureExt};
+use util::ResultExt;
 
 use crate::parser::CodeBlockKind;
 
@@ -98,7 +98,7 @@ pub struct Markdown {
     parsed_markdown: ParsedMarkdown,
     images_by_source_offset: HashMap<usize, Arc<Image>>,
     should_reparse: bool,
-    pending_parse: Option<Task<Option<()>>>,
+    pending_parse: Option<Task<()>>,
     focus_handle: FocusHandle,
     language_registry: Option<Arc<LanguageRegistry>>,
     fallback_code_block_language: Option<String>,
@@ -192,6 +192,10 @@ impl Markdown {
         this
     }
 
+    pub fn is_parsing(&self) -> bool {
+        self.pending_parse.is_some()
+    }
+
     pub fn source(&self) -> &str {
         &self.source
     }
@@ -219,6 +223,7 @@ impl Markdown {
         self.parse(cx);
     }
 
+    #[cfg(feature = "test-support")]
     pub fn parsed_markdown(&self) -> &ParsedMarkdown {
         &self.parsed_markdown
     }
@@ -275,14 +280,19 @@ impl Markdown {
             self.should_reparse = true;
             return;
         }
+        self.should_reparse = false;
+        self.pending_parse = Some(self.start_background_parse(cx));
+    }
 
+    fn start_background_parse(&self, cx: &Context<Self>) -> Task<()> {
         let source = self.source.clone();
         let should_parse_links_only = self.options.parse_links_only;
         let language_registry = self.language_registry.clone();
         let fallback = self.fallback_code_block_language.clone();
+
         let parsed = cx.background_spawn(async move {
             if should_parse_links_only {
-                return anyhow::Ok((
+                return (
                     ParsedMarkdown {
                         events: Arc::from(parse_links_only(source.as_ref())),
                         source,
@@ -290,8 +300,9 @@ impl Markdown {
                         languages_by_path: TreeMap::default(),
                     },
                     Default::default(),
-                ));
+                );
             }
+
             let (events, language_names, paths) = parse_markdown(&source);
             let mut images_by_source_offset = HashMap::default();
             let mut languages_by_name = TreeMap::default();
@@ -343,7 +354,7 @@ impl Markdown {
                 }
             }
 
-            anyhow::Ok((
+            (
                 ParsedMarkdown {
                     source,
                     events: Arc::from(events),
@@ -351,29 +362,23 @@ impl Markdown {
                     languages_by_path,
                 },
                 images_by_source_offset,
-            ))
+            )
         });
 
-        self.should_reparse = false;
-        self.pending_parse = Some(cx.spawn(async move |this, cx| {
-            async move {
-                let (parsed, images_by_source_offset) = parsed.await?;
-
-                this.update(cx, |this, cx| {
-                    this.parsed_markdown = parsed;
-                    this.images_by_source_offset = images_by_source_offset;
-                    this.pending_parse.take();
-                    if this.should_reparse {
-                        this.parse(cx);
-                    }
-                    cx.notify();
-                })
-                .ok();
-                anyhow::Ok(())
-            }
-            .log_err()
-            .await
-        }));
+        cx.spawn(async move |this, cx| {
+            let (parsed, images_by_source_offset) = parsed.await;
+
+            this.update(cx, |this, cx| {
+                this.parsed_markdown = parsed;
+                this.images_by_source_offset = images_by_source_offset;
+                this.pending_parse.take();
+                if this.should_reparse {
+                    this.parse(cx);
+                }
+                cx.refresh_windows();
+            })
+            .ok();
+        })
     }
 }