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

Handle comment nodes within text objects #548

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

qstrahl
Copy link

@qstrahl qstrahl commented Jan 8, 2024

While I was editing my config for this very plugin, commenting and uncommenting lines in the config table, I tried to make use of the @parameter text objects and noticed some unexpected behaviour around comment nodes. The relevant query, it turns out, naively assumes that table constructors consist of single nodes separated by commas. In reality, there could be any number of nodes between those commas: one field, and any number of comment.

Unfortunately, due to some limitations of treesitter queries (in particular, the fact that the dot anchor ignores anonymous nodes) I was unable to find a simple modification to the existing queries that would account for all cases. Instead, two new queries had to be added, in addition to some slight modifications to the existing queries. I would really like to discover that I've missed an obvious, more succinct solution.

I suspect there are many other queries across all supported languages that have similar room for improvement, but I don't have the time or energy to fix them all, hence the draft PR. It's my intention to continue improving queries as I encounter more edge cases, and my hope that others can apply a similar approach across other queries in the project.

@qstrahl
Copy link
Author

qstrahl commented Jan 8, 2024

After a great deal of experimentation with different approaches, I ultimately landed on the conclusion that the best approach was to consider comment nodes to be part of the parameter. This avoids a lot of awkward edge cases. While I think it would be possible - and arguably helpful - to add special cases for commented-out fields to be treated as their own parameter objects, this would considerably complicate the already increasingly complex queries, and the implementation would be naive at best - liable to get in the way more than it helps, in practise.

@qstrahl qstrahl marked this pull request as ready for review January 16, 2024 17:21
@qstrahl
Copy link
Author

qstrahl commented Jan 16, 2024

In the interest of not letting perfect be the enemy of good, and in light of the recent movement on this issue in #549, I'm opening this for review.

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.

1 participant