-
Notifications
You must be signed in to change notification settings - Fork 29
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: add support for :scope
selector
#23
Comments
Of course since tsquery uses esquery to parse the query strings, it will be tricky to fix this until esquery supports it too... |
Yeah that's a great idea, and their implementation looks solid. I'll give the ESQuery team a little while to see if they respond to the comments, but then will definitely look at moving the parser into TSQuery (hopefully temporarily) |
Great! Happy to create PR(s) as necessary. Just give me the nod. |
I had a go at adding unit tests to the PR in ESQuery, but other tests were failing and I don't have time to dig into them right now 😕 @petebacondarwin do you reckon it's worth fixing it up their side, or should we try to do it directly in TSQuery? |
It would be very helpful to be able to use the |
I added some very basic unit tests to the PR in ESQuery, where should I do the pull requests ? If you have anothers tests in mind I'll be glad to add them. |
Thank you so much for your work on this! I'm not really sure how the PR should work, you could just do it against esquery and see what happens? Might be more direct that way. I guess they'll tell you if it doesn't work right. I'd probably suggest adding another test for the use case mentioned in the original issue here as well, just to make sure we're solving the original request? |
I'll add the test for the use case described above and I'll create a new pull request against esquery directly 😃 |
If the pull request is accepted and a new version is published, what would be the next steps (beside updating the ESquery version in package.json)? |
Just updating the version and then publishing it 😊 |
Unfortunately it does not seem so easy 😢
but I have this error when I run the tests:
The tests below are failing with the same error:
Do you have any ideas on why it's failing ? |
It's seems that a new matcher is needed in this file: Lines 1 to 34 in 07baf37
|
Ah yep, too right. Once their PR gets merged and released I can port over the matcher and the tests too. |
@run1t - I think you can test this locally more easily with |
@petebacondarwin - Thanks for the advice ! |
I was trying to implement the :root and :scope selector. I got I looked at the implementation in Do someone know the goal of this ? And why there is a scope parameter added here ?: https://github.com/run1t/esquery/blob/1f3cfdb0e376d5a0aa83a36c793d5f5649c7abda/esquery.js#L50 |
I find this explanation of So in a CSS stylesheet there is only one scope, which happens to be the I believe that the implementation of So to summarize:
|
TSQuery’s matcher implementation is almost identical to ESQuery, I’ve just split it up a bit more and renamed stuff. If you’d like to make a PR of where you’ve got too, I can try to give you advise on what to try next? |
@petebacondarwin Thanks for the explanation So if we take this example: function a() {
function b() {
return 'bar';
}
return 'foo';
}; If I understood correctly: Assuming we are in the
|
That that is how I understand it. Moreover: Assuming we are in the
|
@phenomnomnominal you can now take a look at the PR #37 😃 |
If |
Do we know that they don’t? Let’s give them a little nudge first, and see what needs to happen for it to be merged. |
I hope they will ! :), but it's been 15 days and nobody responded on the pull request. So I was wondering, At which point we need to consider it will not be merged ? I'm new to open source so I don't really know what to expect. I need to be patient I guess ! |
If I'm understanding this correctly, I think About the question, you select the outer ReturnStatement like this: The '*' would be a SyntaxList but seems is treated specially and you cannot mention it in the query (i.e this works: About |
Hello, esquery has recently been updated with a feature similar to |
See https://developer.mozilla.org/en-US/docs/Web/CSS/:scope
This selector would allow queries to limit the matching to only the direct children of the AST on which the query is run.
For example, in the following code:
How do you query for just the outer function's return statement? Currently the query
ReturnStatement
will return both return statements even if it is run on theBlock
node that is the body of the outer function.With the
:scope
selector you would be able to use:scope > ReturnStatement
and then run that on the body of the outer function to only return the outerReturnStatement
.ESQuery has a PR for adding this already estools/esquery#61
The text was updated successfully, but these errors were encountered: