-
Notifications
You must be signed in to change notification settings - Fork 62
update syntax highlighting to more closely match existing languages #110
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
Open
Techatrix
wants to merge
10
commits into
ziglang:master
Choose a base branch
from
Techatrix:syntax-highlighting
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change shouldn't have any functional changes but should make the next commits more readable.
The previous mapping made no sense. The mapping of threadlocal and comptime will be changed in future commits. As a reference I used the builtin syntax highlighting from neovim for the following languages that have similar keywords to Zig's `var` and `const`: - Rust: `let` - Go: `var` - Javascript: `var`,`let`, `const` They all map their keywords to the Keyword syntax highlighting group.
All control flow keywords are now mapped to either Keyword, Statement or Conditional. Previously some of them would be mapped to Special, PreProc or Macro. As a reference I used the builtin syntax highlighting from neovim for the following languages that have similar control flow keywords to Zig: - Rust: - Keyword: return break continue - Conditional: match if else - Repeat: loop while (actually maps to Conditional) - C - Statement: goto break return continue asm - Conditional: if else switch - Repeat: while for do - C#: Conditional, Repeat - Conditional: else if switch - Repeat: break continue do for foreach goto return while - JS - Statement: return with await yield - Conditional: if else switch break continue - Repeat: while for do in of - Go - Statement: defer go goto return break continue fallthrough - Conditional: if else switch select - Repeat: for range - Zig (old) - Macro: defer errdefer - PreProc: asm - Special: return break continue - Conditional: if else switch - Repeat: while for - Keyword: and or orelse - Zig (new) - Statement: return break continue asm defer errdefer and or orelse - Conditional: if else switch - Repeat: while for The try and catch keywords will be adjusted in a future commit.
The nosuspend keyword is syntactically different from the other keywords so this mapping isn't quite right.
- volatile, linksection and allowzero obviously aren't types. The differentiation between what is mapped to "StorageClass" or "PreProc" isn't very clear. At least it's less scattered than the previous mappings.
As a reference I used the syntax highlighting that neovim uses for the following languages that have similar keywords to Zig's `pub`: - C++: - Statement: public protected private - Rust: - Keyword: pub - C#: - StorageClass: internal private protected public - JS: - Keyword: private protected public
The `error` keyword has two different appearances in sytnax that should be differentiated between. For now, this keyword remains mapped twice which isn't correct. I have tested these changes with the following colorschemes: - folke/tokyonight.nvim - morhetz/gruvbox - catppuccin/nvim - joshdick/onedark.vim - Neovim's "vim" colorscheme All of themes appear to map "Exception" to "Statement" which matches well with other keywords in said group.
The "Keyword" group now only has keywords that introduce new declarations
Previously builtin functions were the only thing mapped to "Statement". Now, some control flow keywords are mapped to "Statement", so builtin functions are instead mapped to the more reasonable "Function" group. I believe that "Statement" was previously picked to ensure that builtin functions get a color that would stand out and be unique. Function may not get an equally prominent color but still be unique (usually). Depending on how this affects various colorschemes, it may also make sense to map builtin functions to a different group like "Special".
Most colorschemes will still map "Boolean" to "Constant".
065d3f7
to
aa241a5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current syntax highlighting that is provided for various keywords is in complete disconnect from any conventions established by syntax highlighting configurations from other languages. Here are some examples:
var
,const
,comptime
andthreadlocal
are mapped to the "Function" highlighting group by defaultdefer
,errdefer
andextern
are mapped to the "Macro" highlighting group by defaultvolatile
,linksection
andallowzero
are mapped to the "Type" highlighting group by defaultreturn
,break
andcontinue
are mapped to the "Special" highlighting group by defaultHaving looked at the syntax highlighting of languages like C, C#, Rust, JavaScript, Java and Go, I haven't found a single languages with as many unusual choices as this plugin. My guess is that this syntax highlighting has been developed with some kind of flawed methodology of choosing whatever gives the desired highlighting on some specific colorscheme.
Syntax highlighting configurations and colorschemes do not existing in isolation. We should try our best to match existing configuration of other languages and only deviate from them if we have a good reason for doing so. I ended up changing the configuration for many different keywords. The reasoning for these changes is explained in the individual commit messages.
Here is how these changes affect some popular colorschemes:
Show Images