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

endless heredocs don't consume rest of document #36

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nobodywasishere
Copy link
Member

Closes #23

This works on paper but not 100% as I'm not as familiar with the heredoc parsing here.

Also, this change could go either way, as it's debatable which is preferable. I think this way is better as when adding a new heredoc to the middle of a document, you don't have to deal with everything else in the document being turned into a string until you add the heredoc end. But if you're changing the heredoc name, you may end up with the contents inside parsed as Crystal.

@keidax
Copy link
Collaborator

keidax commented Dec 14, 2024

The implementation looks good.

I don't have a strong opinion about whether this change is an improvement. But I'm leaning towards not making this change, just because it's a little surprising.

My experience with Ruby syntax highlighting in the past has always been that a missing heredoc terminator causes the rest of the document to be highlighted as heredoc contents. tree-sitter-ruby also works this way.

@nobodywasishere
Copy link
Member Author

That's valid. My thought process with this is just to limit the scope of errors as much as possible. I can leave this as a draft PR if we ever decide to come back to it.

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.

Limit parsing of endless heredocs
2 participants