Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node space now required before children block #495

Open
larsgw opened this issue Jan 21, 2025 · 8 comments · May be fixed by #499
Open

Node space now required before children block #495

larsgw opened this issue Jan 21, 2025 · 8 comments · May be fixed by #499

Comments

@larsgw
Copy link
Contributor

larsgw commented Jan 21, 2025

Between v1 and v2, the grammar changed, requiring node-space before node-children:

base-node := slashdash? type? node-space* string
      (node-space+ slashdash? node-prop-or-arg)*
      // slashdashed node-children must always be after props and args.
      (node-space+ slashdash node-children)*
      (node-space+ node-children)?
      (node-space+ slashdash node-children)*
      node-space*

What I don't remember, was this change intentional (I don't see it in the changelog), or was it not? If not, is it a blessing in disguise or should we revert?

@zkat
Copy link
Member

zkat commented Jan 21, 2025

I... don't like that this is required now and I kinda feel like it's an accident

@larsgw
Copy link
Contributor Author

larsgw commented Jan 22, 2025

I didn't mean to come off very opinionated with "blessing in disguise", I'm fine either way.

@zkat
Copy link
Member

zkat commented Jan 22, 2025

do you think it should be required? I feel like tightly-packed kdl like this should be legal, if only for practical purposes of data transfer: foo{bar;baz{quux a b c}}

@larsgw
Copy link
Contributor Author

larsgw commented Jan 22, 2025

That seems fine to me. One thing I wonder about is the interaction with slashdash comments:

  • If foo{bar} is legal I would expect foo/-{bar} to be as well.
  • If foo/-{bar} is legal I would expect foo/-bar to be as well.

So, (node-space+ slashdash? node-prop-or-arg)* should maybe be (node-space* (node-space | slashdash) node-prop-or-arg)* or something equivalent.

@zkat
Copy link
Member

zkat commented Jan 22, 2025

no worries. I think this is a minor thing that we can justify slapping on as a patch.

I'm seeing the RFC process as another opportunity to iron out things we've been discovering since the initial 2.0.0 release, too. THAT is gonna have to be seriously solid, but it's going to be a long process.

@zkat
Copy link
Member

zkat commented Jan 22, 2025

and yes, I like your suggested edit to the grammar. I think we should do it, tbh. I'd love to hear from others, though.

@tabatkins
Copy link
Contributor

No real opinion, fwiw. As long as it's unambiguous I'm fine with compact parsing.

@zkat
Copy link
Member

zkat commented Jan 22, 2025

Let's get a PR, then? :)

larsgw added a commit to larsgw/kdl that referenced this issue Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants