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.

Sign up for GitHub

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

Jump to bottom

Fix an issue with generate_pretty and empty objects in the Ruby and Java implementations #449

Merged
merged 1 commit into from Oct 20, 2020
Merged

Fix an issue with generate_pretty and empty objects in the Ruby and Java implementations #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions java/src/json/ext/Generator.java
View file
Open in desktop
Expand Up @@ -334,13 +334,13 @@ void generate(final Session session, RubyHash object,

buffer.append((byte)'{');
buffer.append(objectNl);
object.visitAll(new RubyHash.Visitor() {
private boolean firstPair = true;

final boolean[] firstPair = new boolean[]{true};
object.visitAll(new RubyHash.Visitor() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine for the patch, but it's now constructing two objects instead of one. Making this an inner class with an accessible firstPair field would get it back down to one object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, read it out of the object field, yeah.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turned out to be quite a lot of refactoring to make the field accessible, as the local holding the visitor needs a new named class or interface in order to be able to read a field out of it, so I didn't do this change. I don't know if you want to @headius? Otherwise I'll leave it as is.

@Override
public void visit(IRubyObject key, IRubyObject value) {
if (firstPair) {
firstPair = false;
if (firstPair[0]) {
firstPair[0] = false;
} else {
buffer.append((byte)',');
buffer.append(objectNl);
Expand All @@ -360,7 +360,7 @@ public void visit(IRubyObject key, IRubyObject value) {
}
});
state.decreaseDepth();
if (objectNl.length() != 0) {
if (!firstPair[0] && objectNl.length() != 0) {
buffer.append(objectNl);
buffer.append(Utils.repeat(state.getIndent(), state.getDepth()));
}
Expand Down
6 changes: 4 additions & 2 deletions lib/json/pure/generator.rb
View file
Open in desktop
Expand Up @@ -332,8 +332,10 @@ def json_transform(state)
first = false
}
depth = state.depth -= 1
result << state.object_nl
result << state.indent * depth if indent
unless first
result << state.object_nl
result << state.indent * depth if indent
end
result << '}'
result
end
Expand Down
5 changes: 5 additions & 0 deletions tests/json_generator_test.rb
View file
Open in desktop
Expand Up @@ -92,6 +92,11 @@ def test_generate
end

def test_generate_pretty
json = pretty_generate({})
assert_equal(<<'EOT'.chomp, json)
{
}
EOT
json = pretty_generate(@hash)
# hashes aren't (insertion) ordered on every ruby implementation
# assert_equal(@json3, json)
Expand Down