-
Notifications
You must be signed in to change notification settings - Fork 254
feat: add default/elvis and coalescing operator #748
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: add default/elvis and coalescing operator #748
Conversation
Add support for the default (Elvis) operator (?:) which provides a default value when the left-hand side expression is falsy. Examples: - order ?: 'default value' - age ?: 42 - missing ?: other ?: 0 (chainable) Co-authored-by: Mark Melville <[email protected]>
|
Thanks for picking this up. When the feedback on my PR was that the |
|
Thanks for your quick feedback! I added a few more tests for the elvis operator |
Looks good to me! @andrew-coleman, I'm curious if features get backported to the v1 release. If so, is there anything that needs to be done here for that? |
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.
Many thanks for working on this. I've left a few comments, but basically I think we can do this purely in the tokeniser / parser since these new operators are syntactic sugar for the existing ternary operator.
I haven't been maintaining a separate v1 branch. |
|
@andrew-coleman thanks for your feedback! I incorporated the changes and added the two new operators to the conditionals section with examples. Please feel free to suggest other changes if I missed something! I also added a few more tests to the default operator following the logic how values are cast to Boolean (https://docs.jsonata.org/boolean-functions). However, this test should evaluate the LHS to false and therefore return 42: But it fails and returns the LHS. Is something wrong here? |
Looks like you've found a bug in
then that should fix it. Thanks :) |
|
Great, I was first doubting myself 😄 Let me know if anything else should be changed in the docs |
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.
Looks good. Thanks!
This PR continues the work of @markmelville in PR #638 for the elvis/default operator
A ?: Band adds the coalescing operatorA ?? Bas described by @andrew-colemanI followed the logic of
$exists()when evaluating the LHS which only checks forundefined, that means other falsy values likenullare evaluated as true. This could be changed to also includenullto match the behavior of JavaScript's??operator.