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

Allow setting [privacy] scope from queries #317

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

Slotos
Copy link
Contributor

@Slotos Slotos commented Oct 17, 2023

This has direct impact on highlighting. If scope is set and set to anything but "public", highlighter will use a separate highlight for the corresponding entry. This was previously used by Elixir extension by manually processing nodes.

Elixir extension processing is removed, its queries updated, and fixed to consider not just defp, but also defguardp and defmacrop as private.

Ruby queries are updated to deal with both predicate and statement versions of scope setting. The query is kinda brittle still, as anything unexpected in between methods definitions will break it. "Not" match would solve the problem, but treesitter doesn't provide one. A custom predicate might be a way to go, but better is the enemy of good, so I'm stopping here.

Copy link
Owner

@stevearc stevearc left a comment

Choose a reason for hiding this comment

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

This looks good, apart from the note about the elixir changes.

You mentioned wanting to have a negated match; Neovim does have some built in predicates for that (see :help lua-treesitter-not-predicate). You might be able to use #not-any-of?

Comment on lines 28 to 38
(call
target: (identifier) @identifier (#any-of? @identifier "defp" "defmacrop" "defguardp")
(arguments [
(call target: (identifier) @name)
(binary_operator left: (call target: (identifier) @name))
((identifier) @name)
])
(#set! "kind" "Function")
(#set! "scope" "private")
) @type

Copy link
Owner

Choose a reason for hiding this comment

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

I think adding the scope with #set! is a good additional capability, but for the elixir case I feel like we should use the previous extension-based logic. It's so much cleaner to have a single if-statement to set the scope than to duplicate this entire query

This has direct impact on highlighting. If scope is set and set to
anything but "public", highlighter will use a separate highlight for
the corresponding entry. This was previously used by Elixir extension
by manually processing nodes.

Ruby queries are updated to deal with both predicate and statement
versions of scope setting.
@Slotos
Copy link
Contributor Author

Slotos commented Oct 19, 2023

I ended up adding quite a few interleaving scope switches to ruby test case, since I'm relying on behaviours that I haven't found any documentation for.
Namely, the scoping query relies on specific nodes (identifier) winning against a catchall (_) one in an alternation query.

I have a few ideas about streamlining the whole proces by processing @scope node capture, but for now they are just that - ideas.

@stevearc
Copy link
Owner

Cool, this LGTM!

@stevearc stevearc merged commit 3a3baf0 into stevearc:master Oct 20, 2023
7 checks passed
@Slotos Slotos deleted the scope-from-queries branch October 20, 2023 07:56
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.

None yet

2 participants