Closed larsgw closed 1 year ago
LGTM, but I'll let others take a look too in case I missed something. I'll also need to update the 2.0 grammar sometime soon after this lands..
I actually want to change some of the whitespace rules now that I look at it. It's missing whitespace after =>
, I think whitespace should be allowed in more places, and I want to allow newlines in all/most whitespace if that's okay with you.
I'm fine with that
Should this be a breaking change? Or just make it non-normative until a new major release?
I tried to split the patch but everything got sent at once so the commit message is a bit misleading, but I:
props()
, values()
)I think we should just make KQL 2.0.0 and be done with it, instead of trying to incrementally improve 1.0, but that's just me. wdyt?
That's fine with me too but I think that should be after/together with KDL 2.0.0, not before (given the pending changes to identifiers). I guess KQL 2.0.0 would also include the new selector operators? I'm excited for those.
Can you update this PR to target kdl-v2
instead, then? You'll have to rebase, but idk if you'll run into conflicts.
Nevermind, that didn't really work I think. I'll rebase locally and force-push.
Then I've removed the part of the grammar concerning the map operator too, since that was removed.
Might need to be more restrictive.