-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Various breakages when formatting json heredocs #5930
Comments
Yeah, unfortunately For now, unfortunately you'll need to avoid using |
Thanks for confirming. I'll skim the documentation — there may be a tweak I can make, and then I can close this. |
Oh I missed this in your post:
Yeah the heredoc tag is purely textual and doesn't affect mime parsing because it's just for the Caddyfile. You'll need to add a |
Thanks — yes, I did know about that. As the docs on the respond directive says,
I thought this automatic detection was working correctly in the example with the non-escaped braces. Where I escaped the braces, I did need to add the header, contrary to the impression from the docs. I only escaped the braces because in my example I had placeholders, and I initially thought there was an issue parsing the placeholder from the JSON (I reported a minimal repro; there is an example with a placeholder in the repro repo). Then I discovered caddy fmt was breaking the heredoc. |
Ah I see, yeah it's not necessary to escape the JSON here. So your second handle-path should be what you use, and just avoid |
As explained in caddyserver/caddy#5930 (comment), `caddy fmt` doesn’t handle heredocs. The existing module always runs `caddy fmt` on native builds, and thus for a configuration containing a heredoc, the service fails at runtime. This change is simple, and backwards compatible. Alternatives I considered include only using the output of `caddy fmt` if `caddy adapt` also succeeds, removing the formatting and adding an option defaulting to false. Adding an option that defaults to true allows users to disable `caddy fmt` for other reasons, thoguh.
The lexer and formatter are separate, and the additional work is a significant undertaking. caddyserver/caddy#5930 (comment)
FYI @dbaynard, thanks to @bbaa-bbaa this should now be properly fixed 🎉 Also FYI @matthewpi since the vscode extension will be improved by this 😅 |
This is great — thanks @bbaa-bbaa caddyserver/website#352 will need a change — I can note the first caddy version for which this feature works? |
We'll just remove the note from caddyserver/website#352 when we release the next version. |
I've hit various issues, with 2.7.5 and earlier versions of caddy, when it comes to heredocs.
caddy fmt
introduces spacing which means the resulting file is invalid.See the caddyfile in https://github.com/dbaynard/caddy-fmt-heredoc-example — I recommend you step through the commits.
Empty commit
I created a sample, unformatted caddyfile, which works.
The first appears to be returned as
text/plain
.I adapted it, and it fails to load
Note the space at the end of the JSON for
/json
.Error: adapting config using caddyfile: parsing caddyfile tokens for 'handle_path': unrecognized directive: response - are you sure your Caddyfile structure (nesting and braces) is correct?, at Caddyfile:18
I removed the
handle_path
for/nope
with escaped bracesError: adapting config using caddyfile: parsing caddyfile tokens for 'handle_path': unrecognized directive: {"response":"nope"} - are you sure your Caddyfile structure (nesting and braces) is correct?, at Caddyfile:18
I tried with various different sigils, and saw no difference. In the repo, I have an example with a placeholder (that works correctly). And I have only tried the json formats.
Incidentally, the correct version of this would be helpful as an example.
The text was updated successfully, but these errors were encountered: