feat: ujson #1038
feat: ujson #1038
Conversation
|
As we discussed earlier, we're working on a more natural way to handle this situation. However, since that might take a while, I'm really impressed with how you've tackled this complex contract. It's great to see you diving into this, and I believe your work will continue to be a good resource for learning—even when it's eventually replaced by an official update. So please, keep up the good work! Many thanks. |
n0izn0iz
force-pushed
the
ujson
branch
3 times, most recently
from
db6dc02
to
72b8699
Compare
n0izn0iz
force-pushed
the
ujson
branch
2 times, most recently
from
c7dd55f
to
42ed2d7
Compare
n0izn0iz
force-pushed
the
ujson
branch
5 times, most recently
from
972ebfd
to
dfb9f7b
Compare
Signed-off-by: Norman Meier <norman@berty.tech>
Signed-off-by: Norman Meier <norman@berty.tech>
There was a problem hiding this comment.
Thank you for your efforts, Norman.
One thing I wonder about is whether doing proper tokenization and then AST is the proper way to go around making a JSON parser. This is a topic where I've personally played around with1 before, so I have a bit of an idea of what libraries are out there.
I tend to like jsonparser for reflection-less JSON parsing. Though as the name suggests, the library doesn't do marshaling, so someting different would still be needed for Gno.
Still, the design is something that I think we can borrow: instead of working with ASTs (which package users would have to support and learn about when starting to use ujson) maybe we can let the end users just work directly with []byte? (Also further strengthening the point I was making of using Unmarshal/MarshalJSON for custom types, rather than a new interface)
Footnotes
| } | ||
|
|
||
| // does not work for slices, use ast exploration instead | ||
| func (ast *JSONASTNode) ParseAny(ptr *interface{}) { |
There was a problem hiding this comment.
*interface{} is not what you think it is, you probably want to use interface{} here
There was a problem hiding this comment.
can you elaborate? I was thinking this was enforcing that I get a pointer as argument so I can assign to what it's pointing at
| type FromJSONAble interface { | ||
| FromJSON(ast *JSONASTNode) | ||
| } |
There was a problem hiding this comment.
I don't like this personally. JSON is a simple format with well-established conventions in Go; the most known of these is that types which should do custom JSON En/decoding should implement MarshalJSON and UnmarshalJSON, with well-defined signatures. I'd prefer if we kept it that way, even while we don't have encoding/json, so that when we do have reflection-based json encoding/decoding, it is relatively painless to port the old code to the new one.
There was a problem hiding this comment.
Also, as a bonus, I also like MarshalText if you want to implement support for that :)
There was a problem hiding this comment.
I was trying to avoid using the same naming as encoding/json to prevent confusion
|
I chose to pass the AST because it makes writing the unmarshalers very easy, the goal here was ease of use since it will be superseded by a proper generic object encoding which will not be in gno code if I understand correctly |
Signed-off-by: Norman Meier <norman@berty.tech>
Signed-off-by: Norman Meier <norman@berty.tech>
❌ Deploy Preview for gno-docs failed.
|
|
now in #1154 |
This is a basic json (un)marshaller
It ports strings (un)quoting directly from golang's
encoding/jsonIt can work on interfaces to ease writing marshallers/unmarshallers but can't fully deduct types since no reflection, for example, for objects you need to implement
FromJSONAbleand/orJSONAbleinterfaceIt also adds
unicode/utf16to stdlibs, copied from golang 1.17.5, this is needed for JSON strings unquotingCaveats:
strconv.Atoifor unmarshal so may loose precisionavl.Trees are automatically supported in marshaller but require a custom parser in unmarshaller since the values' types can't de deducted[]interface{}A version of this package is already used in Teritori's dao and feed contracts to return complex data to ui and also to encode actions that can be executed in proposals
See the tests for example usages
Although this might be improved, I think it's already useful as-is and could be merged if it passes review
Contributors' checklist...
BREAKING CHANGE: xxxmessage was included in the description