Repository navigation
Conversation
shimeoki
marked this pull request as draft
September 20, 2026 16:33
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
i went ahead and decided to write a spec for kdl-in-json on the weekend while i have the ideas to do so: #569. refer to this discussion why this specification is even needed.
this is like 3rd draft and i changed many things about the schema. the following comments are the reasoning behind the decisions and alternate versions i considered, which may be better after all.
the design is not final at all, so i am making this pr as a draft to read some opinions first (i didn't actually wait for any replies in the discussion). i am not english native and this was written fully by hand, so there are probably grammar or logical errors.
naming
i tried to avoid duplicate names for properties in the schema. in kdl-test i was actually pretty confused when writing data types for the schema, because
valueandtypeappears in multiple places several times.so for values i named the "internal" field for the actual value data instead. and the "type" as in "data type and not type annotation" was removed as a whole. before i removed the data type, however, i named the properties for type annotations of nodes and entries as
anno.and i changed
childrentonodes. shorter and aligns with the entries, which point at the "value" type with avaluefield.optionality
while i was making a program to test the validity of the schema and how it can be represented, i bumped into the "go" thing about the zero values. for example, booleans are
falseby default, and in many scenarios unallocated slices (essentially dynamic arrays) are the same as the empty ones.after some time i just thought that the schema can just be pretty forgiving about the skipped/omitted/optional properties. also, just the ability to skip the fields instead of requiring them to appear in the result is less clutter for viewing and less bytes to store.
this approach makes the schema hold more types, because, for example,
nodescan be not just an array, but also anull, but i think that's okay.literals
instead of an enumeration i decided to use a boolean. i don't like to store booleans in all kinds of databases or data structures, because booleans are logical values and usually should be computed for the runtime logic only, and not act as the data.
however, in this case i just disliked using an enum more. this just brings more verification to the schema and makes the representation longer.
my initial idea was to have a
stringfield (also a boolean) that tells that thedatacontains a string. but i think strings appear more often as the data type overall, so for making use of the "false by default" optionality feature i decided thatliteralas an inverted condition is better. after making the example and seeing the matrix, i am not really sure...also, the property for the enum was called
typein kdl-test, and i don't see any other possible names for this field. as mentioned previously, because of that, i renamed type annotations toanno. usage ofliteralorstringallows to just usetypefor annotations without repeated naming.numbers
kdl-test tells the implementers to convert all numbers to a unified format, but i think for this specification this is just not needed.
we need to do that for strings, because syntax for multilines just doesn't exist in v1, but numbers are literally the same. the caller can decide what to do with the numbers by themselves. also, the caller doesn't need to actually compute the numbers, only verify the syntax.
numbers are strings for the reason mentioned in the linked discussion at the start. tl;dr: json and kdl numbers don't map exactly well, and it's probably simpler to just store them as strings.
nulls
i considered instead of requiring to write
#nullas an entry with the literal to just accept jsonnullvalues as valid values. that makes values optional and allows usage like this:{ "name": "node", "args": [ null, null, null ], "props": { "key": null } }while this is a clever way to make representations with many nulls shorter, it looks like a broken document. instead of using
nulljust as an "optional marker", it represents an actual value in source kdl.in this case we either allow two ways to write nulls (like in the example above and like in the spec) or allow only one. it's going to be very confusing if only one way is going to be allowed, because then this is illegal:
{ "literal": true, "data": "null" }. and if we allow both, i don't think that's just good.