Skip to content
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

Support completion and for intra-doc links #8314

Open
Tracked by #86
deontologician opened this issue Apr 3, 2021 · 9 comments
Open
Tracked by #86

Support completion and for intra-doc links #8314

deontologician opened this issue Apr 3, 2021 · 9 comments
Labels
A-completion autocompletion E-hard S-unactionable Issue requires feedback, design decisions or is blocked on other work

Comments

@deontologician
Copy link
Contributor

deontologician commented Apr 3, 2021

It would be nice to trigger the auto completion pane for identifiers after typing something like [` in a comment.

I tried fiddling with adding backtick to

https://github.com/rust-analyzer/rust-analyzer/blob/1ae20d2b894171f0b8368309da727cd365b95fc1/crates/rust-analyzer/src/caps.rs#L36

but (in vscode at least) it didn't seem to actually trigger the completion pane in a comment, though the backtick worked to force it to open elsewhere. From reading the lsp spec for that configuration, it looks like vscode is doing some detection of identifier characters already to try to decide when to bring up the pane, so maybe is also trying to be smart about not opening the pane inside a comment?

cc @Veykril (since you seem to be the intra-doc link person!)

@Veykril
Copy link
Member

Veykril commented Apr 3, 2021

The problem here is that our completion works on idents, meaning it works with syntax tokens that are names. Comments and doc-comments are just one big token of text instead of a recognized identifier so we can't really give auto-completion for this, at least not with the current structure we have I believe.

@deontologician
Copy link
Contributor Author

Can you point me to how an identifier is detected for this purpose? My rough idea is to possibly change the syntax tree so that comments can have children like this (even though it's not the actual rust grammar, you could imagine it's in the grammar of rustdoc)

@Veykril
Copy link
Member

Veykril commented Apr 4, 2021

Changing the syntax tree is out of the question I think, we don't want to parse through comments for intra doc links as that's not part of the language. Completion works by us inserting a pseudoident to fix up the potentially broken parse tree and then analyze the token at the cursor position, which in our case would just be the doc-comment token, we do remember the cursor position so we might be able to calculate the necessary info still. I'll take a look if it's workable.

@Veykril
Copy link
Member

Veykril commented Apr 4, 2021

Mmh ye this seems doable I think? At least in some cases. The main question is what it should complete and how, assuming the user types [, what should the completion do, given some struct Foo should it complete to [Foo](path::to::Foo), or something else? etc. This has to be thought about first I think since links can have a variety of flavours.

I'm unsure how well a client can handle this though as we aren't necessarily working with single identifiers anymore

@matklad
Copy link
Member

matklad commented Apr 5, 2021

Tough nut to crack! Dumped my current knowledge about the problem to #8343

@matklad matklad added E-hard S-unactionable Issue requires feedback, design decisions or is blocked on other work labels Apr 5, 2021
@deontologician
Copy link
Contributor Author

deontologician commented Apr 5, 2021

Changing the syntax tree is out of the question I think, we don't want to parse through comments for intra doc links as that's not part of the language.

My prior is that you're almost certainly right, given you're much more familiar with all of the involved components. But allow me to make the devil's advocate case that rust-analyzer might actually want a slightly different syntax tree than rustc itself:

The way I understand it is that rust-analyzer already has its own hand-rolled parser, and custom AST objects (generated by ungrammar) instead of simply copying over what's inside rustc. I assume this is partially for technical reasons like:

  • the internals of rustc not being available outside of a feature flag, so you'd need to vendor
  • rust-analyzer is built on salsa, and rustc isn't, so there are mismatches (I don't know what they might be, but I assume copy+paste would be too easy)

But I'm betting the parser and AST are home grown at least partially because rust-analyzer has different goals than rustc, and it needs the flexibility to make changes in service of that goal that might never be accepted upstream (since they aren't right for the compiler itself). I'd argue that one place where the IDE's needs are different from the compiler's needs is in doc comments. rust-analyzer already has several places where it parses doc comments for various features like highlighting code snippets and links. In contrast, rustc is never going to care about the contents of comments, so it makes sense not even to represent them in its AST.

Granted, this just isn't done for other languages and IDEs. I think this is probably an artifact of most documentation systems not being baked into the compiler the way rustdoc is. If the user can swap out the doc system and go from, say doxygen to sphinx, it is a complete waste of the parser's time to parse doc comments only to have to ignore that structure later so it doesn't conflict with the user's choices. We don't have that problem in rust fortunately, so we might be free to make a slightly unorthodox decision like this.

This may also solve the "there's nowhere to create a fake node" problem. If we add a minimal amount of structure to the syntax tree:

UnparsedText =
  'text'

AnnotatedPath =
  ('annotation' '@')? Path

MarkdownLink =
  '[' link_text:(AnnotatedPath | UnparsedText) ']'

DocCommentLine =
  ('///' | '//!') chunks:(UnparsedText | MarkdownLink)*

Inserting a fake node becomes copying the DocCommentLine and splicing a link object into the chunks vector (probably a little fiddly since you may need to split an UnparsedText into two parts where the cursor is inserted, but not too bad)

@deontologician
Copy link
Contributor Author

(I cut off this already way too long comment, but it would also allow adding a semantic layer to the doc comment structure, like the check rustdoc does for whether the internal link points to something valid)

@bjorn3
Copy link
Member

bjorn3 commented Apr 5, 2021

Documentation doesn't have to come from just /// doc comments. It can also come from #[doc = "..."] attributes. You can arbitrarily mix them. This is useful for macros as #[doc] allows the use of meta variables ($foo) while doc comments don't.

@flodiebold
Copy link
Member

I think we could add a full semantic layer for doc comments without actually changing the normal syntax trees.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-completion autocompletion E-hard S-unactionable Issue requires feedback, design decisions or is blocked on other work
Projects
None yet
Development

No branches or pull requests

5 participants