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 KDL language #7169

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add KDL language #7169

wants to merge 4 commits into from

Conversation

zkat
Copy link

@zkat zkat commented Dec 18, 2024

Description

This PR adds syntax highlighting support for the KDL Document Language. Note: there's two major versions of KDL, 1.0.0 and 2.0.0, with slightly different syntax. This PR includes a TextMate grammar primarily intended for 2.0.0, which is considered the "blessed" version going forward, but which can still highlight 1.0.0 for the most part, since 2.0.0 is mostly forward-compatible as far as anything TextMate would care about.

Checklist:

@zkat zkat requested a review from a team as a code owner December 18, 2024 23:43
Copy link
Collaborator

@Alhadis Alhadis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. You need to add vscode-kdl as a submodule, which should be done using the script/add-grammar utility:

    $ git rm -rf vendor/grammars/vscode-kdl
    $ ./script/add-grammar https://github.com/kdl-org/vscode-kdl.git

    You appear to have embedded the grammar directly inside Linguist, as opposed to registering it as a submodule.

  2. These samples are huge. Please remove the more excessive cases: kdl-schema.kdl, niri.kdl, zellij.kdl, and consider trimming nuget.kdl to no more than 100 lines (which is plenty).

  3. You haven't updated languages.yml with a definition for KDL. You'll need to add an entry like the following (making sure the entry precedes the KRL entry):

    Toggle diff

       - justfile
       extensions:
       - ".just"
       ace_mode: text
       language_id: 128447695
    +KDL:
    +  type: data
    +  color: "#FFB3B3"
    +  extensions:
    +  - ".kdl"
    +  tm_scope: source.kdl
    +  ace_mode: tcl
    +  codemirror_mode: yacas
    +  codemirror_mime_type: text/x-yacas
     KRL:
       type: programming
       color: "#28430A"
       extensions:
       - ".krl"
       tm_scope: none
       ace_mode: text
       language_id: 186

    After adding the above entry, run the utility script/update-ids to generate a unique language ID. Do not add one manually.

For more information on adding a new language to Linguist, see CONTRIBUTING.md.

@zkat zkat requested a review from Alhadis December 19, 2024 17:26
@zkat
Copy link
Author

zkat commented Dec 19, 2024

Note: I cut down the niri and zellij files because those are two of the most prominent formats in the community right now, and also are two examples of v1 kdl, so I didn't want to remove them altogether. I can still do it if you'd like and you think there's too many samples right now.

Copy link
Collaborator

@Alhadis Alhadis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just one more nit I want to clear up before hitting the big green "Approve" button.

lib/linguist/languages.yml Outdated Show resolved Hide resolved
@zkat zkat requested a review from Alhadis December 24, 2024 02:34
Copy link
Collaborator

@Alhadis Alhadis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@lildude, I've left the popularity metric for you to verify. At a cursory glance, the *.kdl extension appears to be well-established on GitHub.

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.

2 participants