Merge KDL v2

[zkat]

Here it is! The long-awaited KDL v2, which is where we go ahead and make a handful of technically-breaking changes to address some corner cases we've run into over the past year while KDL has been getting implemented in a bunch of languages by various people.

I'd love to get feedback on what we have slated, and whether there's anything else we should definitely include when this goes out.

[zkat]

/cc @CAD97

[CAD97]

I have a slight preference for #241 over #204 personally, though only slight.

[IceDragon200]

Shouldn't there also be an output case for this, and one for forwardslash (node "/") then?

Seeing as the escaped form was removed

EDIT: mixing up my slashes again

[hkolbeck]

I have a preference for #204, because the primary use case I can see for # in bare identifiers is hashtag-like which would be illegal under either, and it seems better to go with the simpler rule.

That preference is not terribly strong, though.

Edit: I misread, I'm fine with either

[CAD97]

the primary use case I can see for # in bare identifiers is hashtag-like

To clarify, #241 allows #ident as a bare ident, and both will of course still allow "r#ident" as a quoted ident.

Argument for allowing: transliterating CSS selectors, for e.g. CSS-in-KDL. Argument against allowing: using the syntax in KQL as a selector like CSS.

[lilyball]

Argument for allowing: transliterating CSS selectors, for e.g. CSS-in-KDL. Argument against allowing: using the syntax in KQL as a selector like CSS.

#foo in CSS is special-casing the id attribute. KQL doesn't have an equivalent to HTML's id, and using #foo syntax in KQL to mean something else might be confusing given its meaning in CSS, so I don't find the argument against compelling.

My inclination is to prefer #241 as well, as I think being able to write hashtags is neat. It also allows for doing things like writing Nix flake references as bare words, e.g. nixpkgs#hello.

[Lucretiel]

Can we squeeze #213 into this? The specific proposal is the addition of escaped whitespace in string literals– that \, followed by literal (non-escaped) whitespace, should consume and discard all that whitespace. This is a slight simplification of the Rust rule, which specifically requires that \ be followed by \n.

[hkolbeck]

I'm also a fan of #213, though it seems like there's some ambiguity in the discussion. Namely, does

- "x\
    y\
    z"

Translate to "xyz", "x y z", or "x\ny\nz"?

[zkat]

Can we squeeze #213 into this? The specific proposal is the addition of escaped whitespace in string literals– that \, followed by literal (non-escaped) whitespace, should consume and discard all that whitespace. This is a slight simplification of the Rust rule, which specifically requires that \ be followed by \n.

@Lucretiel do you have time to put together a PR with this grammar+prose change? I'm game.

[Lucretiel]

Can we squeeze #213 into this? The specific proposal is the addition of escaped whitespace in string literals– that \, followed by literal (non-escaped) whitespace, should consume and discard all that whitespace. This is a slight simplification of the Rust rule, which specifically requires that \ be followed by \n.

@Lucretiel do you have time to put together a PR with this grammar+prose change? I'm game.

Yes, tonight I can put that together :) should it be in the form of an amendment to SPEC.md?

[Lucretiel]

I'm also a fan of #213, though it seems like there's some ambiguity in the discussion. Namely, does

- "x\
    y\
    z"

Translate to "xyz", "x y z", or "x\ny\nz"?

I agree there's some ambiguity in the original. That example would translate to "xyz", because all literal whitespace after the \ is consumed and discarded. If you want to retain whitespace, it should either come before the \ or itself be escaped. I think my comment (#213 (comment)) succinctly describes this.

[zkat]

Can we squeeze #213 into this? The specific proposal is the addition of escaped whitespace in string literals– that \, followed by literal (non-escaped) whitespace, should consume and discard all that whitespace. This is a slight simplification of the Rust rule, which specifically requires that \ be followed by \n.

@Lucretiel do you have time to put together a PR with this grammar+prose change? I'm game.

Yes, tonight I can put that together :) should it be in the form of an amendment to SPEC.md?

yep!

I agree there's some ambiguity in the original. That example would translate to "xyz", because all literal whitespace after the \ is consumed and discarded. If you want to retain whitespace, it should either come before the \ or itself be escaped. I think my comment (#213 (comment)) succinctly describes this.

Is this what Rust does? I would've expected that to at least preserve the first newline. Then again, this is consistent with KDL's existing escline rule where \<newline> is the same as <non-newline whitespace>

[CAD97]

Is this what Rust does?

[playground]

[src/main.rs:2] dbg!("\
    here\
    is\
    an\
    example\
    ") = "hereisanexample"

[hkolbeck]

It's worth noting that bash behaves similarly as far as just dropping the newline, though it doesn't consume space afterward:

❯ echo foo\
… ❯ bar\
… ❯ baz
foobarbaz

With that I think xyz is the right output, and am +1 on including it in v2

Edit: Scratch that, I'm a space cadet:

❯ echo foo\
      bar\
      baz
foo bar baz

I'm more prone to emulating bash over rust, but I'm curious how others feel

[Lucretiel]

Bash's behavior is concerned with syntactic whitespace (ie, allowing commands to spread over multiple lines with line continuations). It doesn't meaningfully behave in terms of consuming or not consuming specific whitespace so much as it extends a line to the next line while retaining the separation of tokens for a command. In your echo example, all that's happened is that the foo and bar and baz have correctly been passed as different arguments to echo; it's no different than:

> echo foo             bar \
   baz
foo bar baz

Kaydle has basically the same behavior with its own line continuation syntax, where you can use a \ to continue a single node into the next line. All these nodes are the same:

node 1 2 3
node 1    2   3
node 1\
  2\
  3

#213 is instead concerned with treatment of escaped whitespace in strings, where I think the plain consumption of unescaped whitespace makes the most sense

Is this what Rust does? I would've expected that to at least preserve the first newline. Then again, this is consistent with KDL's existing escline rule where <newline> is the same as

Rust does just consume all whitespace, regardless of type. The canonical way to add newlines to a whitespace-escaped string to to escape them:

assert_eq!(
    "line 1\n\
    line 2\n\
    line 3\n",

"line 1
line 2
line 3
"
);

Though more commonly I use it to stretch out long sentences with simple spaces:

assert_eq!(
    "This is a sentence with a \
    lot of words in it.",
    "This is a sentence with a lot of words in it."
);

[zkat]

ahhh yes. I see your point. I'll change it back to be dedent-before-escape.

[zkat]

There, that's done :)

[tjol]

This example doesn't look right: surely ( and ) aren't allowed here?

[tjol]

Implementing the multi-line string and whitespace escaping rules is proving quite subtle.

When processing a Multi-line String, implementations MUST resolve all whitespace escapes after dedenting the string.

This sounds simple enough: if there are newlines in the string, I check that the indentation is consistent and remove it. Then, I handle the various backslash escapes.

That should take care of illegal strings like

  "
  foo \
bar
  baz
  "

(from in the spec), and legal strings like

"
    Hello
    \
         World
    "

(which is equal to "Hello\nWorld", if I'm understanding this correctly)

This algorithm does not work for this example in the spec:

    "Hello\n\
    World"

Before considering the \ escaping the newline, this looks very much like a syntax error: there is a newline in the string, but there is no initial or final newline. I believe the formal grammar also prohibits this string.

The spec prose appears to have a solution to this conundrum: (emphasis mine)

When a Quoted or Raw String spans multiple lines with literal, non-escaped Newlines, it follows a special multi-line syntax ...

So if all newlines in the string are escaped, it is not a multi-line string? To my mind that should imply that escaped newlines are not newlines for the purposes of dedenting, and contradicts the rule that dedenting comes before backslash escapes. Overall, not very satisfying.

I would suggest:

• removing “, non-escaped” from the definition of multi-line strings, requiring that every string that contains ANY literal newlines follows the multi-line-string rules. This matches what the formal grammar already says.
• replacing the offending example with

"
Hello\n\
    World
"

Edit: I've added a PR - #391

[tjol]

I wonder if the more intuitive way for strings to work would be:

1. remove escaped whitespace
2. dedent multi-line string
3. resolve other backslash escapes

This should have the same result as the #391 rule for all strings valid under that rule, but also accept more cases with escaped newlines.

[zkat]

@tjol sorry for the delay in responding:

I'm confused, what you're describing is definitely the intended behavior. You should still be able to write "multiline" strings by using whitespace escapes, they're just not going to be beholden to the multiline string rules, and can be easily detected by looking for the character sequence \<NL>.

But maybe allowing that is indeed too confusing and too painful to implement? So your suggestion means that whitespace escapes essentially no longer work unless you're in multiline string mode?

[tjol]

@zkat I think the way the spec is currently written – certainly the way I understood the 'letter of the law' while implementing – whitespace escapes basically can't escape newlines in single-line strings, yeah. But I agree that's probably not the way it should work. (Either way should be easy enough to implement)

I'll have another look at the wording and how to maybe clarify it (probably in a few days' time)

[zkat]

@tjol yeah it could definitely be improved. Thanks for being willing to take a look!!

[tjol]

Ok - alternative suggestion PR: #392

[tjol]

I don't think this matches the description above or the discussion at https://github.com/kdl-org/kdl/pull/286/files#r1483699764

Should this say something like "Empty lines may omit the whitespace prefix:" instead?

[j-tai]

Hey, I like the new barewords! I think it would be useful to document whether the different types of strings MUST be treated identically or not. In other words, we should specify whether the following MUST behave identically:

foo arg
foo "arg"

If they MAY be treated differently, then this could be useful for CLI-like syntaxes, such as

node1 --option  // treated as an "option"
node1 "--option"  // treated as a "positional argument"

[tabatkins]

The intention is definitely that they are identical, just different syntaxes for a "string". All the existing kdl1 locations that allow idents and strings (type annotations, node names, property names) don't allow you to distinguish between the two syntaxes, any more than they let you distinguish between the various quoted string syntaxes. Extending this to attribute values and property values shouldn't change things.

If you do have a node where you want to have both string positional args and boolean flags, the recommended way is to make the flags into boolean properties. node1 --option=#true and node1 "--option" are completely distinguishable. (If you're not mixing these two things, tho, then just having the presence of an ident string indicate a flag is perfectly fine, like node1 --option.)

[j-tai]

Fair enough. In that case, I think it would be worth adding this to the spec. Something like:

"Implementations MUST treat the different string forms identically."

[zkat]

that's not technically correct. A document-oriented parser might represent those differently, because they're represented differently in text and they want to maintain formatting and allow control of formatting. I'm not sure the spec needs any additional language beyond specifying "these two syntaxes represent the same data type," which is already expressed.

[j-tai]

I wonder if it would be a good idea to make this a bit more conservative? For example, we could also disallow NaN, -nan, +inf, Infinity, etc.

[zkat]

This PR has gotten really unwieldy, so I'm going to go ahead and merge it to main. We can continue iterating on v2 off that branch from now on. I've updated the readme to clarify this in case anyone gets confused when first visiting.

[Selonian83]

nice work pretty !!!