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

Show documentation on hover and completion #101

Merged
merged 8 commits into from
Jan 25, 2025

Conversation

PgBiel
Copy link
Contributor

@PgBiel PgBiel commented Jan 25, 2025

Closes #72 (still WIP)

Right now, neither feature works at all in VSCode for some reason. I'm not sure if this is a problem with the extension or with server/client capabilities or something else, but currently, with this PR's changes, the language server is just returning null for any hover or completion with documentation in VSCode, meaning items with documentation are just not suggested / have no hover anymore. I'll try to investigate, but for now, all screenshots below use the Zed extension.

EDIT: It works in VSCode. Turns out I wasn't telling VSCode to use my build, so it was using an outdated version of the LSP. Whoops :)

In addition to trying to get the VSCode extension to work, I'd like to implement at least the following changes:

  • Creating a proper doc comment structure which separates main doc body from contracts (right now it's just a plain string). This will allow us to only send the doc body on completions instead of also including contracts. In addition, it allows us to properly structure contracts visually on hover.
    • To be honest, I'm not entirely sure whether we should omit contracts from completions as long doc strings can happen anyway, and would be shown separately. But I think this is a good heuristic for now and keeps the list of completions more compact.
  • Automatically remove common indentation from the doc comment body.
  • We could also consider adding function documentation to signature help; I'll leave this as a separate and optional task (although it's probably quite trivial, but would need some additional testing).
    • Done, although it's currently missing docs for individual parameters, but that'd need some additional work to parse @param docs.

Future work

  • Parse param docs (e.g. @param value `The requested value.` )

Screenshots

VSCode

hover shows doc
complex docs
module docs on hover

Unfortunately, VSCode doesn't support showing documentation directly on completions; however, they can be manually shown with Ctrl/Cmd + I:

doc on completion

Zed

hovering over function name shows doc
completion shows doc
type doc on hover
module doc on hover

@pherrymason
Copy link
Owner

I'm concerned that this work will need to be replicated and adapted in the branch search_on_astB, which takes a different approach on parsing and searching.
Do you feel brave enough doing the PR on that branch?

@pherrymason
Copy link
Owner

Thinking it better, is ok if you do it on main branch, it will serve me as a base anyway, PR very welcomed.
Anyway autocomplete system is not yet implemented on that other branch..

@PgBiel
Copy link
Contributor Author

PgBiel commented Jan 25, 2025

I'm concerned that this work will need to be replicated and adapted in the branch search_on_astB, which takes a different approach on parsing and searching.
Do you feel brave enough doing the PR on that branch?

I could give it a shot at some point, but to be honest my work here is (comparatively) pretty simple - I just traverse top-level nodes in order and attach each doc comment to the node that immediately follows the doc comment. The rest (hover, completion) just reads the attached doc comments from the Indexable structures. So it wouldn't be that much of a loss to completely redo the parsing part for the new branch as needed. That is, the PR could safely land on main, and then the new branch could be rebased by just ignoring my changes on the parsing part (which uses the tree-sitter CST in principle).

@PgBiel PgBiel marked this pull request as ready for review January 25, 2025 17:09
@PgBiel
Copy link
Contributor Author

PgBiel commented Jan 25, 2025

Should be ready to go 😎

Feel free to give it a spin.

@@ -222,6 +237,9 @@ func (s *Search) BuildCompletionList(
items = append(items, protocol.CompletionItem{
Label: member.GetName(),
Kind: &member.Kind,

// At this moment, struct members cannot receive documentation
Documentation: nil,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because of how you decide which doc comment you assign to a symbol? (Las doc block parsed belongs to next identified symbol)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In part, yes: I'd have to replicate my code to detect and assign doc comments in the struct parsing function as well in order to accept doc comments inside the struct, i.e. to accept something like

struct Name {
  <* doc *>
  int a;
}

But also, I didn't do that because it doesn't compile (through c3c), even though the tree-sitter grammar accepts it.

We could, of course, just copy and paste the struct's own docs to all of its members, but that doesn't feel ideal, at least not at the moment.

@pherrymason pherrymason merged commit 7984cb5 into pherrymason:main Jan 25, 2025
2 checks passed
@pherrymason
Copy link
Owner

Great work! Thank you

@PgBiel PgBiel deleted the show-docs branch January 26, 2025 03:31
@pherrymason pherrymason mentioned this pull request Jan 28, 2025
32 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display doc comments on hover
2 participants