djc / askama

Type-safe, compiled Jinja-like templates for Rust
Apache License 2.0
3.44k stars 218 forks source link

Make JSON prettifying optional #1023

Closed Kijewski closed 3 months ago

Kijewski commented 5 months ago

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's json.dumps():

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.

Kijewski commented 4 months ago

Are there open questions left?

GuillaumeGomez commented 4 months ago

I'm not a big fan of this change. Wouldn't it better to have different filters for each case? Or have the current json filter unchanged and new one which allows to pass options?

djc commented 4 months ago

I'm not a big fan of this change. Wouldn't it better to have different filters for each case? Or have the current json filter unchanged and new one which allows to pass options?

Why aren't you a fan?

GuillaumeGomez commented 4 months ago

Mostly because most of the time, the current json filter does exactly what I want. Rust doesn't allow to have default values if you don't specify them, which is fine but not really compatible with jinja here. So in case you don't want the default filter values, you could call the other json filter (json_with_options for example) and give the options you want for this call.

djc commented 4 months ago

Mostly because most of the time, the current json filter does exactly what I want. Rust doesn't allow to have default values if you don't specify them, which is fine but not really compatible with jinja here.

So you want prettified JSON most of the time? While Rust doesn't allow default values, the Askama template language can.

GuillaumeGomez commented 4 months ago

If you mean allowing a variable number of arguments to the json filter, then yes, I very much like this approach.