correct hover text for items with doc attribute with raw strings #6331
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JoshMcguigan
force-pushed
the
hover-doc-attr-raw-string
branch
from
35b1385
to
d05d1e4
Compare
matklad
reviewed
Oct 23, 2020
| @@ -53,8 +53,21 @@ impl ast::Attr { | |||
| pub fn as_simple_key_value(&self) -> Option<(SmolStr, SmolStr)> { | |||
| let lit = self.literal()?; | |||
| let key = self.simple_name()?; | |||
| // FIXME: escape? raw string? | |||
| let value = lit.syntax().first_token()?.text().trim_matches('"').into(); | |||
There was a problem hiding this comment.
I think we already have "correct" way to do this implemented: https://github.com/rust-analyzer/rust-analyzer/blob/ea25ae614b21237c4a536304da875bdc29f0c65a/crates/syntax/src/ast/token_ext.rs#L149
We can just ast::String::cast our way to victory here, but, ideally, we should improve the API surrounding string literals somewhat: #6308
There was a problem hiding this comment.
Thanks for pointing this out! I've updated this to cast to ast::String/ast::RawString then use the value method.
I did take a look at #6308 but I think that would take a bit more time than I have right now. But I understand if you'd rather handle this after that, so feel free to close this PR if so.
Thanks for the review!
JoshMcguigan
force-pushed
the
hover-doc-attr-raw-string
branch
from
d05d1e4
to
4e76e88
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #6300 by improving the handling of raw string literals in attribute style doc comments.
This still has a bug where it could consume too many
"at the start or end of the comment text, just as the original code had. Not sure if we want to fix that as part of this PR or not? If so, I think I'd prefer to add a unit test for either theas_simple_key_valuefunction (I'm not exactly sure where this would belong / how to set this up) or create afn(&SmolStr) -> &SmolStrto unit test by factoring out thetrimoperations fromas_simple_key_value. Thoughts on this?