From 48d858f54d5b5a1a2808c8621c5d570d12874b88 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Tue, 30 Jun 2020 19:52:25 +1200 Subject: Don't strip affixes from path links --- crates/ra_ide/src/hover.rs | 88 ++++++++++++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 35 deletions(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/hover.rs b/crates/ra_ide/src/hover.rs index 8c972bb41..f7a0af037 100644 --- a/crates/ra_ide/src/hover.rs +++ b/crates/ra_ide/src/hover.rs @@ -17,7 +17,7 @@ use ra_ide_db::{ RootDatabase, }; use ra_syntax::{ - ast, ast::Path, match_ast, AstNode, SyntaxKind::*, SyntaxNode, SyntaxToken, TokenAtOffset, + ast, ast::{Path, MacroCall}, match_ast, AstNode, SyntaxKind::*, SyntaxNode, SyntaxToken, TokenAtOffset, }; use ra_tt::{Ident, Leaf, Literal, TokenTree}; use url::Url; @@ -394,35 +394,36 @@ fn hover_text_from_name_kind(db: &RootDatabase, def: Definition) -> Option( events: impl Iterator>, - callback: impl Fn(&str, &str) -> String, + callback: impl Fn(&str, &str) -> (String, String), ) -> impl Iterator> { let mut in_link = false; - let mut link_text = CowStr::Borrowed(""); + let mut link_target: Option = None; + events.map(move |evt| match evt { - Event::Start(Tag::Link(..)) => { + Event::Start(Tag::Link(_link_type, ref target, _)) => { in_link = true; + link_target = Some(target.clone()); evt } - Event::End(Tag::Link(link_type, target, _)) => { + Event::End(Tag::Link(link_type, _target, _)) => { in_link = false; - let target = callback(&target, &link_text); - Event::End(Tag::Link(link_type, CowStr::Boxed(target.into()), CowStr::Borrowed(""))) + Event::End(Tag::Link(link_type, link_target.take().unwrap(), CowStr::Borrowed(""))) } Event::Text(s) if in_link => { - link_text = s.clone(); - // TODO: This can unintentionally strip words from path-based links. - // See std::box::Box -> std::box link as an example. - Event::Text(CowStr::Boxed(strip_prefixes_suffixes(&s).into())) + let (link_target_s, link_name) = callback(&link_target.take().unwrap(), &s); + link_target = Some(CowStr::Boxed(link_target_s.into())); + Event::Text(CowStr::Boxed(link_name.into())) } Event::Code(s) if in_link => { - link_text = s.clone(); - Event::Code(CowStr::Boxed(strip_prefixes_suffixes(&s).into())) + let (link_target_s, link_name) = callback(&link_target.take().unwrap(), &s); + link_target = Some(CowStr::Boxed(link_target_s.into())); + Event::Code(CowStr::Boxed(link_name.into())) } _ => evt, }) } -/// Rewrite documentation links in markdown to point to local documentation/docs.rs +/// Rewrite documentation links in markdown to point to an online host (e.g. docs.rs) fn rewrite_links(db: &RootDatabase, markdown: &str, definition: &Definition) -> Option { let doc = Parser::new_with_broken_link_callback( markdown, @@ -431,21 +432,22 @@ fn rewrite_links(db: &RootDatabase, markdown: &str, definition: &Definition) -> ); let doc = map_links(doc, |target, title: &str| { - match Url::parse(target) { - // If this is a valid absolute URL don't touch it - Ok(_) => target.to_string(), - // Otherwise there are two main possibilities - // path-based links: `../../module/struct.MyStruct.html` - // module-based links (AKA intra-doc links): `super::super::module::MyStruct` - Err(_) => { - let resolved = try_resolve_intra(db, definition, title, &target) - .or_else(|| try_resolve_path(db, definition, &target)); - - if let Some(resolved) = resolved { - resolved - } else { - target.to_string() - } + // This check is imperfect, there's some overlap between valid intra-doc links + // and valid URLs so we choose to be too eager to try to resolve what might be + // a URL. + if target.contains("://") { + (target.to_string(), title.to_string()) + } else { + // Two posibilities: + // * path-based links: `../../module/struct.MyStruct.html` + // * module-based links (AKA intra-doc links): `super::super::module::MyStruct` + let resolved = try_resolve_intra(db, definition, title, &target) + .or_else(|| try_resolve_path(db, definition, &target).map(|target| (target, title.to_string()))); + + if let Some((target, title)) = resolved { + (target, title) + } else { + (target.to_string(), title.to_string()) } } }); @@ -520,7 +522,7 @@ fn try_resolve_intra( definition: &Definition, link_text: &str, link_target: &str, -) -> Option { +) -> Option<(String, String)> { // Set link_target for implied shortlinks let link_target = if link_target.is_empty() { link_text.trim_matches('`') } else { link_target }; @@ -534,6 +536,7 @@ fn try_resolve_intra( // Parse link as a module path // This expects a full document, which a single path isn't, but we can just ignore the errors. let parsed = SyntaxNode::new_root(ra_syntax::parse_text(link_target).0); + // TODO: Proper parsing let path = parsed.descendants().filter_map(Path::cast).next()?; let modpath = ModPath::from_src(path, &Hygiene::new_unhygienic()).unwrap(); @@ -566,7 +569,7 @@ fn try_resolve_intra( let import_map = db.import_map(krate.into()); let path = import_map.path_of(ns)?; - Some( + Some(( get_doc_url(db, &krate)? .join(&format!("{}/", krate.display_name(db)?)) .ok()? @@ -575,7 +578,7 @@ fn try_resolve_intra( .join(&get_symbol_filename(db, &Definition::ModuleDef(def))?) .ok()? .into_string(), - ) + strip_prefixes_suffixes(link_text).to_string())) } /// Try to resolve path to local documentation via path-based links (i.e. `../gateway/struct.Shard.html`). @@ -1485,15 +1488,30 @@ fn func(foo: i32) { if true { <|>foo; }; } } #[test] - fn test_hover_intra_link() { + fn test_hover_path_link_no_strip() { check_hover_result( r" //- /lib.rs pub struct Foo; - /// [Foo](Foo) + /// [struct Foo](struct.Foo.html) pub struct B<|>ar ", - &["pub struct Bar\n```\n___\n\n[Foo](https://docs.rs/test/*/test/struct.Foo.html)"], + &["pub struct Bar\n```\n___\n\n[struct Foo](https://docs.rs/test/*/test/struct.Foo.html)"], + ); + } + + #[test] + fn test_hover_intra_link() { + check_hover_result( + r" + //- /lib.rs + pub mod foo { + pub struct Foo; + } + /// [Foo](foo::Foo) + pub struct B<|>ar + ", + &["pub struct Bar\n```\n___\n\n[Foo](https://docs.rs/test/*/test/foo/struct.Foo.html)"], ); } -- cgit v1.2.3