Scope names: overhaul support for bat themes
#17
Merged
Conversation
This file contains 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
Main reason: the default Monokai theme colorizes strings in yellow. As
stated in docs/Principles.md, this is a constraint for scope names &
theme support because it mirrors how clap colorizes `bat --help`.
Other than that, census of changes:
PROS:
- Monokai*: makes the colors for headings & arguments different. Very
important because this is the main theme.
- OneHalf* & TwoDark: makes the colors for headings & options different.
- base16*: from blue (= options) to green (= nothing), yay!
- DarkNeon: headings from beige (too similar to default white color) to
fluorescent yellow (much more visible!)
- GitHub (light theme): from default black color to dark blue. Not very
visible but still a net improvement.
CONS:
- loses the bold style for headings in Coldark* and Solarized*
- Nord: headings go from light blue to default gray color. Support for
this theme was already a lost cause anyways.
NEUTRAL:
- Dracula: from blue to yellow
- Coldark*: from blue to green, which makes it a bit more similar to
arguments (teal). But maybe the loss of bold aggravates this change?
- Solarized*: from orange to green. Dang, I really liked orange headings.
Same as Coldark about bold.
- Visual Studio Dark+: headings from blue to orange. They were pretty
similar to arguments (a lighter blue) so a bit of an improvement.
- ANSI: from blue (= options) to green (= arguments). Meh.
Whoever thought 2 scopes per line was a good idea? tsk tsk
Allow it to take options (like -U5) or arguments (like a commit SHA).
It's all PROS: - Coldark* and Solarized* regain the bold style on headings - Nord regains the blue color for headings (from default gray) This commit fixes all of the CONS from commit 0852965 It's cool to keep the constant.etc scope because it makes headings bold in the Zenburn theme.
Intended to fix base16 but doesn't. Fixes ansi.
Lots of weird changes in {a|b|c} |-separators
For comparison purposes, with what comes next.
The main improvement this gives is it fixes no-color arguments in base16 and arguments=headings in ansi, which are two themes I definitely wanted to support. Because they use the terminal's color palette, instead of arbitrary colors. It also brings back some cool colors I lost on changing the heading scope: blue on Coldark, orange on Solarized. It even improves support for gruvbox (but still bad). It's kind of weird to see Monokai arguments being purple instead of brown-orange, but it makes sense in the context of the larger body of themes. I will reuse that color (variable.parameter) on files' scopes probably. Things I tried: - comment: bad, subtle hues on most themes - support.type: bad - constant: not targeted by many themes - constant.numeric: may be an option - constant.other: an excellent option - constant.language: worse than .other - constant.character: same - storage.type: not particularly good - storage.modifier: bad, too similar to keyword (for subcommands) - support.constant: not particularly good - support.function: bad, too similar to entity.name.function (for options) - support.class: not particularly good - support.type: bad - support.module: bad - keyword.control: bad, mostly like keyword.other (for subcommands) - keyword.operator: same - keyword.declaration: same - variable.function: bad, often similar to entity.name.function - variable.language: bad, no support on Monokai - variable: bad, not matched by many themes - entity.name.tag: often similar to keyword, not good enough. Recap of scopes that I may use elsewhere: - variable.parameter - entity.name.tag - constant. variants - keyword. variants I basically combed through the common scope names here: https://www.sublimetext.com/docs/scope_naming.html
Cause I feel like it looks too prominent in Monokai, that it may make sense to switch it with the other-defs one (for subcommands mainly).
To free up the variable.parameter class for visual comparisons, now that I'm focusing on file definitions.
i.e.: `constant.other` (A) for subcommands and `keyword.other` (B) for
arguments & option arguments.
Because, on most themes [3], the colors for A coordinate better with
the colors for options: `entity.name.function` (C). As compared to the
colors for B.
Particularly, one or both of the following hold:
1. A's hue has more contrast to C than B, which helps make apart options
and option arguments.
2. B is significantly more vivid* than A, sometimes even more than C,
which puts the focus on option arguments and detracts from options. But
options are more important than their arguments.
* Vivid as in brighter / more saturated.
For (2) I also fancy that subcommands are displayed in a more vivid
color than arguments and option arguments.
[3] A recap of how the change works on most themes:
- Coldark *: (1) applies
- Dracula: neutral?
- GitHub Light: 2
- Monokai *: 2
- Monokai Extended Light: the opposite is true, worse off
- OneHalfDark: 2
- OneHalfLight: 2
- TwoDark: 2
- Solarized *: 2
- Snazzy: 2
- Visual Studio Dark+: 1
. ansi: 2 but may lose out a bit on 1 (yellow is usually brighter / more
saturated than magenta; but the later is closer to blue)
. base16: idem but swap yellow for red.
- gruvbox: neutral?
- zenburn: loses out on 1, but makes subcommands bold, which is better
than option arguments
I was quickly testing the swap without properly changing scope names and I missed these unchanged names when I committed. No effective changes.
This is more in line with how scope names work everywhere. Now they will be targeted with the same color / style as command arguments like this: `keyword.other.argument.cmd-help` And it can still be targeted with a different color than that like this: `option.name.function.option.cmd-help keyword.other.argument.cmd-help` Before, you needed to use a different selector for each of them. We should probably differentiate between option arguments and _plus_ option arguments but let's go with this until somebody complains. Less complexity.
Cause it's supposed to take the same color as section headings. This was pending since I changed those.
Context: Monokai Extended is the default theme for bat. It's also the theme we use for highlight regression tests. Earlier in this overhaul of scopes, I changed the scope for headings in order to better support most themes, particularly themes based on the 16 customizable terminal colors (ansi & base-16), which previously used the same color for headings & options. Before that change, Monokai used the same orange color for both headings and arguments (which is bad). After it, headings became a pale yellow. Later on, I also changed the scope for arguments, because it caused color repetitions in other themes I also care about. So in the end, for the default Monokai theme, we went from an overuse of the orange color to an absence of it. Which looks weird to me, I'm used to the orange. Furthermore, and perhaps more relevant, I'm not much a fan of the pale yellow color that headings took. I don't think it helps highlight them over other elements (options in particular), which is bad. So I looked for orange color assignations in the theme [1], and found the previous orange is a bit hard to target without messing up other themes. But there's this alternative hue of orange that's used on a few scopes, including this very specific one that other themes don't target: `meta.constructor.argument.css`. I don't even know what that is, Google didn't help. Coincidentally, this is a slightly different hue of orange, closer to gold, with more brightness. Which is great for highlighting headings over other elements. At the same time, it's close enough to the previous orange hue that users (or me) won't feel the difference unless they're looking for it. It's also closer to my preferred shade of yellow (cf. the change on docs/Principles.md). ETA: another nice side effect I noticed testing this against highlighted regression tests: this orange-golden color is closer to what Manpage headings get with that syntax (it adds a scope that's painted with the previous orange). The pale yellow looks weird next to that. The only other built-in theme where this tweak causes a diff is Dark Neon, where headings become a darker green (worse). But it's an acceptable regression because Dark Neon was badly supported already (options use the default foreground color). If this hack causes problems with other built-in themes down the line, I'm open to reverting it. For now, it's a net improvement in my book. [1] https://github.com/jonschlinkert/sublime-monokai-extended/blob/master/Monokai%20Extended.tmTheme
`option.name` is not a thing, `entity.name` is. Fixing this, a previous bug rears its head. But I already had a solution in a stash so :shrugs: The bug: de-scope by `clear_scopes: 1` doesn't work if the outer context assigns a composite scope (like `entity.name.function variable.parameter`). To properly clear that (and go back to default foreground color) you need to clear all the scopes except the base one (`text.cmd-help`) Which means that to properly do this in arguments (1 scope as opposed to 2 from option arguments), we should only pop 1 scope, not 2. This is not a problem in most themes, only in Gruvbox (which is broken anyways), so not a priority to properly handle this. The non-wrong way to do this is having a de-scope context for contexts that add 1 scope, 2 scopes, etc. As long as these context have to de-scope at some point. The correct way to do this is devising an entirely different algorithm, but refactoring this hack is not a priority
This changes the color for this element in Monokai, from orange (not the same as headings golden, but pretty close to) to blue. This affects all of the Monokai variants except Monokai Extended Bright. There's also some weird bug in ansi with how the color is not properly cleared after the token and it propagates to the "bla bla bla" explanation. I don't know if the error is on bat/syntect's side or ours.
Because it was yellow, same as subcommands. Now it's cyan. I was unable to fix base16 though (where FILE_DEF remains the default foreground color). Fixing that implies removing the `variable.parameter` scope and that would make a lot of other themes repeat colors. Supporting even one of the themes that uses the terminal's 16 configurable colors (ansi, base16, base16-256) is already plenty. The current version on main has a suboptimal ansi (headings' color == options) and a broken base16 (same plus arguments are default fg).
victor-gp
force-pushed
the
overhaul-color-theme-support
branch
2 times, most recently
from
a6355dd
to
1964733
Compare
In order to keep the PR diff to the point. I plan to update syntax & highlight regression tests on the merge commit. Plus, syntax tests passed on the first run of CI, but they shouldn't. I've changed the scope names for most classes yet I haven't updated syntax tests. Possibly an issue with the caching? I must look into this asap.
victor-gp
force-pushed
the
overhaul-color-theme-support
branch
from
1964733
to
3293a3f
Compare
|
Closing note: I don't want to do an overhaul of scopes ever again. This one was life-draining. |
victor-gp
added a commit
to victor-gp/bat
that referenced
this pull request
Dec 7, 2022
This update includes an overhaul of scope names to better support the set of themes included with bat. You can find a visual diff for every theme in this PR: victor-gp/cmd-help-sublime-syntax#17 This commit updates the cmd-help syntax test because the scopes (-> colors) have changed.
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 main motivations for this overhaul were:
What follows is a side-by-side diff of the color changes for every theme. Left is before, right is after. Dark themes go first, light themes last.
The images include italics. But for each theme that shows italics there's also a (default) version without italics, just run bat without
--italic-text.Emoji key:
Monokai Extended↗️
ansi ⬆️
1337 ➡️
Coldark-Dark ➡️
DarkNeon ➡️ 🚫
options not colorized.
Dracula ➡️
Monokai Extended Bright↗️
headings = pathnames, but at least now headings != arguments. pathnames are infrequent.
Monokai Extended Origin↗️
Nord ➡️ 🚫
subcommands & pathnames not colorized.
OneHalfDark ⬆️
Solarized (dark) ➡️
Sublime Snazzy ➡️
TwoDark ⬆️
Visual Studio Dark+↗️
better hue distribution across classes.
base16↗️
pathnames not colorized.
base16-256↗️ 🚫
subcommands colorized in black.
gruvbox-dark↗️ 🚫
headings = options, pathnames not colorized.
zenburn ➡️
Coldark-Cold ➡️
GitHub ⬆️
Monokai Extended Light ➡️
headings != arguments anymore. but previous color for headings had less brightness -> better contrast.
OneHalfLight ⬆️
Solarized (light) ➡️
ansi (with light terminal theme) ⬆️
base16 (with light terminal theme)↗️ 🚫
normal text barely visible, probably a grey variation (colors 0x7 or 0x8).
base16-256 (with light terminal theme)↗️ 🚫
normal text barely visible.
gruvbox-light↗️ 🚫
headings = options, pathnames not colorized.