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

GitHub Syntax Highlighting #82

Open
miraclx opened this issue Feb 8, 2021 · 9 comments
Open

GitHub Syntax Highlighting #82

miraclx opened this issue Feb 8, 2021 · 9 comments
Labels
enhancement New feature or request

Comments

@miraclx
Copy link

miraclx commented Feb 8, 2021

Would be nice to have GitHub identify and add syntax highlighting to KDL-formatted files

https://github.com/github/linguist

@shieldo
Copy link

shieldo commented Feb 24, 2021

Will note that GitHub will likely not include a new grammar for a language unless it can be demonstrated that the language is in significant enough usage ("hundreds of repositories").

However, including it will require a TextMate-compatible language definition (i.e. for TextMate, Sublime Text Editor, Atom). For example, the YAML syntax highlighting uses the Atom package. The production of such a package for KDL would be the biggest step towards enabling GitHub to start using it when usage of KDL is sufficiently large.

See below comment re existing TextMate definition.

@larsgw
Copy link
Contributor

larsgw commented Feb 24, 2021

I think we have a TextMate definition here, but yes, as far as I am aware more usage is required for GitHub to consider adding it.

@zkat zkat added the enhancement New feature or request label Mar 5, 2021
@uncenter
Copy link

uncenter commented Oct 3, 2023

I think KDL does have enough usage now! 2.3k hits for searching for path:*.kdl which is enough under the current rules. I am going through the steps to add a language but problems were found with the grammar (linked in the previous comment):

  • Invalid regex in grammar: source.kdl (in syntaxes/kdl.tmLanguage.json) contains a malformed regex (regex "(?![\\{\}<>;\[\]\=,\(\)\s])[\u00...": PCRE does not support \L, \l, \N{name}, \U, or \u (at offset 29))
  • Invalid regex in grammar: source.kdl (in syntaxes/kdl.tmLanguage.json) contains a malformed regex (regex "(?![\\{\}<>;\[\]\=,\(\)\s])[\u00...": PCRE does not support \L, \l, \N{name}, \U, or \u (at offset 29))

Any regex pros wanna jump in here? This sequence doesn't seem to be anywhere in the grammar.

@uncenter
Copy link

uncenter commented Oct 3, 2023

I'm also not entirely sure what license the grammar is under. It needs to be one of these (see github-linguist/linguist/CONTRIBUTING.md#L125).

@uncenter
Copy link

Maybe we can fix the grammar issues before 2.0? #285

@zkat
Copy link
Member

zkat commented Feb 8, 2024

@uncenter The grammar is under Apache-2.0 by virtue of being in the vscode-kdl repo.

And yes, it would be nice to fix the grammar issues before 2.0! Although the textmate grammar has to kinda be redone to support 2.0.

@zkat
Copy link
Member

zkat commented Feb 8, 2024

Oh it looks like the fixes already got merged.

Still, we should probably wait to pursue syntax highlighting until 2.0 is out.

@zkat
Copy link
Member

zkat commented Feb 8, 2024

as far as that sequence being present, it's in the regex for identifiers: https://github.com/kdl-org/vscode-kdl/blob/main/syntaxes/kdl.tmLanguage.json#L97

I'm not really sure how to work around this. KDL, and especially KDL 2.0, has specific restrictions around unicode codepoints. Does this mean that linguist's PCRE doesn't support unicode matching at all, or is there a different syntax I should be using?

@zkat
Copy link
Member

zkat commented Feb 8, 2024

oh maybe all that needs to be done is to use \x instead of \u 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants