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.

Sign up for GitHub

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

Jump to bottom

KDL 2.0.0 support #3891

Open
zkat opened this issue Dec 22, 2024 · 6 comments · May be fixed by #3908
Open

KDL 2.0.0 support #3891

zkat opened this issue Dec 22, 2024 · 6 comments · May be fixed by #3908

Comments

@zkat
Copy link

zkat commented Dec 22, 2024

KDL 2.0.0 has been released!

Along with this new spec release, I've released [email protected], which supports both KDL 1.0.0 and KDL 2.0.0, and is able to translate document text between both versions.

This means that zellij would not need to change much to add 2.0.0 support, except update some KdlValue-related API calls/matches, since that enum has changed.

If the v1-fallback feature is enabled, you would even be able to smoothly support both v2 and v1, safely, since both versions are unambiguously parseable. You can even convert v1 docs to v2 docs automatically, and vice-versa!

let mut doc = kdl::parse(input)?;
doc.ensure_v1();

let v1 = doc.to_string();

kdl-rs's 2.0 parser is able to collect many syntax errors in a single pass, and so can give richer error messages than either the 1.0 parser (for now).

Always happy to answer questions! I hope this is useful to y'all!
@imsnif
Copy link
Member

imsnif commented Dec 23, 2024

Very cool! The meticulous work on backwards compatibility is very much appreciated. I'm looking forward to seeing this implemented.

Incidentally, some really nice related work from our community: https://github.com/dj95/kdl-fmt

@zkat
Copy link
Author

zkat commented Dec 24, 2024 •
edited
Loading

Unless someone claims this soon, I’ll probably give this an initial pass. It shouldn’t be much effort.

One thing that’s going to be interesting is what the errors should be: most people out there will likely be on v1 for a while, until docs, defaults, etc get updated. But the kdl-rs fallback mechanism only prints v2 errors if both formats fail (which would be the case in a truly malformed v1 file, for example).

I might want to poke at kdl-rs and make the fallback a bit more heuristic: that is, if both fail, do a quick scan of the document for “hints”, like unprefixed keywords, triple quotes, unquoted strings, or even the optional /- kdl-version 1/2 hint.

I might do that first, actually tbh.

(to clarify, this applies only when both versions fail to parse, and is only meant to improve error messages. It has no effect on parsing documents that are correct in v1, v2, or both)

@zkat zkat mentioned this issue Dec 24, 2024
Open
@ck3mp3r
Copy link

ck3mp3r commented Dec 28, 2024

Here's another quirk for you folks, and this might just me missing something; when using kdl-rs to generate values that only have single words or phrases joined by legal, non white-space characters, no quotes are added. Zellij however rejects unquoted values, even if they are legal according to spec.
I'm currently adding functionality to a utility I am writing to generate Zellij layouts programatically, hence why I stumbled on this. Would be good to know if I am missing something and this is just a red herring...

@zkat
Copy link
Author

zkat commented Dec 29, 2024

@ck3mp3r that’s because zellij currently runs only on KDL v1, which requires string quoting. Changing this is exactly what this issue is about :)

kdl-rs has a KdlDocument::ensure_v1 that takes any successfully parsed document and makes sure to turn it into v1 format as needed.

@ck3mp3r
Copy link

ck3mp3r commented Dec 29, 2024 •
edited
Loading

@zkat thanks, that worked, I could get rid of my hack 😄

zkat added a commit to zkat/zellij that referenced this issue Jan 2, 2025
@zkat
feat(config): add KDL v2 support
6151dd9 
Fixes: zellij-org#3891
@zkat zkat linked a pull request Jan 2, 2025 that will close this issue
Draft
@zkat
Copy link
Author

zkat commented Jan 2, 2025

PR up! #3908

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(config): add KDL v2 support zkat/zellij
3 participants
@zkat @ck3mp3r @imsnif