-
Notifications
You must be signed in to change notification settings - Fork 17
Exclude comment sections from workspace symbols by default #866
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
Merged
+287
−21
Conversation
This file contains hidden or 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
This was referenced Jul 7, 2025
DavisVaughan
approved these changes
Jul 16, 2025
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.
Looks good to me, mostly just want some test coverage since this is a bit tricky
1c4fa66
to
a4d669b
Compare
120984c
to
ee55fe4
Compare
We've now got the requested test coverage and a test-only RAII indexer guard to help clean up the index after tests have run. |
a4d669b
to
eccee92
Compare
eb5b2aa
to
0eb735e
Compare
eebe91d
to
ce75104
Compare
`lsp_types::Url::from_file_path()` returns `None` if not a valid path. Path should start with e.g. `C:\` on Windows.
ce75104
to
40e9c0f
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Branched from #865.
Addresses posit-dev/positron#4886.
When a comment section like
# name ---
clashes with an actual symbol likename <- function() {}
, whichever symbols comes first (usually the section) is included in the workspace index, and the other is ignored. Ideally we'd track both but for now we only track one identifier per file.This causes sections to win over function definitions in cases like this:
If you then jump to definition on a reference to that function, Positron jumps to the section instead of the function.
To fix this, we now allow functions and variables to overwrite sections in the indexer.
In addition, we no longer emit sections as workspace symbols by default. This can be turned back on with a new setting
positron.r.workspaceSymbols.includeCommentSections
. This is consistent with the fix in Exclude section headers from workspace symbols in R packages quarto-dev/quarto#755 where we no longer export markdown headers as workspace symbols by default, to avoid flooding workspace symbol quickpicks (#
prefix in command palette) with section headers.QA Notes
You should be able to command+quick on the
my_section
reference and be taken to the function definition rather than the section. This should be the case no matter the value ofpositron.r.workspaceSymbols.includeCommentSections
.When
positron.r.workspaceSymbols.includeCommentSections
is set totrue
, you should see comment sections in the workspace symbol quickpick. Otherwise they shouldn't be included.