-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
caddyfile: Implement heredoc support #5385
caddyfile: Implement heredoc support #5385
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, despite my personal feelings about heredoc, I hereby accept that it is inevitable. 😜 This looks like a fairly decent change.
What happens if the heredoc isn't terminated?
877a634
to
b7649be
Compare
Oh yeah, good point. Forgot to handle the case where we run out of runes to lex but we didn't find the heredoc marker. Adding a condition for that. I still want to add some more tests to this before merging. I'll do that next chance I get. |
869c862
to
fc38852
Compare
I'm reopening my old feature branch from #3802 since I still think this should be implemented. Closes #5357
I've made some improvements to the implementation, some of which were possible because of improvements to the lexer we've made since 2020 when I first tried this.
This branch has a few commits at the end of the branch with some unrelated changes, mainly cleanup to some Caddyfile parsing/dispensing code that I noticed I could improve while reading around.
Copying the text from my previous PR:
Adds support for heredoc syntax to the Caddyfile, inspired by HCL (HashiCorp Config Language), and PHP 7.3's flexible syntax (trimming the front of every line based on the indentation of the ending marker: https://wiki.php.net/rfc/flexible_heredoc_nowdoc_syntaxes)
Here's an example:
Adapted JSON:
Notice that the response body has the leading whitespace on each line stripped away.
This could use some more tests in
lexer.go
before merging. There is an adapt test which is enough at a minimum for the moment.