feat: Implement nested JSON in api subcommand#389
Conversation
Signed-off-by: ayn <[email protected]>
Signed-off-by: ayn <[email protected]>
59f246b to
25ad53b
Compare
Signed-off-by: ayn <[email protected]>
32e293a to
ed7471b
Compare
Signed-off-by: ayn <[email protected]>
ed7471b to
024eb4d
Compare
Signed-off-by: ayn <[email protected]>
2ca9dc4 to
291c1ca
Compare
Signed-off-by: ayn <[email protected]>
| return json.Unmarshal([]byte("{}"), &v.Value) | ||
| } | ||
| var items []string | ||
| for line := range strings.SplitSeq(s, "\n") { |
There was a problem hiding this comment.
currently it expects that input is being seperated with a \n for each element.
Need some thinking.
There was a problem hiding this comment.
See above, we can allow multiple Unmarshals.
jedevc
left a comment
There was a problem hiding this comment.
Sorry for my slowness! I finally got round to it (chaos week sorry) - just some early feedback, I saw this is still in draft. But it's so neat!
Much love for this new Jason type 🎉
| var bodyData jason.Jason[map[string]any] | ||
| data := []byte(strings.Join(items, "\n")) | ||
| if err := jason.Unmarshal(data, &bodyData); err != nil { | ||
| return nil, err | ||
| } | ||
| return jason.Marshal(bodyData) |
There was a problem hiding this comment.
Instead of this, you could just loop over all items, and unmarshal them one-by-one.
So essentially:
tags = []Unmarshal("tags[]=production")tags = [production]Unmarshal("tags[]=eu-west")tags = [production, eu-west]
There was a problem hiding this comment.
the way I am thinking of it, is that, a valid JSON is [....] or {} with proper syntax that have proper key value.
On other hand a valid nested-JSON is list of key=value pairs or valid JSON
so jason.Unmarshal somehow should take all of these and parse them as a one unit, that was my vision behind that Unmarshaling.
What we can do is keep parser to expect a \n to seperate them with clear comment about expected input, but in api.go we pass items one by one instead of joining them which also results in a valid end result.
Another thing we can do is make the parser bit smarter about spaces, but that ofc will come with complexity cost depends if spaces are allowed or not. For example tags [] = production is this valid nested-JSON or not, based on that the parsing becomes complex.
| } | ||
|
|
||
| for _, arg := range c.Args { | ||
| if strings.HasPrefix(arg, "@") { |
There was a problem hiding this comment.
We have a check here and also in readBodyItems. It's probably worth consolidating them, ideally if we could just have all the parsing logic in one place, like readBodyItems.
There was a problem hiding this comment.
This is so neat, I quite like this.
Any reason for naming it Jason? It's cute.
My only real big comment here is that we should split the package down. jason.go contains top-level public interfaces, while the parsing goes in parse.go, debug goes in debug.go, etc.
There was a problem hiding this comment.
I first named it nested, but it's not only nested, it's nested-JSON + JSON together.
Couldn't find a better name, jason was closest to my head because of a meme stuck in my head from youtube I guess. Prob Primeagen can't remember.
JavaScript Advanced Object Notation 🥼
There was a problem hiding this comment.
Aha, I love that 🎉 Maybe just make a note of it in the file, then it's all good 😁
|
|
||
| // VisualizeItems is a convenience function that parses the given items | ||
| // and returns a tree visualization of the result. | ||
| func VisualizeItems(items []string) (string, error) { |
There was a problem hiding this comment.
Probably best to just have Visualize.
| case []any: | ||
| b.WriteString("[\n") | ||
| for i, elem := range val { | ||
| b.WriteString(indent) |
There was a problem hiding this comment.
You can use fmt.Fprintf(b, "%s [%d] =, indent, i)` to simplify some of this string building.
| // DebugTree returns a detailed multi-line breakdown showing how each | ||
| // input item is parsed and where it lands in the final tree. This is | ||
| // useful for understanding the parsing of complex nested expressions. | ||
| // | ||
| // The output shows every item grouped by its top-level key, with parsed | ||
| // path segments and the final assigned value. | ||
| func DebugTree(items []string) (string, error) { |
There was a problem hiding this comment.
Can make this debugTree to keep it internal?
| return json.Unmarshal([]byte("{}"), &v.Value) | ||
| } | ||
| var items []string | ||
| for line := range strings.SplitSeq(s, "\n") { |
There was a problem hiding this comment.
See above, we can allow multiple Unmarshals.
| // or more bracketed segments. After the path an '=' or ':=' separates the | ||
| // value. If the item is a valid JSON object or array it is returned as a | ||
| // raw value with an empty path so callers can merge it at the root. | ||
| func parseItem(item string) (parsedItem, error) { |
There was a problem hiding this comment.
Note to self: read and review this code in a little depth.
| assert.Equal(t, expected, output) | ||
| } | ||
|
|
||
| func ExampleDebugTree() { |
There was a problem hiding this comment.
Fun little idea. Because this writing a new (fairly fiddly) parser, what do you think about writing a little fuzzer to search for issues? https://go.dev/doc/security/fuzz/
Then just run it for an hour or so, and see if it can find any crashes 🎉 (we've had some really annoying and difficult crashes in the last week, so hardening things to avoid needing to debug broken stuff later is very nice).
Signed-off-by: ayn <[email protected]>
452b5e0 to
5cecff2
Compare
Signed-off-by: ayn <[email protected]>
Signed-off-by: ayn <[email protected]>
Signed-off-by: ayn <[email protected]>
5f593fa to
8e84b1f
Compare
closes Use nested JSON syntax in
unikraft api#369Signed-off-by: ayn [email protected]