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

Properly format documentation for SignatureHelpRequests #14974

Merged
merged 1 commit into from Jun 10, 2023

Conversation

max-heller
Copy link
Contributor

Properly formats function documentation instead of returning it raw when responding to SignatureHelpRequests.

I added a test in crates/rust-analyzer/tests/slow-tests/main.rs -- not sure if this is the best location given the relevant code is in crates/rust-analyzer or if it's possible to test in a less heavyweight manner.

Closes #14958

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 4, 2023
@lowr
Copy link
Contributor

lowr commented Jun 7, 2023

Fix looks good, thanks!

As for the test, a slow test for this sounds a bit overkill. I think we can write a lighter test similar to this. The procedure would be like:

  1. feed test fixture to Analysis::from_single_file()
  2. call analysis.signature_help() and get SignatureHelp
  3. feed it to signature_help() (the one you're fixing) and assert it successfully hides ignored code

Can you try making such test? Let me know if there's anything unclear! Also, since the changes are pretty small, you can squash everything into one commit and just force push it.

@Veykril Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 9, 2023
@max-heller
Copy link
Contributor Author

Can you try making such test? Let me know if there's anything unclear! Also, since the changes are pretty small, you can squash everything into one commit and just force push it.

Done, thanks!

Copy link
Contributor

@lowr lowr left a comment

Choose a reason for hiding this comment

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

All looks great, thank you!

@lowr
Copy link
Contributor

lowr commented Jun 10, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 10, 2023

📌 Commit 78fab7d has been approved by lowr

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 10, 2023

⌛ Testing commit 78fab7d with merge 68bdf60...

@bors
Copy link
Collaborator

bors commented Jun 10, 2023

☀️ Test successful - checks-actions
Approved by: lowr
Pushing 68bdf60 to master...

@bors bors merged commit 68bdf60 into rust-lang:master Jun 10, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hiding of code lines (#) doesn't work when calling a function
5 participants