Skip to content
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

Compatibility with Night Owl VS Code theme #1

Open
wooorm opened this issue Mar 25, 2023 · 4 comments
Open

Compatibility with Night Owl VS Code theme #1

wooorm opened this issue Mar 25, 2023 · 4 comments

Comments

@wooorm
Copy link
Owner

wooorm commented Mar 25, 2023

Originally reported by @kentcdodds at mdx-js/mdx-analyzer#318

Steps to reproduce

Install the latest version of vscode-mdx
Open a markdown file with some markdown, even simple markdown will do. Here are some examples:

# Hello world

This **should** be highlighted

image

import Comp from './comp'

<Comp prop={34} prop2="hi">
  This: `should be syntax highlighted`.
</Comp>

image

## This should be highlighted

And _this_ should be too...

image

This is a list:

- why
- are
- these
- hyphens
- green?

No big deal, just kinda surprised...

image

Expected behavior

Things like headings, strong, italics, and inline code blocks should be highlighted.

Actual behavior

The colors are either non-existent or different from what I was used to before. The components look great though 👍

I'm using Night Owl, but these behaviors show up in others as well. I guess what I would really like is if my MDX highlighting looked as close to the regular markdown highlighting as possible.

Here's what the examples above look like with regular markdown highlighting in Night Owl:

image

image

(Yeah, MDX does better on this one for sure)

image

image

This could just be a case of "someone moved my cheese," but I do think there could be something wrong with the highlighting of the headings, strong, italics, and text in the back-tics.

Thanks!

@remcohaszing
Copy link

remcohaszing commented Mar 25, 2023

I agree. I also like to have inline markup fully highlighted, instead of just the wrapping tokens.

Headers are highlighted though. That might be theme related. Maybe it’s best to keep the token names as close to the VSCode markdown grammar as possible? (Assuming that one is the most widely used)

Also I like the highlighted list item markers.

@wooorm
Copy link
Owner Author

wooorm commented Mar 25, 2023

Hey Kent! :)

Thanks for the detailed report. And for the reference screenshots!

Important to know, is that the goal of this project is broad: to work in GitHub and in VS Code and in other places.
The most important is compatibility with GitHub, because we can’t change that. And that’s what tons of folks will see.
Then come VS Code themes, particularly the defaults first, then stuff like Night Owl!

Every tool and every theme will select different things and choose how to show them. So it’s always going to be different.

Here’s what I get with VS Code Solarized:

Screenshot 2023-03-25 at 09 32 51

Screenshot 2023-03-25 at 09 33 09

Screenshot 2023-03-25 at 09 33 21

Screenshot 2023-03-25 at 09 33 36

So several things do work there, such as headings!

Headings

And that’s where we find a very important bug in Night Owl:

Looking at almost all scopes in Night Owl, they are super specific. Specific to JS, or to JSX, or to markdown.
And they’re not supposed to be like that, they’re supposed to as vague as possible (markdup.heading), unless you really need to differentiate markup.heading.markdown from others (e.g. markup.heading.html) or so.

So that’s where a lot of the problems come from!

Inline code

  • GitHub : markup.raw
  • VS Code Solarized: markup.inline.raw
  • VS Code Night Owl: markup.inline.raw.markdown

I think this is a weird choice in VS Code themes to go with markup.inline.raw.
markup.raw.inline makes more sense, then you can also have markup.raw.block.
But I’ve just pushed the name VS Code uses: 60fa64b.
Note that Night Owl still should drop the .markdown suffix.

Lists

Yep, the markers will typically be highlighted. Also the digits in ordered lists.
As for why it’s green for you?
The marker is tagged as variable.unordered.list.mdx.
This is the name atom/language-gfm uses.
I’m not sure why it’s a “variable”, but well, everyone supports it!
Including Night Owl.

Emphasis (italics), strong (bold)

This is going to be a whole discussion, so I’ll open a separate issue for this!


I think the main take away here is that there’s a bug in Night Owl, it strictly specifies that only several scopes provided by vscode-markdown-tm-grammar get highlighted, which prevents this grammar from working.

wooorm added a commit that referenced this issue Mar 25, 2023
VS Code uses `markup.inline.raw`, which is weird (`markup.raw.inline`
or so makes more sense), but oh well.

Related-to: GH-1.
@kentcdodds
Copy link

Thanks! I know it can be frustrating to have to triage issues that turn out not to be relevant for your own project, so thank you for taking the time to help me with my problem. I've opened an issue on Night Owl: sdras/night-owl-vscode-theme#319

@wooorm
Copy link
Owner Author

wooorm commented Mar 29, 2023

No problem, thanks for raising an issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants