Fix markdown removal in hover handling whitespace weirdly

crates/ide/src/hover.rs

@@ -19,6 +19,7 @@
 
 use crate::{
     doc_links::token_as_doc_comment,
+    markdown_remove::remove_markdown,
     markup::Markup,
     runnables::{runnable_fn, runnable_mod},
     FileId, FilePosition, NavigationTarget, RangeInfo, Runnable, TryToNav,
@@ -26,14 +27,9 @@
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct HoverConfig {
     pub links_in_hover: bool,
-    pub documentation: Option<HoverDocFormat>,
+    pub documentation: bool,
     pub keywords: bool,
-}
+    pub format: HoverDocFormat,
-
-impl HoverConfig {
-    fn markdown(&self) -> bool {
-        matches!(self.documentation, Some(HoverDocFormat::Markdown))
-    }
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -90,12 +86,23 @@ pub struct HoverResult {
 // image::https://user-images.githubusercontent.com/48062697/113020658-b5f98b80-917a-11eb-9f88-3dbc27320c95.gif[]
 pub(crate) fn hover(
     db: &RootDatabase,
-    FileRange { file_id, range }: FileRange,
+    file_range: FileRange,
     config: &HoverConfig,
 ) -> Option<RangeInfo<HoverResult>> {
     let sema = &hir::Semantics::new(db);
-    let file = sema.parse(file_id).syntax().clone();
+    let mut res = hover_impl(sema, file_range, config)?;
+    if let HoverDocFormat::PlainText = config.format {
+        res.info.markup = remove_markdown(res.info.markup.as_str()).into();
+    }
+    Some(res)
+}
 
+fn hover_impl(
+    sema: &Semantics<'_, RootDatabase>,
+    FileRange { file_id, range }: FileRange,
+    config: &HoverConfig,
+) -> Option<RangeInfo<HoverResult>> {
+    let file = sema.parse(file_id).syntax().clone();
     if !range.is_empty() {
         return hover_ranged(&file, range, sema, config);
     }

crates/ide/src/hover/render.rs

@@ -26,13 +26,12 @@
 use crate::{
     doc_links::{remove_links, rewrite_links},
     hover::walk_and_push_ty,
-    markdown_remove::remove_markdown,
     HoverAction, HoverConfig, HoverResult, Markup,
 };
 
 pub(super) fn type_info(
     sema: &Semantics<'_, RootDatabase>,
-    config: &HoverConfig,
+    _config: &HoverConfig,
     expr_or_pat: &Either<ast::Expr, ast::Pat>,
 ) -> Option<HoverResult> {
     let TypeInfo { original, adjusted } = match expr_or_pat {
@@ -55,19 +54,15 @@ pub(super) fn type_info(
         let adjusted = adjusted_ty.display(sema.db).to_string();
         let static_text_diff_len = "Coerced to: ".len() - "Type: ".len();
         format!(
-            "{bt_start}Type: {:>apad$}\nCoerced to: {:>opad$}\n{bt_end}",
+            "```text\nType: {:>apad$}\nCoerced to: {:>opad$}\n```\n",
             original,
             adjusted,
             apad = static_text_diff_len + adjusted.len().max(original.len()),
             opad = original.len(),
-            bt_start = if config.markdown() { "```text\n" } else { "" },
-            bt_end = if config.markdown() { "```\n" } else { "" }
         )
         .into()
-    } else if config.markdown() {
-        Markup::fenced_block(&original.display(sema.db))
     } else {
-        original.display(sema.db).to_string().into()
+        Markup::fenced_block(&original.display(sema.db))
     };
     res.actions.push(HoverAction::goto_type_from_targets(sema.db, targets));
     Some(res)
@@ -75,7 +70,7 @@ pub(super) fn type_info(
 
 pub(super) fn try_expr(
     sema: &Semantics<'_, RootDatabase>,
-    config: &HoverConfig,
+    _config: &HoverConfig,
     try_expr: &ast::TryExpr,
 ) -> Option<HoverResult> {
     let inner_ty = sema.type_of_expr(&try_expr.expr()?)?.original;
@@ -151,14 +146,12 @@ pub(super) fn try_expr(
     let ppad = static_text_len_diff.min(0).abs() as usize;
 
     res.markup = format!(
-        "{bt_start}{} Type: {:>pad0$}\nPropagated as: {:>pad1$}\n{bt_end}",
+        "```text\n{} Type: {:>pad0$}\nPropagated as: {:>pad1$}\n```\n",
         s,
         inner_ty,
         body_ty,
         pad0 = ty_len_max + tpad,
         pad1 = ty_len_max + ppad,
-        bt_start = if config.markdown() { "```text\n" } else { "" },
-        bt_end = if config.markdown() { "```\n" } else { "" }
     )
     .into();
     Some(res)
@@ -166,7 +159,7 @@ pub(super) fn try_expr(
 
 pub(super) fn deref_expr(
     sema: &Semantics<'_, RootDatabase>,
-    config: &HoverConfig,
+    _config: &HoverConfig,
     deref_expr: &ast::PrefixExpr,
 ) -> Option<HoverResult> {
     let inner_ty = sema.type_of_expr(&deref_expr.expr()?)?.original;
@@ -195,15 +188,13 @@ pub(super) fn deref_expr(
             .max(adjusted.len() + coerced_len)
             .max(inner.len() + deref_len);
         format!(
-            "{bt_start}Dereferenced from: {:>ipad$}\nTo type: {:>apad$}\nCoerced to: {:>opad$}\n{bt_end}",
+            "```text\nDereferenced from: {:>ipad$}\nTo type: {:>apad$}\nCoerced to: {:>opad$}\n```\n",
             inner,
             original,
             adjusted,
             ipad = max_len - deref_len,
             apad = max_len - type_len,
             opad = max_len - coerced_len,
-            bt_start = if config.markdown() { "```text\n" } else { "" },
-            bt_end = if config.markdown() { "```\n" } else { "" }
         )
         .into()
     } else {
@@ -213,13 +204,11 @@ pub(super) fn deref_expr(
         let deref_len = "Dereferenced from: ".len();
         let max_len = (original.len() + type_len).max(inner.len() + deref_len);
         format!(
-            "{bt_start}Dereferenced from: {:>ipad$}\nTo type: {:>apad$}\n{bt_end}",
+            "```text\nDereferenced from: {:>ipad$}\nTo type: {:>apad$}\n```\n",
             inner,
             original,
             ipad = max_len - deref_len,
             apad = max_len - type_len,
-            bt_start = if config.markdown() { "```text\n" } else { "" },
-            bt_end = if config.markdown() { "```\n" } else { "" }
         )
         .into()
     };
@@ -233,7 +222,7 @@ pub(super) fn keyword(
     config: &HoverConfig,
     token: &SyntaxToken,
 ) -> Option<HoverResult> {
-    if !token.kind().is_keyword() || !config.documentation.is_some() || !config.keywords {
+    if !token.kind().is_keyword() || !config.documentation || !config.keywords {
         return None;
     }
     let parent = token.parent()?;
@@ -257,7 +246,7 @@ pub(super) fn keyword(
 /// i.e. `let S {a, ..} = S {a: 1, b: 2}`
 pub(super) fn struct_rest_pat(
     sema: &Semantics<'_, RootDatabase>,
-    config: &HoverConfig,
+    _config: &HoverConfig,
     pattern: &RecordPat,
 ) -> HoverResult {
     let missing_fields = sema.record_pattern_missing_fields(pattern);
@@ -286,11 +275,7 @@ pub(super) fn struct_rest_pat(
         // get rid of trailing comma
         s.truncate(s.len() - 2);
 
-        if config.markdown() {
+        Markup::fenced_block(&s)
-            Markup::fenced_block(&s)
-        } else {
-            s.into()
-        }
     };
     res.actions.push(HoverAction::goto_type_from_targets(sema.db, targets));
     res
@@ -344,13 +329,8 @@ pub(super) fn process_markup(
     config: &HoverConfig,
 ) -> Markup {
     let markup = markup.as_str();
-    let markup = if !config.markdown() {
+    let markup =
-        remove_markdown(markup)
+        if config.links_in_hover { rewrite_links(db, markup, def) } else { remove_links(markup) };
-    } else if config.links_in_hover {
-        rewrite_links(db, markup, def)
-    } else {
-        remove_links(markup)
-    };
     Markup::from(markup)
 }
 
@@ -463,8 +443,9 @@ pub(super) fn definition(
         Definition::DeriveHelper(it) => (format!("derive_helper {}", it.name(db)), None),
     };
 
-    let docs = match config.documentation {
+    let docs = docs
-        Some(_) => docs.or_else(|| {
+        .filter(|_| config.documentation)
+        .or_else(|| {
             // docs are missing, for assoc items of trait impls try to fall back to the docs of the
             // original item of the trait
             let assoc = def.as_assoc_item(db)?;
@@ -472,10 +453,8 @@ pub(super) fn definition(
             let name = Some(assoc.name(db)?);
             let item = trait_.items(db).into_iter().find(|it| it.name(db) == name)?;
             item.docs(db)
-        }),
+        })
-        None => None,
+        .map(Into::into);
-    };
-    let docs = docs.filter(|_| config.documentation.is_some()).map(Into::into);
     markup(docs, label, mod_path)
 }
 

crates/ide/src/hover/tests.rs

@@ -2,7 +2,7 @@
 use ide_db::base_db::{FileLoader, FileRange};
 use syntax::TextRange;
 
-use crate::{fixture, hover::HoverDocFormat, HoverConfig};
+use crate::{fixture, HoverConfig, HoverDocFormat};
 
 fn check_hover_no_result(ra_fixture: &str) {
     let (analysis, position) = fixture::position(ra_fixture);
@@ -10,8 +10,9 @@ fn check_hover_no_result(ra_fixture: &str) {
         .hover(
             &HoverConfig {
                 links_in_hover: true,
-                documentation: Some(HoverDocFormat::Markdown),
+                documentation: true,
                 keywords: true,
+                format: HoverDocFormat::Markdown,
             },
             FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
         )
@@ -26,8 +27,9 @@ fn check(ra_fixture: &str, expect: Expect) {
         .hover(
             &HoverConfig {
                 links_in_hover: true,
-                documentation: Some(HoverDocFormat::Markdown),
+                documentation: true,
                 keywords: true,
+                format: HoverDocFormat::Markdown,
             },
             FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
         )
@@ -47,8 +49,9 @@ fn check_hover_no_links(ra_fixture: &str, expect: Expect) {
         .hover(
             &HoverConfig {
                 links_in_hover: false,
-                documentation: Some(HoverDocFormat::Markdown),
+                documentation: true,
                 keywords: true,
+                format: HoverDocFormat::Markdown,
             },
             FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
         )
@@ -68,8 +71,9 @@ fn check_hover_no_markdown(ra_fixture: &str, expect: Expect) {
         .hover(
             &HoverConfig {
                 links_in_hover: true,
-                documentation: Some(HoverDocFormat::PlainText),
+                documentation: true,
                 keywords: true,
+                format: HoverDocFormat::PlainText,
             },
             FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
         )
@@ -89,8 +93,9 @@ fn check_actions(ra_fixture: &str, expect: Expect) {
         .hover(
             &HoverConfig {
                 links_in_hover: true,
-                documentation: Some(HoverDocFormat::Markdown),
+                documentation: true,
                 keywords: true,
+                format: HoverDocFormat::Markdown,
             },
             FileRange { file_id, range: position.range_or_empty() },
         )
@@ -105,8 +110,9 @@ fn check_hover_range(ra_fixture: &str, expect: Expect) {
         .hover(
             &HoverConfig {
                 links_in_hover: false,
-                documentation: Some(HoverDocFormat::Markdown),
+                documentation: true,
                 keywords: true,
+                format: HoverDocFormat::Markdown,
             },
             range,
         )
@@ -121,8 +127,9 @@ fn check_hover_range_no_results(ra_fixture: &str) {
         .hover(
             &HoverConfig {
                 links_in_hover: false,
-                documentation: Some(HoverDocFormat::Markdown),
+                documentation: true,
                 keywords: true,
+                format: HoverDocFormat::Markdown,
             },
             range,
         )

crates/ide/src/markdown_remove.rs

@@ -14,9 +14,154 @@ pub(crate) fn remove_markdown(markdown: &str) -> String {
             Event::SoftBreak | Event::HardBreak | Event::Rule | Event::End(Tag::CodeBlock(_)) => {
                 out.push('\n')
             }
-            _ => {}
+            Event::End(Tag::Paragraph) => {
+                out.push('\n');
+                out.push('\n');
+            }
+            Event::Start(_)
+            | Event::End(_)
+            | Event::Html(_)
+            | Event::FootnoteReference(_)
+            | Event::TaskListMarker(_) => (),
         }
     }
 
+    if let Some(p) = out.rfind(|c| c != '\n') {
+        out.drain(p + 1..);
+    }
+
     out
 }
+
+#[cfg(test)]
+mod tests {
+    use expect_test::expect;
+
+    use super::*;
+
+    #[test]
+    fn smoke_test() {
+        let res = remove_markdown(
+            r##"
+A function or function pointer.
+
+Functions are the primary way code is executed within Rust. Function blocks, usually just
+called functions, can be defined in a variety of different places and be assigned many
+different attributes and modifiers.
+
+Standalone functions that just sit within a module not attached to anything else are common,
+but most functions will end up being inside [`impl`] blocks, either on another type itself, or
+as a trait impl for that type.
+
+```rust
+fn standalone_function() {
+    // code
+}
+
+pub fn public_thing(argument: bool) -> String {
+    // code
+    # "".to_string()
+}
+
+struct Thing {
+    foo: i32,
+}
+
+impl Thing {
+    pub fn new() -> Self {
+        Self {
+            foo: 42,
+        }
+    }
+}
+```
+
+In addition to presenting fixed types in the form of `fn name(arg: type, ..) -> return_type`,
+functions can also declare a list of type parameters along with trait bounds that they fall
+into.
+
+```rust
+fn generic_function<T: Clone>(x: T) -> (T, T, T) {
+    (x.clone(), x.clone(), x.clone())
+}
+
+fn generic_where<T>(x: T) -> T
+    where T: std::ops::Add<Output = T> + Copy
+{
+    x + x + x
+}
+```
+
+Declaring trait bounds in the angle brackets is functionally identical to using a `where`
+clause. It's up to the programmer to decide which works better in each situation, but `where`
+tends to be better when things get longer than one line.
+
+Along with being made public via `pub`, `fn` can also have an [`extern`] added for use in
+FFI.
+
+For more information on the various types of functions and how they're used, consult the [Rust
+book] or the [Reference].
+
+[`impl`]: keyword.impl.html
+[`extern`]: keyword.extern.html
+[Rust book]: ../book/ch03-03-how-functions-work.html
+[Reference]: ../reference/items/functions.html
+"##,
+        );
+        expect![[r#"
+            A function or function pointer.
+
+            Functions are the primary way code is executed within Rust. Function blocks, usually just
+            called functions, can be defined in a variety of different places and be assigned many
+            different attributes and modifiers.
+
+            Standalone functions that just sit within a module not attached to anything else are common,
+            but most functions will end up being inside impl blocks, either on another type itself, or
+            as a trait impl for that type.
+
+            fn standalone_function() {
+                // code
+            }
+
+            pub fn public_thing(argument: bool) -> String {
+                // code
+                # "".to_string()
+            }
+
+            struct Thing {
+                foo: i32,
+            }
+
+            impl Thing {
+                pub fn new() -> Self {
+                    Self {
+                        foo: 42,
+                    }
+                }
+            }
+
+            In addition to presenting fixed types in the form of fn name(arg: type, ..) -> return_type,
+            functions can also declare a list of type parameters along with trait bounds that they fall
+            into.
+
+            fn generic_function<T: Clone>(x: T) -> (T, T, T) {
+                (x.clone(), x.clone(), x.clone())
+            }
+
+            fn generic_where<T>(x: T) -> T
+                where T: std::ops::Add<Output = T> + Copy
+            {
+                x + x + x
+            }
+
+            Declaring trait bounds in the angle brackets is functionally identical to using a where
+            clause. It's up to the programmer to decide which works better in each situation, but where
+            tends to be better when things get longer than one line.
+
+            Along with being made public via pub, fn can also have an extern added for use in
+            FFI.
+
+            For more information on the various types of functions and how they're used, consult the Rust
+            book or the Reference."#]].assert_eq(&res);
+    }
+}

crates/ide/src/static_index.rs

@@ -16,8 +16,7 @@
     inlay_hints::AdjustmentHintsMode,
     moniker::{def_to_moniker, MonikerResult},
     parent_module::crates_for,
-    Analysis, Fold, HoverConfig, HoverDocFormat, HoverResult, InlayHint, InlayHintsConfig,
+    Analysis, Fold, HoverConfig, HoverResult, InlayHint, InlayHintsConfig, TryToNav,
-    TryToNav,
 };
 
 /// A static representation of fully analyzed source code.
@@ -137,8 +136,9 @@ fn add_file(&mut self, file_id: FileId) {
         });
         let hover_config = HoverConfig {
             links_in_hover: true,
-            documentation: Some(HoverDocFormat::Markdown),
+            documentation: true,
             keywords: true,
+            format: crate::HoverDocFormat::Markdown,
         };
         let tokens = tokens.filter(|token| {
             matches!(

crates/rust-analyzer/src/config.rs

@@ -1393,7 +1393,8 @@ pub fn highlighting_config(&self) -> HighlightConfig {
     pub fn hover(&self) -> HoverConfig {
         HoverConfig {
             links_in_hover: self.data.hover_links_enable,
-            documentation: self.data.hover_documentation_enable.then(|| {
+            documentation: self.data.hover_documentation_enable,
+            format: {
                 let is_markdown = try_or_def!(self
                     .caps
                     .text_document
@@ -1409,7 +1410,7 @@ pub fn hover(&self) -> HoverConfig {
                 } else {
                     HoverDocFormat::PlainText
                 }
-            }),
+            },
             keywords: self.data.hover_documentation_keywords_enable,
         }
     }

crates/rust-analyzer/src/handlers.rs

@@ -936,8 +936,7 @@ pub(crate) fn handle_hover(
 
     let line_index = snap.file_line_index(file_range.file_id)?;
     let range = to_proto::range(&line_index, info.range);
-    let markup_kind =
+    let markup_kind = snap.config.hover().format;
-        snap.config.hover().documentation.map_or(ide::HoverDocFormat::Markdown, |kind| kind);
     let hover = lsp_ext::Hover {
         hover: lsp_types::Hover {
             contents: HoverContents::Markup(to_proto::markup_content(