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

feat!: use queries for determining context #198

Merged
merged 1 commit into from
Mar 8, 2023
Merged

Conversation

lewis6991
Copy link
Member

@lewis6991 lewis6991 commented Jan 5, 2023

This plugin has been significantly rewritten to use Treesitter queries instead of patterns for determining context regions for languages.

The main benefits of this change are:

  • it is a much simpler implementation since we can leverage core APIs.
  • it fits in more generally with the Treesitter eco-system.
  • it allows configuration of contexts to be provided from multiples sources.
  • it allows more sophisticated configuration of contexts since queries (with directives and predicates) are much more powerful than patterns.
  • the query format should be usable for other editors.

The major downside of this new implementation is that it requires each language to provide it's own query as opposed to using the general purpose patterns. This means that some languages which had contexts before may not have them now. If this is the case then please raise an issue. Adding queries for a specific language is fairly simple but too much work to implement for all 170+ parsers that exist.

This commits provides explicit support for:

  • bash
  • c
  • cpp
  • typescript
  • rust
  • json
  • lua
  • markdown
  • python
  • yaml
  • php
  • scala
  • teal
  • toml
  • vim

Please see the README for instructions on how to add support for other languages.

This commit also drops explicit support for Nvim 0.7. If you still need support for this version then you can use the compat/0.7 release.

@lewis6991 lewis6991 force-pushed the feat/queries branch 9 times, most recently from 2c4a9ed to f1ae7bd Compare January 5, 2023 11:20
@syphar
Copy link
Contributor

syphar commented Jan 5, 2023

this is awesome :)

I'm happy to help testing / reviewing, if needed.

Could this also somehow handle the last_nodes logic through different queries?

( I only have basic knowledge of TS queries, so I'm not sure).

@lewis6991 lewis6991 force-pushed the feat/queries branch 3 times, most recently from 75d5116 to fca8b2f Compare January 5, 2023 11:49
@lewis6991
Copy link
Member Author

I hope so. I don't fully understand the last_nodes stuff, so need to get my head around it.

@lewis6991
Copy link
Member Author

I'm happy to help testing / reviewing, if needed.

Yes please. Any help is welcome.

This is a huge breaking change so I'll just be daily driving this branch for a while.

@lewis6991
Copy link
Member Author

Yeah, I don't really understand the last nodes stuff. Can you provide an example of what it does (or should do) and I'll just implement it with queries?

@lewis6991 lewis6991 force-pushed the feat/queries branch 2 times, most recently from 66ad6fe to d8540eb Compare January 5, 2023 12:29
@syphar
Copy link
Contributor

syphar commented Jan 5, 2023

Yeah, I don't really understand the last nodes stuff. Can you provide an example of what it does (or should do) and I'll just implement it with queries?

I can look up some more details & examples later .

From memory:
when you have a multi-line node for a function it will extend the context up to the ( for example) return-types at the end of the function header.

@syphar
Copy link
Contributor

syphar commented Jan 5, 2023

Also the skip_leading_types logic was probably needed for PHP.

I remember by default the context shows the whole line, not only what is covered by the TS node. And in some cases it would skip over things at the start of the line.

Since this was only used for PHP I'm not sure if we should invest time into fixing this

@lewis6991

This comment was marked as resolved.

@syphar

This comment was marked as resolved.

@syphar
Copy link
Contributor

syphar commented Jan 7, 2023

while playing around with queries I saw that the current implementation will actually merge the whole TS node content into one line, which for the python function example (function_definition) will lead to the whole function body being added too.

@syphar
Copy link
Contributor

syphar commented Jan 7, 2023

reading longer, it could be that #make-range! ( from treesitter-textobjects ) would help here?

( while the problem where would be that the def at the beginning of the function definition wouldn't be included in context, assuming we're trying to write a query that includes only the definition of the function, not the body

@lewis6991
Copy link
Member Author

I've got a rough idea to solve this by just adding a second capture (or reuse @context) to a match that will be used to control what gets displayed.

@lewis6991 lewis6991 force-pushed the feat/queries branch 3 times, most recently from 3b2fd25 to 086793c Compare February 1, 2023 14:15
@lewis6991
Copy link
Member Author

Just looked at the last_node stuff and can't seem to replicate the issue.

@lewis6991

This comment was marked as resolved.

@syphar
Copy link
Contributor

syphar commented Feb 23, 2023

@lewis6991 after the last bugfix I started to use this branch in my day-to-day work,

here are some new queries:

I'm not sure if every user would like the expression_statement pieces in there, for me it belongs into the context.

For example in a multi line function call this would give me the first line of the call:
grafik

@lewis6991
Copy link
Member Author

lewis6991 commented Feb 23, 2023

Updated both.

@lewis6991 lewis6991 marked this pull request as ready for review March 2, 2023 16:12
@lewis6991
Copy link
Member Author

@romgrk I'm thinking of merging this fairly soon. Can you have a look?

This plugin has been significantly rewritten to use Treesitter
queries instead of patterns for determining context regions for
languages.

The main benefits of this change are:
- it is a much simpler implementation since we can leverage core APIs.
- it fits in more generally with the Treesitter eco-system.
- it allows configuration of contexts to be provided from multiples sources.
- it allows more sophisticated configuration of contexts since queries
  (with directives and predicates) are much more powerful than patterns.
- the query format should be usable for other editors.

The major downside of this new implementation is that it requires each
language to provide it's own query as opposed to using the general
purpose patterns. This means that some languages which had contexts
before may not have them now. If this is the case then please raise an
issue. Adding queries for a specific language is fairly simple but too
much work to implement for all 170+ parsers that exist.

This commits provides explicit support for:
  - bash
  - c
  - cpp
  - typescript
  - rust
  - json
  - lua
  - markdown
  - python
  - yaml
  - php
  - scala
  - teal
  - toml
  - vim

Please see the README for instructions on how to add support for other
languages.

This commit also drops explicit support for Nvim 0.7. If you still need
support for this version then you can use the `compat/0.7` release.
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.

2 participants