-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show docs on hover for keywords and primitives #7795
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit annoying that this requires the SyntaxNode and Semantics to be pulled through hover_for_definition just so we can get the std crate but I couldn't think of a better way.
👍 for recognizing a strong code smell early. I think a general solution is applicable:
If an argument to the function feels awkward, replace the argument with the result of the computation involving this argument. In this case, I think we can add an Option<FamousDefs>
argument instead of Sema
, SyntaxNode
pair.
Cargo.lock
Outdated
source = "registry+https://github.com/rust-lang/crates.io-index" | ||
checksum = "6093c460064572007f885facc70bb0ca5e40a83ea7ff8b16c1abbee56fd2e767" | ||
checksum = "7f6af9f8119104697b0105989a73c578ce33f922d9d6f3dae0e8ae3d538db321" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's better to avoid lockfile chnges
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh this wasn't there before, I think this got in when I rebased earlier, weird...
fn process_markup( | ||
db: &RootDatabase, | ||
def: Definition, | ||
markup: &Markup, | ||
links_in_hover: bool, | ||
markdown: bool, | ||
) -> Markup { | ||
let markup = markup.as_str(); | ||
let markup = if !markdown { | ||
remove_markdown(markup) | ||
} else if links_in_hover { | ||
rewrite_links(db, markup, &def) | ||
} else { | ||
remove_links(markup) | ||
}; | ||
Markup::from(markup) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also feel that our docs pipeline is in the need of refactor. In particular, markdown removal really should be a methond on markap, called from an ide:
// in handlers.rs
if snap.config.caps.support_markdown() {
markup.as_markdown()
} else {
markup.as_plain_text()
}
bors r+
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit annoying that this requires the
SyntaxNode
andSemantics
to be pulled throughhover_for_definition
just so we can get thestd
crate but I couldn't think of a better way.