-
Notifications
You must be signed in to change notification settings - Fork 588
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
sublime-syntax.sublime-syntax #95
Conversation
I have a few issues with that syntax. I worked on a new yaml syntax definition in #90 and the lack of block scalars here is severe. Furthermore, quoted strings and their escape mechanisms are not accounted for and plain scalar termination rules (which are not that trivial, see more in Also, I acutally intended to work on a syntax def for this on my own, since I also maintain AAAPackageDev and this seemed like a good fit, but I wanted to wait for my YAML def to be merged first because I intended to re-use some contexts I defined there (and had different things to do afterwards). Furthermore, I'm interested in a stance from @sublimehq regarding special syntax highlighting of its own configuration formats in the shipped packages, including all the JSON formats. In the past they were not obviously (which is the reason why PackageDev exists). It also seems that package development here has stalled because pull requests haven't been merged or reviewed in a while. (It would also be nice if anchors/aliases and merge keys were actually implemented in sublime-syntax ...) |
Thanks for your feedback @FichteFoll . I'll work on scalar blocks.
in your YAML syntax, and they aren't correctly highlighted. I think simple forms like that with grouping parenthesis could be easily added. But I think it would be interesting to have a syntax for I'd be happy to contribute to |
What exactly do you mean here? |
You said:
So I was hoping that you could show me some examples of poorly highlighted code. |
I prepared this test file for myself a while ago. It will serve as the base for syntax tests if I eventually start working on this again, but I want to wait a bit first due to the reasons outlined above. |
Thanks, I'll have a look at it and come back. |
Hi @FitcheFoll, I used your test set and improved the regexes. I also made a few improvements on character classes and scope lists. |
Highlighting is broken for
Problem is "-?" |
Hi @gschenck thanks for your report. I think the |
Please have a look at https://github.com/sublimehq/Packages/blob/master/Lua/Lua.sublime-syntax, |
@gschenck sorry I was testing with a more recent version of the syntax. I commited the new version here. |
Works. Thank you. |
@@ -453,9 +453,9 @@ contexts: | |||
variable_call: | |||
- match: '(\{\{)([a-zA-Z_0-9]*)(\}\})' | |||
captures: | |||
1: punctuation.definition.parameters.begin.syntax | |||
1: punctuation.definition.arguments.begin.syntax |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for my use, I added the line: 0: variable.parameter
so that variables would be highlighted by the color scheme and thus be easier to spot :)
Thanks @keith-hall for your suggestions, I added them in. If any one has an idea on how to do this I'll be glad 😃 |
@@ -0,0 +1,44 @@ | |||
## SYNTAX TEST "Packages/User/scalaSublimeSyntax/sublime-syntax".sublime-syntax" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should be ## SYNTAX TEST "Packages/YAML/sublime-syntax.sublime-syntax"
.
That is why your tests aren't being picked up.
As a note, I added my own sublime-syntax highlighting to PackageDev, which has a lot of other goodies for syntax (test) development too. It's still in "alpha" because I want to finish all the other syntaxes as well, but it's very usable and an absolute must-have for anyone who works on syntax definition (not an under-statement). See https://forum.sublimetext.com/t/looking-for-packagedev-alpha-testers-sublime-syntax-highlighting/24270 for the announcement thread. |
As PackageDev is indeed a very helpful package not only for syntax development but handling all the different sublime-text-file-formats by providing functionalities which go far beyond syntax highlighting, I wonder whether this PR is still required to be open. |
I think PackageDev shouldn't be required for basic syntax editing, maybe including their sublime-syntax.sublime-syntax with default Packages is an alternative. |
It isn't. There is a syntax definition for YAML that you can easily use to edit syntaxes (and apparently wbond only uses that), so it's perfectly possible to edit syntax definitions without PackageDev. However, I wouldn't recommend it and installing a third-part package is arguably trivial. Furthermore, it allows us to provide more functionality that goes beyond highlighting, as @deathaxe mentioned, like context completions, scope name completions and the syntax test stuff with a much higher release frequency than would be possible with having the package in the default packages. Of course, it would still be nice if ST shipped syntax definitions for its config files, but as the maintainer and main developer of PackageDev, I enjoy the freedom in deployment and independence I currently have. |
I appreciate the convenience provided by PackageDev, and the sublime-syntax.sublime-syntax from PackageDev is also full-featured. However, not everyone is looking for full support, nor all the other awesome features from PackageDev, when only basic editing of sublime-syntax is needed. Currently YAML isn't an alternative, as it doesn't support indexing. I think a default syntax with only basic functions should be shipped by default, and only when more features are needed, install the package. Installing a Package is, yes, trivial, but the overhead of installing features not needed causes many troubles, e.g. conflictions, which can all be avoided by including only necessary features. |
I would consider editing a Another reason why I don't like things to be added as default package/syntax: It just updates SUPER slow due to ST's release cycle. Sure, there is a way that people can manually update it locally but installing a package is far simpler... I am not against approving this PR because I can still and will use Another question here. If we are going to have a default |
The PR is basically a meta question whether to include a syntax definition at all at this point. At least that's how I perceive it. However, I'm not the person to decide here. PackageDev's syntax definition is not self-contained, because it uses the separate syntax definitions for Oniguruma regular expressions and the generic JSON-focused context library I built for many ST configuration files. They could of course be embedded or included, but it's not as simple as just taking a single file and be done with it. While I would of course be okay with ST including a syntax definition for syntax definition by default, it then raises the question about the other files like settings or build system. And, because of what @jfcherng said, I would still want to develop PD in parallel because of quicker release cycles and the fact that many extra features provided through plugins are strictly dependent on tokenization of the files with the syntax definitions. I would also need to check how well they override each, which is based on the lexicographical order of the package names. @bumfo, maybe you could clarify on your areas of concern regarding conflicts of the features that PD provides? |
I think for right now it makes sense that https://packagecontrol.io/packages/PackageDev be the place for augmented syntaxes. The release cadence is separate, there is a group of people who are enthusiastic about what it provides, and the average user does not get into writing syntaxes. If they do, the default YAML syntax does work, I mean, I use it. 😄 We may revisit this at some future point, but for now I am going to close. |
Good for me :-) I totally forgot about this PR. |
Meta syntax :-)
I tried to cover as much as possible from the Onigruma syntax.