Regression with colors

[edahlseng]

Prerequisites

• Put an X between the brackets on this line if you have done all of the following:
  • Reproduced the problem in Safe Mode: https://flight-manual.atom.io/hacking-atom/sections/debugging/#using-safe-mode
  • Followed all applicable steps in the debugging guide: https://flight-manual.atom.io/hacking-atom/sections/debugging/
  • Checked the FAQs on the message board for common solutions: https://discuss.atom.io/c/faq
  • Checked that your issue isn't already filed: https://github.com/issues?utf8=✓&q=is%3Aissue+user%3Aatom
  • Checked that there is not already an Atom package that provides the described functionality: https://atom.io/packages

Description

I just upgraded to Atom v1.38.1, and all of the colors for JSON files look wildly different. Specifically, the keys and values are now the same color (see attached screen shot), whereas previously they were different. I don't know what packages provide this syntax highlighting functionality, so if more information is needed, or this issue needs to be posted somewhere else, please let me know!

Versions

› atom --version
Atom    : 1.38.1
Electron: 2.0.18
Chrome  : 61.0.3163.100
Node    : 8.9.3

› apm --version
apm  2.1.7
npm  6.2.0
node 8.9.3 x64
atom 1.38.1
python 2.7.16
git 2.21.0

Additional Information

[Arcanemagus]

What theme are you using?
Does it look correct if you try the default One Dark/Light theme?
If you temporarily disable Tree-sitter parsers in the settings, do things look normal again?

[edahlseng]

Thanks for the help @Arcanemagus.

I'm currently using the One Dark UI Theme, and the Monokai Syntax Theme. Switching to the One Dark Syntax Theme brings back separate colors for keys and values:

Disabling the Tree-sitter parsers (while on the Monokai Syntax Theme) returns the old colors:

[caleb531]

@edahlseng For the time being, you can use my fork: https://github.com/caleb531/monokai

Just clone, cd into the project, and run:

apm install
apm link

I have a PR open that should fix the highlighting for the original package—it just hasn't been merged yet. I will check on the status of that, though.

[rsese]

Just to clarify @caleb531 @Arcanemagus - do y'all think this is an issue specific to that syntax theme or something that also needs to be resolved in this package?

[caleb531]

@rsese I don't think it's a syntax theme issue because the invalid syntax is not being tokenized with a scope in the first place. The tree-sitter parser does assign a syntax node called ERROR, but those ERROR nodes never get assigned a proper scope name like invalid.illegal, so themes can't target them anyway. Assigning the scope names was ultimately what my PR was meant to accomplish.

[Arcanemagus]

I don't think it's a syntax theme issue because the invalid syntax is not being tokenized with a scope in the first place.

This doesn't have anything to do with invalid JSON, but a difference in the scopes assigned by each parser.

The difference here that is changing things is that Text Mate is applying the meta.structure.dictionary.key.json scope, while Tree-sitter is simply adding the dictionary.key as part of the string itself.

Now whether that's an issue that needs to be fixed here in language-json, or in monokai (which hasn't been updated in 4 years...) isn't something I'm sure about saying either way. It looks like the method that one-dark-syntax added to highlight keys way back in atom/one-dark-syntax@e5c3a3c is still working, while the version that monokai implemented in kevinsawicki/monokai@4c523ef isn't now 🤷‍♂.

[rsese]

Now whether that's an issue that needs to be fixed here in language-json, or in monokai (which hasn't been updated in 4 years...) isn't something I'm sure about saying either way.

Just wanted to follow up about this - brought this up with some of the other maintainers and the consensus was that it's an issue for monokai rather than language-json. If you have feedback about that @caleb531 let me know.

Going to go ahead and close this out since the team believes it's not something to be resolved in language-json. This will be fixed for the monokai theme if Caleb's PR is merged in that repo and folks can otherwise use Caleb's fork as mentioned above.

[caleb531]

@rsese I agree. I will see about getting my monokai PR merged in, but as you said, it doesn't appear to be a language-json issue.