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

chore(grit): implement node-like compilers + fixes #3253

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

arendjr
Copy link
Contributor

@arendjr arendjr commented Jun 21, 2024

Summary

I feel I will have a pretty major announcement coming up soon, since this PR brings the bindings in a state where an actual Grit query can execute and match against an arbitrary JS snippet. The pattern execution appears to return a correct true or false result indicating whether the query matched or not. Also, if I change the pattern in the quick test to use the rewrite operator (=>) I can see the Grit engine produce an Effect, which is a data structure describing the rewrite that should take place, so this is really exciting.

What does not work yet, is extracting the ranges where matches are found when I don't use a rewrite operator. I suppose I could also try modifying the query to include an empty rewrite and extract the range from the Effect, but maybe @morgante knows an easier trick to do what I want. In any case, I'll keep that for a follow-up PR.

Of course there are plenty of other things that still need polishing/implementation before this could be considered ready for real adoption, but now that we are reaching the point where the engine is finally doing actual useful stuff, it may also become easier for other contributors to help out since many of the improvements that still need to be made should be doable independently. I'll write up a bit more about the things to do when I send out the announcement.

Test Plan

Updated the quick test, but I'll start adding more systematic tests soon.

@arendjr arendjr requested a review from a team June 21, 2024 20:11
@github-actions github-actions bot added the A-Project Area: project label Jun 21, 2024
@morgante
Copy link

Exciting milestone! You shouldn't need a rewrite to get matches, instead you look at the file binding history like we do here. If there's only a single version of the file it's a match.

@arendjr
Copy link
Contributor Author

arendjr commented Jun 22, 2024

@morgante In this PR I tried to port that approach, but I still end up without results. I traced it to input_matches being always None. In your repository input_matches seems to be set inside exec_step() which is one of the things I haven't ported yet (right now both multifile and sequential are still TODO). So I guess I was hoping to postpone the introduction of steps until I get to multifile queries, but maybe I should pull that part forward.

@morgante
Copy link

@morgante In this PR I tried to port that approach, but I still end up without results. I traced it to input_matches being always None. In your repository input_matches seems to be set inside exec_step() which is one of the things I haven't ported yet (right now both multifile and sequential are still TODO). So I guess I was hoping to postpone the introduction of steps until I get to multifile queries, but maybe I should pull that part forward.

Steps are relatively fundamental (as you can see, we do a fair amount of tairdown/cleanup in them) so I'd probably prioritize them before multifile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-Project Area: project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants