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

Assert null lookup when rendering an unescaped value #553

Merged
merged 2 commits into from
Mar 27, 2016

Conversation

dasilvacontin
Copy link
Collaborator

It was not being asserted anywhere else, and I was suspecting that
PR #552 was changing current (and correct) behaviour.

@dasilvacontin
Copy link
Collaborator Author

Gonna try placing the new assertion in a more adequate test file.

Either vim or git was going crazy with the indentation when I tried
improving the test via changing one of the lines, so I linted it all
together.
It was not being asserted anywhere else, and I was suspecting that
PR #552 was changing current (and correct) behaviour.
@dasilvacontin
Copy link
Collaborator Author

Waiting for confirmation from @bobthecow, just in case: https://twitter.com/dasilvacontin/status/709796886568423424.

@bobthecow
Copy link

This is probably in the category of things — like the definition of "falsey" — that is up to the individual library to decide what best fits in the host language. In my opinion the JavaScripty thing to do when encountering a null is to treat it as "the user intentionally put nothing here", and thus to interpolate it as an empty string. This is what Mustache.php does as well:

screen shot 2016-03-15 at 10 37 17 pm

Note that null in a block context will also stop context stack lookups from propagating.

At the very least, if I'm wrong it'd be a backwards compatibility break, so it's not allowed until the next major release :)

@dasilvacontin
Copy link
Collaborator Author

In my opinion the JavaScripty thing to do when encountering a null is to treat it as "the user intentionally put nothing here", and thus to interpolate it as an empty string.

Note that null in a block context will also stop context stack lookups from propagating.

Thanks for your insight! We are doing both things – great! 😄. We treat null as a marker for "nothing" (which is what null really is), so:

  • the lookup function stops and returns null when it finds it (we fixed some edge-cases regarding this behaviour not long ago),
  • it's considered as falsy for blocks, and
  • it's currently being rendered as an empty string. 😄

@bobthecow
Copy link

👍 sgtm

@dasilvacontin dasilvacontin merged commit 9017163 into master Mar 27, 2016
@dasilvacontin dasilvacontin deleted the escaped-lookup-test branch March 27, 2016 19:01
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.

None yet

2 participants