-
Notifications
You must be signed in to change notification settings - Fork 54
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
Quoted heredoc delimiter gives parse error #104
Comments
I'm planning a full overhaul of heredoc to support string interpolation there as well. |
@cfroystad when you start overhauling heredocs (and nowdocs?) is there any chance that you'll also be adding more child nodes to them? As I understand it, the For example, Atom currently allows the injection and highlighting of SQL/HTML/JS/JSON/XML in heredocs if the heredoc identifier matches one of those. This would be nice to have. Thank you! |
I've yet to look into it in detail (quite busy at the moment), but my current thinking is that since heredoc is basically a slightly more advanced double-quoted string in PHP, it should have the same node structure as the The current heredoc implementation should (after some fixes) be a viable nowdoc implementation (disclaimer: unverified from the top of my head) I'm not sufficiently familiar with the tree-sitter family to fully understand the ramifications of your last paragraph. Could you point me to some relevant documentation/example to help me understand this better. That way I'll try to account for it when I get started on this task |
We currently have this:
Which covers the entire thing. As I understand it, @claytonrcarter wants more like this:
Then they can check the content of the start and end delimiters, and highlight in the correct language according to delimeters. E.g.
Here the contents of the heredoc will be highlighted as SQL, which is indicated by the delimiters. To implement this, they need an easy way to extract the delimiters from the parse tree. |
Oh, a few hours later and that last paragraph of mine isn't clear at all, but yes @Sjord is correct on what I'm asking for. Thank you both! |
It took a while before I found the time, but I think #121 provides what you asked for. Any additional testing and review would be great! |
Thank you @cfroystad for this! I have not had a chance to actually play w/ it, but based on what I see in the test files, yes, this should work nicely for my needs. If I'm reading this correctly, I could look for any If I get a chance to play with it before merge, I'll be sure to let you know what I find. Thanks again for working on this! |
Expected:
Actual:
The text was updated successfully, but these errors were encountered: