Skip to content
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

Feat/dump filter spaces #868

Merged
merged 3 commits into from
Oct 12, 2016
Merged

Conversation

JesseE
Copy link
Contributor

@JesseE JesseE commented Sep 19, 2016

Summary

Proposed change:

Nunjucks has a convenient dump filter which stringifies JSON. But the output is a hard to read string.

This PR proposes a change to the dump filter to accept an optional space argument and pass that through to JSON.stringify(input, null, space). You can use |dump(2) or |dump('\t') which gives much more meaningful output.

closes #867

Checklist

I've completed the checklist below to ensure I didn't forget anything. This makes reviewing this PR as easy as possible for the maintainers. And it gets this change released as soon as possible.

@@ -972,7 +973,69 @@ a b c d e f
### dump (object)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

signature changed from dump (object) to dump (object, [space]), but I think in any case simply dump would be better as heading.

**Input**

```jinja
{% set items = [{ a : 1}, { b : false}] %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to illustrate dump works on different primitives, so I'd use ['a', 1, { b : false}] in all examples in the docs. The only thing you'd change between examples is the space parameter (none, 2 spaces, 1 tab).

@@ -86,6 +86,18 @@
finish(done);
});

it('dump', function(done) {
equal('{{ {"container":[{\'a\': true}, {\'b\': 2 }]} | dump }}',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again I think you want to illustrate dump works on different primitives, so I'd use ['a', 1, { b : false}] (same as in doc examples) in all tests. The only thing you'd change between test cases is the space parameter (none, 2 spaces, 1 tab).

@JesseE JesseE mentioned this pull request Sep 20, 2016
4 tasks
add dump filter documentation

added changelog update

Fix mobile viewport (meta) (mozilla#865)

closes mozilla#860

Order builtin filters alphabetically (mozilla#863)

Ordered filters alphabetically in English and French documentation.
Chinese docs only have references to Jinja docs, so are not updated.

Closes mozilla#858

added more elaborate tests and adding variants of values that can be passed in json

remove single quotes and added spaces here and there

change foo dump to items dump consistency

change key types

correcting documentation dump filter

perfecting dump documentation

perfecting dump testing

added different indentation

complete tests
@jbmoelker jbmoelker self-assigned this Sep 20, 2016
@jbmoelker
Copy link
Collaborator

@JesseE looks good to me! I'd like to get this change in. The added filesize is neglectable (so doesn't hurt browser use). @vecmezoni do you agree?

@vecmezoni vecmezoni merged commit a654117 into mozilla:master Oct 12, 2016
@vecmezoni
Copy link
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dump filter needs spaces parameter for better readability
3 participants