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

Make line-space a superset of node-space #440

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

tjol
Copy link
Contributor

@tjol tjol commented Dec 16, 2024

This is a proposal to simplify the line-space and node-space rules with the effect of allowing escline anywhere where whitespace is allowed.

This comes out of the discussion in #438.

The general idea (which I find intuitive) is that nodes can contain all whitespace except unescaped newlines, because these would end the nodes. The space between nodes can simply contain all whitespace. Sure, \ doesn't do anything, but so what? In the current grammar it is already allowed in a few select places where it doesn't do anything.

@eilvelia
Copy link
Contributor

I personally would very much support this. One of the things I didn't like about kdlv1 is that there's too many types of whitespace used in different contexts. Kdlv2 is better in this regard, and this change would make all whitespace rules (outside of strings) identical except for the non-newline requirement inside a node. It'll make parsing using a bottom-up parser easier (not requiring the lexer to emit escline as a token).

@zkat
Copy link
Member

zkat commented Dec 16, 2024

I really really support this. Has this broken any of the tests for your parser in interesting ways?

@tjol
Copy link
Contributor Author

tjol commented Dec 16, 2024

I'm not using the formal grammar directly in ckdl, so all I can say is that the way I understand this change is equivalent to what ckdl does, and that passes all the tests in this repo (and in #438).

There are no tests in release-2.0.0 right now that test that you can't escape newlines outside of nodes, and one test that tests an edge case where you can (slashdash_escline_before_node.kdl).

@zkat
Copy link
Member

zkat commented Dec 17, 2024

@tjol do you mind adding some tests to confirm that free esclines are valid? Or were they already there?

@zkat
Copy link
Member

zkat commented Dec 17, 2024

oh duh nevermind that's what #438 was about lol. Sorry my head's all over the place.

@tjol
Copy link
Contributor Author

tjol commented Dec 17, 2024

Still, can't hurt to add a few more!

@zkat zkat merged commit 23918bd into kdl-org:release-2.0.0 Dec 17, 2024
1 check passed
zkat added a commit that referenced this pull request Dec 22, 2024
* Release KDL 2.0.0

* fix grammar for multiline quoted strings to allow escaped whitespace on closing line

* Add unicode-space to raw string

* Remove nonexistent equals-sign from the grammar (#435)

* fix multiline string tests

* grammar: fix disallowed-keyword-identifiers and string-character (#436)

* Back out "fix multiline string tests"

This backs out commit 0c5604b.

* add extra javascript implementation (#437)

* reword interaction multiline + whitespace escape (#439)

* More tests for backslash behaviour (#438)

* More tests for baskslash behaviour

* Incorrect example of escaped final newline

* Test with non-literal indent

* Make line-space a superset of node-space (#440)

* Allow escline everywhere

* escline tests

* Always escape \ inside single quotes in the grammar text (#441)

to match the other uses of it and the metalanguage description below

* Add tests for mandatory whitespace between arguments or properties (#442)

* Add an optional version marker (#444)

* Add version marker to the grammer

* Add version marker to the Changelog

* Update SPEC.md

Co-authored-by: eilvelia <[email protected]>

* add a mandatory newline after the version marker

* add mandatory space between version number

---------

Co-authored-by: eilvelia <[email protected]>

* Fix a changelog line erroneously truncated in #444 (#445)

* fix: move vertical tab to the line-breaking whitespace to match Unicode (#446)

* add vertical tab change test

* final tweaks before release

---------

Co-authored-by: eilvelia <[email protected]>
Co-authored-by: Bram Gotink <[email protected]>
Co-authored-by: Thomas Jollans <[email protected]>
Co-authored-by: Evgeny <[email protected]>
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 this pull request may close these issues.

3 participants