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

[wip] allow extended syntaxed to reference updated variables #2

Draft
wants to merge 1 commit into
base: implement-extends
Choose a base branch
from

Conversation

keith-hall
Copy link

Hi,

Thanks very much for the PR implementing extends. Sorry to let it linger for so long.
I had some ideas of how we could get the match patterns from the base syntax definition (which has already been parsed) to use the variables from the derived syntax definition. And that is to store the original "raw regex" on the MatchPattern so we can reevaluate it with the "latest" variables. It is probably not the best way to do it, but I feel like at this stage, it is better to get something implemented, merge it and improve it later...
To this end, I added a test which should succeed when everything works as expected. I confess that I haven't actually had time to complete the implementation yet, but thought I'd push it here in case you or someone else wants to take over, or in case it pushes me to find more time for it myself...

so can do a re-replacement of the variable references against the new extended syntax's variables
@jalil-salame
Copy link
Owner

I've forgotten how this code works, but I'm happy to dive back into this. I'll get back to you within a week with a review as I'm a bit busy rn and need to re-read the code.

@jalil-salame
Copy link
Owner

This looks good, I still need to go back to the original PR and rebase it/update it. Lazily evaluating stuff seems like a better approach, definitely. The only problem with extends is that it needs access to the syntax file database, and I don't know if you have that information when highlighting.

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