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

Add an explicit marker for v2 to differentiate from v1 for non-programming parsers #443

Closed
eugenesvk opened this issue Dec 21, 2024 · 14 comments

Comments

@eugenesvk
Copy link
Contributor

This was originally raised in #372 (comment), but is still unresolved. The suggestions offered in that discussion are only applicable to the full-blown programming parsers that can allow easy application of arbitrary logic with arbitrary "backtracking".

But for "simpler" cases like syntax highlighting it's not a viable option since you don't have that kind flexibility.

Case in point: the only syntax highlighting extension supporting v2 (VSCode) illustrates it - it doesn't support both v1 and v2, instead it suggests installing a separate extension for v1, but then which of the extensions would you apply to an actual file if there is no indicator???

Why not add something like %YAML 1.2 as an optional indicator on the first line? Or maybe, in the unique spirit of KDL, the first slash-dashed node on the first line could signal version?
/-v2
/-v 2
/-kdl v=2

@zkat
Copy link
Member

zkat commented Dec 21, 2024

I really strongly don’t want this—I think it adds too much noise, and is ultimately unnecessary.

For things like syntax highlighters, all syntax highlighters I know of usually support a “heuristic” that you can provide to disambiguate files with the same extension. I haven’t done that with the VS Code one mostly because time, but it’s definitely a thing that should be done

@eugenesvk
Copy link
Contributor Author

support a “heuristic”

That's precisely what I'm asking KDL to add!!!

Here it is from Sublime https://www.sublimetext.com/docs/syntax.html#header

Header
The allowed keys in the header area are:

file_extensions ...

first_line_match
When a file is opened without a recognized extension, the first line of the file contents will be tested against this regex, to see if the syntax should be applied.

There is no magic that would allow syntax highlighters to guess the syntax if the syntax supports nothing!

I mean, it's impossible to do in the general case even with the flexibility of programmable parsers: let's say my syntax highlighter is awesomely flexible and allows me to parse both and pick whichever version has no version-revealing errors (might be a weird editing experience where adding v2 syntax at the end of the file would suddenly conver the whole document to v2 and highlight errors in all the previosly valid doc which might not even be visible to the user).

But then it turns out that the user is editing a config file that will be used in an app that supports a different syntax version! So my flexibly helpful highlighter failed - instead of accepting valid whatever-matched version it should've been strict and rejected the wrong version. But if there is no extension and no in-band heuristic, how would it know???

I think it adds too much noise

It's signal, not noise! But also .kdl2 would add 0 textual noise if that's such a big concern.

@zkat
Copy link
Member

zkat commented Dec 21, 2024

https://github.com/microsoft/vscode/blob/main/extensions/yaml/syntaxes/yaml.tmLanguage.json It looks like it’s pretty straightforward to write a tmLanguage with any custom heuristic to detect v1 vs v2.

But if it’s really important, I would take a PR that adds a note to the spec that /- kdl-version 1/2 MAY be added to the beginning of a kdl document, optionally preceded by the BOM, and parsers MAY use that as a hint as to which version to parse as, but are under no obligation to do so. Basically just an agreed-upon convention.

But I really don’t think it will be needed in practice.

@zkat
Copy link
Member

zkat commented Dec 21, 2024

Oh and I don’t want a .kdl2 extension because I really don’t want multiple extensions—it complicates some stuff, such as adding highlighting support in major platforms like GitHub.

@zkat
Copy link
Member

zkat commented Dec 21, 2024

Note that I’m tagging 2.0.0 in the next couple of hours, so if you want this in, it needs to be done ASAP, or it’s going to go into a minor addendum post-2.0.0 and thus not part of the official spec.

@eugenesvk
Copy link
Contributor Author

eugenesvk commented Dec 21, 2024

So something like this?:

document := bom? version? nodes

// Version marker
version := '/-' unicode-space* 'kdl-version' unicode-space* ('1' | '2')

I know the real slashdash would allow newlines between /- and kdl-version, but for the simple version it's better mandating a single liner

Could try to add it shortly to this latest branch https://github.com/kdl-org/kdl/tree/release-2.0.0

@eilvelia
Copy link
Contributor

Note that if the majority of kdl documents do not specify the version marker, it wouldn't really help syntax highlighting in the general case

@eugenesvk
Copy link
Contributor Author

That's fine, my guess is that the majority of docs would be v2 and the default would be v2, but then for the minority you'd have a way to avoid errors

@eilvelia
Copy link
Contributor

eilvelia commented Dec 21, 2024

well, per the legacy v1 spec that version marker would still be illegal! (edit: that is, not part of the spec)

@eugenesvk
Copy link
Contributor Author

eugenesvk commented Dec 21, 2024

That's still a legal node. But that's also not something the parser has to be very strict about, e.g., I do plan to add this to the v1 highlighter (though would prefer the v1 spec to reflect that officially)

@zkat
Copy link
Member

zkat commented Dec 21, 2024

@eilvelia how is /-foo 1 illegal in v1?

@eilvelia
Copy link
Contributor

eilvelia commented Dec 21, 2024

@zkat i instead meant to say it would not be part of the v1 spec (hence the edit on that message). so by this logic, it can only be used to detect v2, not v1.

@zkat
Copy link
Member

zkat commented Dec 21, 2024

@eilvelia it won’t be part of the v1 spec explicitly but it will work just as well. It’s ultimately just a suggestion we’re putting out there.

We could also add a note in the compatibility section of the v1 spec about it

@zkat
Copy link
Member

zkat commented Dec 22, 2024

I added a note to SPEC_v1.md about this, and it was also added to the main spec. Closing

@zkat zkat closed this as completed Dec 22, 2024
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

No branches or pull requests

3 participants