-
Notifications
You must be signed in to change notification settings - Fork 45
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
vscode: output styling and formatting in "Wallaby Tests" is not as expected in most themes #2611
Comments
Aren't At any rate, it looks like this is a bug in the themes, not in Wallaby. In the case of One Dark Pro, it looks like it is somewhere overriding the You may refer to Wallaby's textMate grammar for In the case of {
"editor.tokenColorCustomizations": {
"textMateRules": [
{
"name": "Deleted",
"scope": "markup.deleted",
"settings": {
"foreground": "#ff0000"
}
},
{
"name": "Inserted",
"scope": "markup.inserted",
"settings": {
"foreground": "#036A07"
}
},
{
"name": "Underline",
"scope": "markup.underline",
"settings": {
"fontStyle": "underline"
}
},
]
}
} Which would make your Given that all of the default VS Code themes have values for the tokens that we are using for syntax highlighting, I think this is a bug in the themes that you are using. You should be able to pass on these details to the theme creators for them to adjust their themes. We'd be happy to assist communicating with them if you need help. |
Nope, distinct themes. Material theme can be found here. Both themes have millions of installs, so I think this is a significant issue. It's interesting that one dark pro is called material-theme, probably started as a fork at one point which might explain the common bug! I've noticed this problem is the case for 95%+ of the themes I have tried (and I have a LOT of themes installed, something like 30-40) . I remember quite recently going through my entire collection in order to find a theme that works with wallaby js output and finding that only 2 or 3 worked. It seems that the textmate scopes used by wallaby's output tmLanguage are quite uncommon? Are there appropriate, more commonly used alternatives that wallaby might opt to use? If the markup textmate scopes are appropriate, then I do think that this should probably be documented somewhere on the wallaby.js website. Just my 2 satoshi. Thanks for your instructions and guidance, I hope that this issue serves as a useful resource for anyone that runs into this problem in the future. |
Thanks - I don't disagree it's a problem, but it needs to be fixed by the theme creators.
I wanted to see how wide-spread this issue is. I checked the top 10 themes (by install count) and the issue affects exactly 50% of them. I've created an issue in the repositories of the 5 that weren't working so that they can fix the problem. The issue is reproduceable outside of Wallaby (see this sample repo). If you are using any other themes that have the issue, please report to them via their issue repo (you may provide my sample repo that includes steps to reproduce).
They are standard and required by many grammars, not just Wallaby. These are the most appropriate for us to use (and give us exactly what we need). |
FYI - One Dark Pro has already been fixed in v3.9.13 |
That's really proactive of you, thank you so much for submitting these tracking issues. |
FYI - Material Theme has been fixed in v33.2.0 |
On the wallaby.js marketing site there is a screenshot of wallaby's output showing a number of features:
In particular, the diff has color coding and highlighting.
Unfortunately, you don't get any highlighting in MOST themes, including the most popular themes:
THEME: Default Dark:
THEME: Material (the second most popular vscode community theme iirc)
THEME: One Dark Pro (THE most popular vscode community theme)
This is obviously not ideal, but I have been unable to find out what needs to be modified/added to these themes in order to support wallaby.js
The documentation for wallaby.js vscode should contain a section describing the theming requirements in order for diff to be displayed correctly.
The text was updated successfully, but these errors were encountered: