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

JSON.pretty_generate generates different output to MRI for at least an empty hash #2053

Closed
chrisseaton opened this issue Jul 22, 2020 · 5 comments
Assignees

Comments

@chrisseaton
Copy link
Collaborator

require 'json'
puts JSON.pretty_generate({})
% chruby 2.6.5
% ruby -v test.rb
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-darwin19]
{
}
% chruby truffleruby-20.1.0
% ruby -v test.rb
truffleruby 20.1.0, like ruby 2.6.5, GraalVM CE Native [x86_64-darwin]
{

}

So does JRuby jruby/jruby#6338

@eregon
Copy link
Member

eregon commented Jul 22, 2020

It's an issue of json/pure, it also happens on MRI:

$ ruby -v -e 'require "json/pure"; puts JSON.pretty_generate({})'
ruby 2.6.6p146 (2020-03-31 revision 67876) [x86_64-linux]
{

}

Could you file an issue or make a PR to https://github.com/flori/json ?

@chrisseaton
Copy link
Collaborator Author

Fixed by flori/json#449.

@chrisseaton
Copy link
Collaborator Author

That's merged now. I'll close on anticipation you'll pick up the new version soon enough.

@eregon
Copy link
Member

eregon commented Oct 20, 2020

I'll watch (= enable notifications for) the json repo for releases.
We should probably use the json version that MRI uses for the stdlib though.
So if this fix is important, we could manually cherry-pick it on top of the stdlib version.
Do you think that's worth it? Are some tests failing without it?

We might also consider switching to the C extension for maximum compatibility, the main question there is what's the impact on performance.

@chrisseaton
Copy link
Collaborator Author

It caused a spec to fail in practice.

But it's not actually a problem - minor workaround to allow for it. I wouldn't cherry-pick it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants