optional-node-space around equals sign in properties

[larsgw]

The grammar for a property looks like this at the moment:

prop := string optional-node-space '=' optional-node-space value

However, the text says:

A Property is composed of a String, followed immediately by an equals sign, and then a Value.

Probably the latter needs to be reworded to allow whitespace, esclines, and slashdash comments.

(Sidenote, I'm a bit unsure if slashdash comments are good in this context. Firstly, node prop= /-"foo" "bar" would be equivalent to node prop="bar" which is not very obvious (it looks like "foo" and especially "bar" are just Values to me on first sight). Secondly, node prop=/-"foo" "bar" is invalid which seems a bit inconsistent, I feel like that would be clearer. ~Thirdly, I think node prop /-="foo" ="bar" is probably very annoying to parse, especially now prop is a valid Value.~)

[larsgw]

Also, I think node prop /-foo = bar is ambiguous, either a Node with a Value "prop" or with a Property prop=bar. Given that, we might need to change the grammar as well.

[zkat]

I think a good test for where /- should go is whether we would also allow comments in there.

Would prop = /-foo bar not make sense, but `prop = /foo/ bar would? Why? I feel like the latter is very sensible (and imo, so is the former)

[zkat]

The latter is definitely a bigger concern. I'm not sure what to do about that ambiguity: you definitely should be able to slashdash entire props...

[larsgw]

Maybe a better test for where /- should go, is whether the entity you're commenting out could occur there without being commented out. i.e. you already cannot put a slash-dashed Node between two Values, or between a Value and a Children block. So I would suggest no slash-dash before a = in a prop, and after a = only allow slash-dashed Values, not Properties. I would also prefer to remove the mandated whitespace between = and /- in the current grammar.

Does this work?

prop-value-space := plain-node-space* ('/-' plain-node-space* value plain-node-space+)*
prop := string plain-node-space* '=' prop-value-space value

Would prop = /-foo bar not make sense, but prop = /*foo*/ bar would? Why? I feel like the latter is very sensible (and imo, so is the former)

I think prop=/-foo "bar" would be confusing, but I thought prop=/-foo was allowed (it's not, yet), and with spaces it looks okay (in KDL v1, spacing around = was disallowed).

[eilvelia]

It makes sense that an entity next to /- is parsed as it normally would be, but the /- modifier then makes it fully "discarded", same as #_ works in edn (and clojure). This seems conceptually simple and also has no parsing ambiguities. I think it was similar in KDLv1 but later has been changed?

[larsgw]

What would be the "entity next to /-" though in the case of node prop /-foo = bar? It could be either foo or foo = bar, at least in the current grammar.

[eilvelia]

I mean that it would be parsed as if /- is not there, i.e. node prop foo = bar, which can only be <node-name "node"> <value "prop"> <property "foo" "bar">. Then the /- modifier is applied to the property (say, in a semantic action) and this property is not added to the AST. (But the current grammar is ambiguous, yes.)

edit: That is, I view /- just as an entity modifier in the grammar, which alters the semantics of the document.

[larsgw]

Ah I see, that makes sense.

[larsgw]

That explains why node prop=/-foo bar does not make as much sense to me then. Without /- it'd be <node><property "prop" "foo"><value "bar"></node>, making the effect of /- to be "remove the second part of <property> and replace it with the subsequent <value>".

[zkat]

So thinking about it, what we want is for slashdash to work in exactly three places:

1. On a node
2. On a node children block (before the {)
3. Before an entire node entry. That is, if applied to a prop, it has to precede the ENTIRE thing, including a type specifier. Ditto for arguments.

Does that sound like what we want? It seems to be a pretty straightforward rule

[larsgw]

That sounds good to me.

Are we okay with slashdashed children blocks in a non-final position? E.g. node foo /-{ node; } bar is currently allowed, I believe.

[zkat]

idk. Personally, I'm willing to live with that, even though it's a bit weird.

[tabatkins]

If it was easy to block I'd prefer to not allow that, but I'm fine with allowing it if needed, yes.

[zkat]

lol looking into making this change, this is what the spec actually says already:

* A [Node](#node) name (or its type annotation): the entire Node is
  treated as Whitespace, including all props, args, and children.
* A node [Argument](#argument) (or its type annotation), in which case
  the Argument value is treated as Whitespace.
* A [Property](#property) key, in which case the entire property, both
  key and value, is treated as Whitespace.
* A [Children Block](#children-block), in which case the entire block,
  including all children within, is treated as Whitespace.

I should change that to "including its type annotations" but other than that, this is what we've been talking about. The thing that needs to be done now is to fix the grammar to reflect this logic.

I was thinking about the restrictions around children blocks, but considering how the following is, I think, reasonable, I don't think we should bother with the added complexity:

node foo /-{ bar 1 } /-{ bar 2 } { bar 3 }

Likewise, and more interestingly, I think this should be legal, if we're going to be enabling the above use-case:

node foo { bar 1 } /-{ bar 2 } /-{ bar 3 }

That is, slashdashes after the children block should still be valid.

But this should not be:

node foo; /-{ bar 1 }

So that complicates the node terminator a bit, I think. We'll see.

Any thoughts? @tabatkins @larsgw ?

[zkat]

PR over here: https://github.com/kdl-org/kdl/pull/407