Skip to content

Conversation

@amterp
Copy link

@amterp amterp commented Apr 23, 2025

See my comments on #9 . The fix in #8 doesn't work as intended and breaks formatting for the below case:

Broken (current master version):

{
  "foo":"bar", << notice missing space
  "quz": []    << notice *correct* space
}

Fixed with this PR:

{
  "foo": "bar",
  "quz": []
}

I also include a couple of basic test cases demonstrating the fix to #7 and #9.

The individual commits in this PR contain some commentary and should probably not be squashed.

amterp added 4 commits April 23, 2025 21:16
This test should pass, but is failing, demonstrating the issue described
in PR nwidger#9. A following commit will fix this test.
This commit succeeded in fixing nwidger#7, but it broke other formatting.
Let's undo it, and fix nwidger#7 another way which keeps other functionality
intact.

With this commit, *both* tests are failing, including the general one
added in the previous commit. Both tests will pass in the next commit.
Consolidates some of the space-printing logic for fields, should be more
consistent, and we can now correctly format JSON output, while
fixing nwidger#7.

With this commit, both tests pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant