-
-
Notifications
You must be signed in to change notification settings - Fork 219
Add whitespace stripping in the input #598
Conversation
|
Wait, I misinterpreted it, it's only for whole lines, right? Hmm. |
|
It's like trim() is called on every line, then empty lines are dropped, and for the eager case Of course, white spaces are significant in |
djc
left a comment
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.
Awesome that you're working on this, did you guys already see rust-lang/rust#92526? It's pretty amazing! This also came up there. I'll link to this PR in from there so that we can feedback from them.
I guess this also needs some good documentation in the book.
My main point of feedback is that I think we'd want a way to change this setting on a crate-wide basis, in the askama.toml configuration. There, we might make it part of an extension/type-based configuration (I think we already have some?) so that people can easily apply this to HTML but not plain text templates. I was going to suggest that we ditch any kind of per-template configuration but it just occurs to me that that might make it harder to test this well (end-to-end) the way you've done here.
|
Am I correct in thinking that this is all done at compile-time? If so, that makes this extra-exciting :) |
Yes, that would all be done at compile time. |
|
I moved Strip from askama_shared's root into askama_shared::input. That's were the other arguments life. |
|
Did you have any thoughts about the configuration approach? |
Yes, that should be a project-wide configuration. I added the optional strip parameter to the config, too. But I actually have not idea how to write a test for it. Can you give me some pointers? |
|
I'm not convinced anymore that this is the best implementation. Most likely it would be simpler and better to use Strip::apply() in Generator::write_buf_writable() instead of preformatting the input. |
|
I simplified the PR. Now only "none" and "eager" are allowed. |
In HTML sources indentation can aid readability, but it only bloats the output. This PR adds the argument "strip" to templates, which lets you automatically strip the template input: * "none": Don't strip any spaces in the input * "tail": Remove a single single newline at the end of the input. This is the default * "trim-lines": Remove all whitespaces at the front and back of all lines, and remove empty lines * "eager": Like "trim", but also replace runs of whitespaces with a single space. The automatic stripping does not try to understand the input at all. It does not know what `xml:space="preserve"`, `<pre>` or `<textarea>` means.
|
My "simplified" version does not work. The "complicated" version 766d049 did. Reverted and rebased. |
|
Superseded by #673. |
In HTML sources indentation can aid readability, but it only bloats the
output. This PR adds the argument "strip" to templates, which lets you
automatically strip the template input:
at the back of the input.
"tail": The current default, i.e. pop a single newline character atthe end.
"trim": remove all spaces at the front and back of a line, and removeempty lines.
space.
The automatic stripping does not try to understand the input at all. It
does not know what
xml:space="preserve",<pre>or<textarea>means.