-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
Make JSON prettifying optional #1023
Conversation
askama/src/filters/json.rs
Outdated
} | ||
} | ||
|
||
impl AsIndent for isize { |
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.
Does this still work if we impl for usize
directly?
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.
Not if it should support jinja's API where negative value = no beautification.
value: S, | ||
indent: I, | ||
) -> Result<impl fmt::Display, Infallible> { | ||
Ok(ToJson { value, indent }) |
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.
Is there reason not to store an Option<'static str>
in ToJson::indent
?
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.
Yes, the lifetimes wouldn't work.
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
to_writer_pretty(JsonWriter(f), &self.0).map_err(|_| fmt::Error) | ||
let f = JsonWriter(f); | ||
if let Some(indent) = self.indent.as_indent() { |
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.
Nit: would prefer to match
this, yielding out the self.indent
value and doing an early return for the None
case.
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.
No indentation is not the same as no beautification. A Some("") prefix still generates newlines, None does not. So both cases are entirely different. I could replace the if
with a match
, though.
This PR adds an optional argument to the `|tojson` filter, which controls if the serialized JSON data gets prettified or not. The arguments works the same as flask's [`|tojson`][flask] filter, which passes the argument to python's [`json.dumps()`][python]: * Omitting the argument, providing a negative integer, or `None`, then compact JSON data is generated. * Providing a non-negative integer, then this amount of ASCII spaces is used to indent the data. (Capped to 16 characters.) * Providing a string, then this string is used as prefix. I attempts are made to ensure that the prefix actually consists of whitespaces, because chances are, that if you provide e.g. `&nsbp;`, then you are doing it intentionally. This is a breaking change, because it changes the default behavior to not prettify the data. This is done intentionally, because this is how it works in flask. [flask]: https://jinja.palletsprojects.com/en/3.1.x/templates/#jinja-filters.tojson [python]: https://docs.python.org/3/library/json.html#json.dump
Are there open questions left? |
I'm not a big fan of this change. Wouldn't it better to have different filters for each case? Or have the current |
Why aren't you a fan? |
Mostly because most of the time, the current |
So you want prettified JSON most of the time? While Rust doesn't allow default values, the Askama template language can. |
If you mean allowing a variable number of arguments to the |
This PR adds an optional argument to the
|tojson
filter, which controls if the serialized JSON data gets prettified or not. The arguments works the same as flask's|tojson
filter, which passes the argument to python'sjson.dumps()
:None
, then compact JSON data is generated.&nsbp;
, then you are doing it intentionally.This is a breaking change, because it changes the default behavior to not prettify the data. This is done intentionally, because this is how it works in flask.
This PR is based on #1008, i.e. only the last commit is actually part of this PR.
Resolves #1019.