-
-
Notifications
You must be signed in to change notification settings - Fork 48
Json.print() api in stdlib json package #223
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
Changes from all commits
c67abf7
393484e
be2554c
4fafb6d
b530eef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| - Feature Name: json-encode-jsonvalue | ||
| - Start Date: 2026-03-12 | ||
| - RFC PR: (leave this empty) | ||
| - Pony Issue: (leave this empty) | ||
|
|
||
| # Summary | ||
|
|
||
| Adapt the stdlib [`json` package][json-package] in order to add a `JsonPrinter` primitive exposing functions to encode arbitrary [`JsonValue`][JsonValue]s into `String iso^`, in a similar manner as the existing `JsonParser` for parsing [`JsonValue`][JsonValue] from String. | ||
|
|
||
| # Motivation | ||
|
|
||
| The basic functionality of a json library should be to provide a representation for structured json values (which is [JsonValue][JsonValue]) and methods to decode a string or bytes as JSON into this [JsonValue][JsonValue] and vice versa, to encode all possible representations of [JsonValue][JsonValue] into valid JSON bytes (or string). The current [`json` package][json-package] is lacking the means of encoding all [JsonValue][JsonValue] instances into valid JSON. | ||
|
|
||
| `JsonObject` and `JsonArray` provide methods for encoding them as valid JSON with the `.string()` and `.pretty_string()` methods, but for all other possible JSON values (string, number, bool, null) there is no such method. The `.string()` method on `None` is producing `"None"` (without the quotes), which is invalid JSON. Another example: the String `"\""` is copying itself when `.string()` is called on it, without any escaping, which is also invalid JSON. The method `.pretty_string()` is missing from all those other [JsonValue][JsonValue] instances. | ||
|
|
||
| This package needs a consistent and convenient way to encode and decode json bytes from and to all possible [JsonValue][JsonValue] instances. | ||
|
|
||
| # Detailed design | ||
|
|
||
| This PR suggests adding a new primitive called `JsonPrinter`: | ||
|
|
||
| ```pony | ||
| primitive JsonPrinter | ||
| """ | ||
| Serialize any `JsonValue` to a JSON string. | ||
| """ | ||
| fun print(value: JsonValue): String iso^ => | ||
| """Compact JSON serialization of any `JsonValue`.""" | ||
| _JsonPrint.compact(value) | ||
|
|
||
| fun pretty(value: JsonValue, indent: String = " "): String iso^ => | ||
| """Pretty-printed JSON serialization of any `JsonValue`.""" | ||
| _JsonPrint.pretty(value, indent) | ||
| ``` | ||
|
|
||
| All the bits and pieces for encoding arbitrary values are there already, they just aren't exposed: `_JsonPrint` is private. | ||
|
|
||
| To avoid confusion and misuse, the methods `JsonObject.string()` and `JsonArray.string()` (and `.string_pretty()`) are being renamed to `JsonObject.print()`, `JsonArray.print()` and `JsonObject.pretty_print()`, `JsonArray.pretty_print()`. This has the side-effect of both `JsonObject` and `JsonArray` and thus `JsonValue` not implementing `Stringable` anymore. | ||
|
|
||
| # How We Teach This | ||
|
|
||
| The package documentation of the `json` package should show usage examples of the new `JsonPrinter` functions as prominently as `JsonParser` usage and then show all other components of the crate, like `JsonValue`, `JsonPath` or `JsonNav`. | ||
|
|
||
| The intend of this primitive is to expose an easy-to-use interface for turning JSON values into strings as a dual to `JsonParser`. This should also answer the question of how to serialize Pony objects and data-structures as JSON (build a `JsonValue`, then pass it to `JsonPrinter.print(value)`). | ||
|
|
||
|
|
||
| # How We Test This | ||
|
|
||
| The [`json` package][json-package] already has print-parse roundtrip tests with arbitrary json values. Those need to be adapted to use the public API. Furthermore several "example"-tests are needed, verifying that printing a concrete `JsonValue` yields an expected valid JSON string (i.e. bools, ints, nulls, floats, strings with values needing escape sequences etc.). Those are there to proof that we don't have a common bug in printing and parsing which makes roundtripping work, but produces invalid JSON. | ||
|
|
||
| # Drawbacks | ||
|
|
||
| Renaming `.string()` and `.string_pretty()` on `JsonObject` and `JsonArray` to `.print()` and `.pretty_print()` is a breaking change. It will cause libraries and applications using the `json` package to be changed in order to be compiled with the ponyc version containing this change. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the early age, I'm not worried about this being a breaking change. |
||
|
|
||
| # Alternatives | ||
|
|
||
| ## Function naming alternatives | ||
|
|
||
| - `parse` / `print` - this would follow the existing naming in the package the closest, hence considered as the best candidate. | ||
| - `encode` / `decode` - this pair would make sense if the decoding would not expose `JsonParseError`. It feels inconsistent to create a `ParseError` from a `decode` function call. | ||
| - `serialize` / `deserialize` - these terms are not used anywhere yet in pony, so didn't favor them. | ||
| - `to_string` / `from_string` - The most humble variant of them all, getting along without any fancy terminology. | ||
|
|
||
|
mfelsche marked this conversation as resolved.
|
||
| Note that this RFC only includes `print` in scope, so renaming the `parse` would be out of scope unless we decided to pursue one of the above alternatives for a broader change. | ||
|
|
||
| ## Removing pretty | ||
|
|
||
| It has also been considered to condense both `pretty` and `print` into only one exposed function `JsonPrinter.print()`, similar to `JsonPrinter.print_with_options()` below. But this makes it cumbersome to enable pretty-printing: All parameters need to be examined and understood to configure a printing mode which is equivalent to what `.pretty()` would do. | ||
|
|
||
| ```pony | ||
| primitive JsonPrinter | ||
| fun print_with_options(value: JsonValue, indent: String = "", add_newlines: Bool = false, add_spaces: Bool = false): String iso^ => | ||
| """JSON serialization with all possible formatting options exposed. Defaults to compact formatting.""" | ||
| // TBD | ||
| ``` | ||
|
|
||
| ## Exposing verbose `print_with_options` as third method of JsonPrinter | ||
|
|
||
| Another alternative is to expose `print_with_options` above as third function and define `.print()` and `.pretty()` as aliases with arguments provided that are equivalent to their current formatting. | ||
| This would allow for greater control for users, if both `.print()` and `.pretty()` formatting do not fit their needs. This was not suggested as the use-case for further control was considered way too exotic. | ||
|
|
||
| # Unresolved questions | ||
|
|
||
| None. | ||
|
|
||
| [json-package]: https://github.com/ponylang/ponyc/tree/main/packages/json | ||
| [JsonValue]: https://github.com/ponylang/ponyc/blob/main/packages/json/json.pony#L243 | ||
|
Comment on lines
+82
to
+87
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would the optional parameter be an indent? IE if you give an indentation it pretty prints?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are several options, one I explored in this comment: #223 (comment) I still like exposing |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the decode already exists. What we are talking about is taking arbitrary JsonValue items and encoding to Json rather than starting from a JsonArray or JsonObject, yes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes