Added more type options for status in the html_with_status macro #1568
Conversation
…us` as the status argument.
src/lucky/renderable.cr
Outdated
| html {{ page_class }}, _with_status_code: {{ status }}, {{ **assigns }} | ||
| {% elsif status.is_a?(SymbolLiteral) %} | ||
| html {{ page_class }}, _with_status_code: HTTP::Status::{{ status.upcase.id }}.value, {{ **assigns }} | ||
| {% elsif status.resolve.is_a?(NumberLiteral) %} |
There was a problem hiding this comment.
This is for the HTTP::Status enum case. When in "macro land" status is a Crystal::Macros::Path when an HTTP::Status enum is passed. At first i tried status.is_a?(HTTP::Status) however that would return false.
There was a problem hiding this comment.
Ah, interesting. So the Crystal::Macros::Path#resolve will turn that enum in to an Integer?
There was a problem hiding this comment.
That's right, so it will actually work with all Enums with the current implementation, maybe we should add a check to see if the Path starts with HTTP::Status
There was a problem hiding this comment.
That might not be a bad idea. You wouldn't want to accidentally send out a status of 4 😂 Then again.. maybe you do? 🤔 The whole reason for this method was more of an escape hatch since for the most part, you'd want to just use the html macro...
There was a problem hiding this comment.
I added the check, if you really wanted to sent out a status of 4 you could just pass a Number. Here be dragons 🐉.
Maybe it won't be an uncommon escsape hatch for long, the Turbo library from Hotwired expects a 422 unprocessable entity when a form fails with validation errors.
src/lucky/renderable.cr
Outdated
| html {{ page_class }}, _with_status_code: {{ status }}, {{ **assigns }} | ||
| {% elsif status.is_a?(SymbolLiteral) %} | ||
| html {{ page_class }}, _with_status_code: HTTP::Status::{{ status.upcase.id }}.value, {{ **assigns }} | ||
| {% elsif status.resolve.is_a?(NumberLiteral) %} |
There was a problem hiding this comment.
Ah, interesting. So the Crystal::Macros::Path#resolve will turn that enum in to an Integer?
| {% if status.is_a?(NumberLiteral) %} | ||
| html {{ page_class }}, _with_status_code: {{ status }}, {{ **assigns }} | ||
| {% elsif status.is_a?(SymbolLiteral) %} | ||
| html {{ page_class }}, _with_status_code: HTTP::Status::{{ status.upcase.id }}.value, {{ **assigns }} |
There was a problem hiding this comment.
Nice! I didn't know you could call upcase right on the symol at the macro level like that.
There was a problem hiding this comment.
It was also a first for me
The
html_with_statusmacro now also accepts Symbols andHTTP::Statusas the status argument.Purpose
Fixes #1558
Description
I also added the option to use symbols, no worries they are typesafe, so you will get a compile time error in case of typos.
I think it reads better with a symbol, maybe i'm weird like that 🤷
Checklist
crystal tool format spec src./script/setup./script/test