Skip to content
This repository has been archived by the owner on Sep 12, 2018. It is now read-only.

Parse and display EDN values for NaN, +Infinity and -Infinity. Fixes … #238

Merged
merged 1 commit into from
Feb 3, 2017

Conversation

jsantell
Copy link
Contributor

@jsantell jsantell commented Feb 2, 2017

#232

@victorporof can you elaborate on what EDN escaping you're talking about in the TODO? And is there any other Float formatting issues you had in mind when writing that TODO?

While we're here, is null a special EDN type? From what I gather, this should be printed as nil (that's what our parser looks for, anyway)

@@ -54,7 +55,19 @@ impl Display for Value {
Boolean(v) => write!(f, "{}", v),
Integer(v) => write!(f, "{}", v),
BigInteger(ref v) => write!(f, "{}N", v),
Float(OrderedFloat(v)) => write!(f, "{}", v),
Float(ref v) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Matching on constants have been deprecated, is there a more idiomatic way than if/elses?

@victorporof
Copy link
Contributor

victorporof commented Feb 3, 2017

@jsantell Should make sure that escaped characters in strings are printed/parsed in an isomorphic manner. See https://github.com/edn-format/edn#strings Test added here: #241

Nil is indeed an EDN type: https://github.com/edn-format/edn#nil Should be printed as nil, not null though, https://github.com/mozilla/mentat/blob/rust/edn/src/edn.rustpeg#L31

@victorporof
Copy link
Contributor

Also filed #240

@victorporof
Copy link
Contributor

Sorry you'll have to rebase this on top of c038c11

Please make sure the formatting respects the other rules in the peg file.

@jsantell jsantell merged commit 0b20d76 into mozilla:rust Feb 3, 2017
@jsantell jsantell deleted the 232 branch February 3, 2017 18:14
jsantell added a commit to jsantell/mentat that referenced this pull request Feb 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants