Visual Studio 1.10 (released 1st March 2017) breaks themes

[cjke]

I'm using the Halflife theme, which was working perfectly for both Javascript and PHP yesterday, is now broken in the latest Visual Studio Code.

Left is VSCode, right is what it looked like yesterday (and what it looks like on the colorsublime website):

[cjke]

Happy to do a PR, if there is a recommended contribution guide

[gerane]

@cjke I'm a bit tied up, just had a new baby. Let me see if I can figure out what changed in Code first. Was this the stable or insiders?

Any ideas @tyriar ?

[cjke]

No worries @gerane - happy to have a fiddle. It isn't mission critical stuff 🚀 Stable - the announcement went out this morning about 1.10

Congrats on the baby btw

[Tyriar]

@gerane I'm not aware of any changes in this area. @cjke you sure you didn't upgrade from 1.8?

[cjke]

I'm sure - I was running 1.9 yesterday (I did a fresh install of the entire mac only two days ago, so every app has been running latest).

[gerane]

I've been seeing a few changes in insiders, but this one seems much more extreme than the others. I believe they have been altering how the scoping works. So, I'd like to see what might have changed since I'll likely have other themes impacted as well.

I've seen them say they are fixing broken scoping, but if the theme looked right, and breaks after fixing scoping, then I'm going to have a lot of headaches 😀

[cjke]

@gerane yeah I bet!

If there is something I can do from my end, just let me know.

[gerane]

If you could get me some more code samples from the languages you've seen issues with that would be great.

Also, if you want to get more information on what scope is being applied, you can enable the developer TM scope option from the command palette.

[gerane]

Also, looks like I'm not the only ones who's themes were broken. Microsoft/vscode#18357

[cjke]

Woah, yep seems like there is an ongoing discussion over there.

I will get some samples together in the next hour or two.

Cheers mate

[gerane]

@tyriar I commented on that one expressing my opinion about unannounced breaking changes. I think lack of sleep and everything else has me a lot more agitated than normal. Edit: meh, looks like wife has flu or something too.

If I am understanding their changes, weren't they supposed to get code more in line with sublime and TM themes? I'm seeing changes lately that do the opposite. The theme looked like sublime/TM theme, but now they are off. This seems like it might be similar to that issue I messaged you about. I got too busy with baby stuff and wasn't able to file an issue.

[cjke]

Included some example snippets with scopes as per above:

PHP

Current:

Expected:

Scopes: $foobar

$foofoo

function keyword

Javascript

Current:

Expected:

Scopes:

[cjke]

Btw - just read your comment on the other thread, I think fair call. Again, if there is something I can help with, just let me know.

[Tyriar]

https://github.com/Microsoft/vscode/issues/18357 was 1.9 though, not 1.10. The changes are definitely expected to get it more inline with Sublime. They are also definitely the right thing to do for performance and correctness reasons (it's the reason minimap is in 1.10).

It was a failure on part of the team which I raised after seeing reports like https://github.com/Microsoft/vscode/issues/18357, there was a blog post planned but it didn't make it before the release (not sure the reason). Here's the post to give some context https://code.visualstudio.com/blogs/2017/02/08/syntax-highlighting-optimizations

Still not sure what it would be in 1.10, can you verify whether it repros in 1.9? Here's the installer for 1.9.1 https://az764295.vo.msecnd.net/stable/f9d0c687ff2ea7aabd85fb9a43129117c0ecf519/VSCodeSetup-1.9.1.exe

[cjke]

I'm happy to help where I can, though I'm not especially close to this software in this particular case (the case of Visual Studio Code) - I would consider myself an enduser (rather than a theme or plugin developer).

If you (@Tyriar) could provide the mac installer, I can see if it repro's in 1.9 if you like.

[Tyriar]

@cjke thanks, here it is https://az764295.vo.msecnd.net/stable/f9d0c687ff2ea7aabd85fb9a43129117c0ecf519/VSCode-darwin-stable.zip

[cjke]

1.9 looks fine (1.9 left, 1.10 right):

Scope comparison:

[Tyriar]

This might actually be https://github.com/Microsoft/vscode/issues/20141

[cjke]

@Tyriar Thanks, sounds like we should hold tight until that issue plays out

[Tyriar]

If this is indeed the issue you could fix the problem by removing any trailing commands in scope lists.

[cjke]

I've had a look at https://github.com/gerane/VSCodeThemes/blob/master/gerane.Theme-HalfLife/themes/HalfLife.tmTheme and there appears to be no trailing commas.

[alexdima]

In 1.10 we haven't made any fundamental changes to how themeing works, but we did consolidate some color parsing code (there were 2 places where we had some color parsing code doing similar things and now it's been consolidated into one implementation).

I think red is the result when the parser fails to parse a color. Let me dig deeper and try to find out what's going on.

[cjke]

Great - thanks @alexandrudima (btw excellent work on VSCode - I'm a big sublime to code convert)

[alexdima]

It appears that two unexpected empty foreground colors cause the color parser to miss-behave:

https://github.com/gerane/VSCodeThemes/blob/master/gerane.Theme-HalfLife/themes/HalfLife.tmTheme#L213

https://github.com/gerane/VSCodeThemes/blob/master/gerane.Theme-HalfLife/themes/HalfLife.tmTheme#L485

Editing those two locations to have a color (e.g. #FFFFFF) makes the theme work as expected.

I don't yet understand why an empty foreground color ends up affecting the default foreground color. I will have to dig even deeper :)

[alexdima]

Now I understand, the missing colors were somehow causing also the Inspect TM Scopes widget to malfunction, and to report that no theme selector matched:

The theme selectors for variable.parameter and variable.other had empty foreground colors, and the color parser decides to parse "broken" inputs as red (sort of like an error), causing all variables to be colorized in red. Once colors are defined, the widget also reports what's going on correctly:

I would be grateful if you could workaround our parsing issue for now and define colors in those two locations.

If anyone knows, what's the intent of writing an empty foreground color? Is the intent for the scope to get the default foreground color?

[cjke]

Farrrr - awesome research there @alexandrudima

I have zero idea around intent (not a theme dev), but I will submit a PR to @gerane to (selfishly) cover just the Halflife theme. I'm guessing the main piece of work here is identifying which themes in his big collection of themes has the missing locations, then filling them in.

However, the good news is, at least its on the radar now, so other people hitting the same problem at least know a) the temp work around and b) there could be a long term solution.

[alexdima]

Thank you @cjke

I'm also looking into a fix on the vscode side of things, since so many other themes might be impacted, a fix that would also go in the upcoming recovery stable build.

[gerane]

Sorry it took longer than expected to get caught up on this. The second week went way worse than the first and I got almost no sleep. Baby decided he just wasn't going to sleep.

@alexandrudima thanks for looking into this. I ported these themes from sublime themes, so I am assuming it was to use the default scope. I've seen some odd behaviors in other themes where their Inspect TM Themes was not properly showing what the actual theme was. For example

You can see that the json is all white, but the TM Scope shows the orange color.

[gerane]

@alexandrudima I went ahead and merged this, but would like to know if you know what might be going on in the images above

[alexdima]

@gerane Sorry for the slow response. I've opened Microsoft/vscode#23460 to investigate it.

[alexdima]

Mystery solved. There are two distinct rules that have the exact same scope selector. The last one wins:

The inspect scopes widget is misleading and shows the wrong information (the inspect scopes widget goes through an entirely different code path)