-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
std.json improvements #4543
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
std.json improvements #4543
Conversation
| Array: Array, | ||
| Object: ObjectMap, | ||
|
|
||
| pub fn jsonStringify( |
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.
The "json" part of this function name is redundant. The full path to this function is:
std.json.Value.jsonStringify
but it would be better to be
std.json.Value.stringify
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.
Also this function appears to be redundant with std.json.WriteStream().emitJson
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.
The "json" part of this function name is redundant. The full path to this function is:
std.json.Value.jsonStringifybut it would be better to be
std.json.Value.stringify
This function is a trait.
Also this function appears to be redundant with
std.json.WriteStream().emitJson
Yes: I have a followup PR to make that function use this one.
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.
This function is a trait.
To make things clear you are expecting that you can
pub fn foo(val: var) []u8 {
return val.jsonStringify()
}
and it will for any enum, struct, or whatever that has a jsonStingify method
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.
@JustinRyanH yes. and if you implement jsonStringify on your own struct/union/enum then std.json.stringify uses that instead of the "normal" routine for that data type.
lib/std/json/write_stream.zig
Outdated
| } else { | ||
| try self.stream.write("false"); | ||
| } | ||
| try std.json.stringify(value, std.json.StringifyOptions{}, self.stream, OutStream.Error, OutStream.write); |
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 think this is backwards, std.json.stringify should be utilizing std.json.WriteStream rather than the other way around.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
c64b77d to
c5de50c
Compare
c5de50c to
7a3d700
Compare
Follow-ups now that #4304 was merged.
jsonStringifytrait onjson.Valueobjects