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.
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
Add tests for E-expressions using simple system macros. #100
Add tests for E-expressions using simple system macros. #100
Changes from 1 commit
2c78736
0ac44db
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Interesting; does
$0
also behave this way? I think I expectednull
arguments to be an error.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.
Great question, I tend to forget about
$0
. Heck, we also have absent symbols to deal with.I cut amazon-ion/ion-docs#324 to deal with that later.
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.
I spec'd nulls to act like
""
because their presence doesn't seem problematic enough to treat exceptionally. This way all (known) symbol and string values are accepted without error.(The cases you call out are arguably encoding defects, so I think they deserve more global handling.)
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.
Adding a bit more detail on my thought process: the typed signature of this macro would be
(text::v*)
and I think its preferable for it to succeed given anytext
value. So that would includenull.string
andnull.symbol
for sure, and$0
is technically a valid non-null symbol. All of the above have no text, so I conclude that its reasonable to accept and ignore them (which happens to be indistinguishable from treating them as""
).