Add 'none' as a literal evaluating to null. #480
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 21, 2015
Closed
|
@carljm This PR looks ready to be merged |
|
@SamyPesse Thanks for the nudge, I think you're right. Merging now. |
* master: (30 commits) Consolidate changelog. Catch correctly error from loaders Add multiline multi block raw test Fix "broken raw blocks test" by strictly searching for raw or endraw blocks only Fix multiple raw blocks with less greedy regex Add test for multiple raw blocks add maintenance note fix links for docs in french add error handler to chokidar instance (fixes mozilla#469) fix jshint error don't break in environment where Error.prototype.name can't be set (fixes mozilla#454) use chokidar correctly (fixes mozilla#450) include bin and src when using bower (fixes mozilla#444) fix version in browserfiles prep for 2.0.0 Docs in french Docs : fix link fix docs: typo parameter caseSens Update web-loaders.js Rewrite Parser.parseRaw ...
carljm
added a commit
that referenced
this pull request
Sep 2, 2015
Add 'none' as a literal evaluating to null.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I ran across this in working on the fix for #478.
Jinja2 has three special constants:
true,false, andnone(mapping to Python'sTrue,False, andNone, respectively).Nunjucks has
trueandfalse(mapping to JStrueandfalse), but no explicit support fornone.This has gone unaddressed thus far, because in practice a Jinja template using
noneports just fine to nunjucks anyway, as the symbolnoneevaluates toundefined, which behaves similarly-enough to Python'sNone(mostly just in that it's falsy) to make no real difference.But there are some subtle problems with this.
First, it's pointlessly inefficient. There's no reason for use of
noneto require acontextOrFrameLookupat runtime every place it is used.Second, it allows for the name
noneto be re-bound to some other value, resulting in confusing code and needless Jinja incompatibility.Third, the logical mapping for
noneto JS is reallynull, notundefined. Most of the time this makes no difference, but it does make a difference with thedefaultfilter. In Jinja,{{ none|default('d', false) }}evaluates toNone(becauseNoneis not an undefined value), whereas in nunjucks it printsd(because it is undefined).This pull request turns
noneinto a lexed constant just liketrueandfalse, with the valuenull.This could be backwards incompatible, if someone is relying on the current behavior of
nonewith thedefaultfilter, or is re-binding the namenoneto some other value. I think this backwards-incompatibility is acceptable as a bugfix, and the time to do it is now (before 2.0 is released).