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

Print string versions of NaN, Infinity and -Infinity - resolves #696 #735

Closed
wants to merge 5 commits into from

Conversation

mdibaiee
Copy link
Contributor

@mdibaiee mdibaiee commented Oct 8, 2015

#696

Edit: I forgot to add tests, will add now.

@Marsup
Copy link
Collaborator

Marsup commented Oct 8, 2015

That's not really what I had in mind, it's not exactly done like that in lab. I'd like to be able to substitute with the correct syntax afterwards so with simple strings it won't be enough.

@Marsup Marsup added the feature New functionality or improvement label Oct 8, 2015
@Marsup Marsup self-assigned this Oct 8, 2015
@mdibaiee
Copy link
Contributor Author

mdibaiee commented Oct 8, 2015

@Marsup: I'll see if I can find a reference in lab's source.

@Marsup
Copy link
Collaborator

Marsup commented Oct 8, 2015

It's in the console reporter.

@mdibaiee
Copy link
Contributor Author

mdibaiee commented Oct 8, 2015

@Marsup: Thanks for the reference, I think it's better now.

Do we have to handle Functions and Symbols, too?

@Marsup
Copy link
Collaborator

Marsup commented Oct 8, 2015

Ideally yes. You'll also need that post-replace I talked about.

@mdibaiee
Copy link
Contributor Author

@Marsup Sorry about the delay.

By post-replace do you mean converting the serialized values back?

Do we already have a function doing this (I couldn't find one) or do I have to add it to internals? If so, any idea on the name? I'd say deserialize

@Marsup
Copy link
Collaborator

Marsup commented Oct 15, 2015

The annotate function is doing that with regexps at the bottom.

…ls.annotate

Test annotate's behavior over functions
@mdibaiee
Copy link
Contributor Author

@Marsup Alright, I think this is what you want, if there are any further changes required, ask right away.

@Marsup
Copy link
Collaborator

Marsup commented Oct 16, 2015

I think the function will be a little more complex than that if you have brackets inside the code.

@mdibaiee
Copy link
Contributor Author

@Marsup It seems to work as .* is greedy and continues until the last bracket, I modified the test as a proof.

@Marsup
Copy link
Collaborator

Marsup commented Oct 16, 2015

Then you're just lucky the function is last.

@mdibaiee
Copy link
Contributor Author

@Marsup

This input:

{
  "x": {
    "z": "[Infinity]",
    "f": "[function (x) { return [ { y: 2 } ]; }]",
    "u": "[-Infinity]",
    "y_$key$_1_$end$_": "[NaN]"
  }

gives this:

function (x) { return [ { y: 2 } ]; }

I think that's because each property is put on a newline, which in turn counts the last bracket of function as the last bracket of the current line, if that always happens in stringify, we're fine, else we should do something more complex to find out the "actual" closing bracket. Does stringify always put properties in new lines? Yes, because we pass an space argument which indents the object.

Now another question rises here, are functions always inlined? I found out that while stringifying, because we manually use fn.toString(), the function's linebreaks turn into \n characters, so we always have a single-line string, which in turn works with the used Regular Expression.

Edit: But I'm not sure about arrays, let me see.
Edit: Each array element is put on a new line when using the space argument.

@Marsup
Copy link
Collaborator

Marsup commented Oct 20, 2015

Can you try with multiple of each in the tests to make sure all are replaced ?

@Marsup Marsup added this to the 8.0.0 milestone Dec 17, 2015
@Marsup
Copy link
Collaborator

Marsup commented Dec 17, 2015

Merged manually as 0301604 with a few modifications, thanks for the ground work.

@Marsup Marsup closed this Dec 17, 2015
@mdibaiee
Copy link
Contributor Author

@Marsup: Sorry, I had completely forgot about it, thanks.

@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants