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

Headings H3 - H6 are assigned incorrect textmate scopes #483

Closed
4 tasks done
icorbrey opened this issue Nov 11, 2024 · 12 comments
Closed
4 tasks done

Headings H3 - H6 are assigned incorrect textmate scopes #483

icorbrey opened this issue Nov 11, 2024 · 12 comments
Labels
📦 area/deps This affects dependencies 💪 phase/solved Post is done 👶 semver/patch This is a backwards-compatible fix 🐛 type/bug This is a problem

Comments

@icorbrey
Copy link

Initial checklist

Affected packages and versions

v1.8.11

Link to runnable example

No response

Steps to reproduce

  1. Paste the following text into a VSCode window:
# Heading 1

## Heading 2

### Heading 3

#### Heading 4

##### Heading 5

###### Heading 6
  1. Execute "Developer: Inspect Editor Tokens and Scopes" through the Ctrl+Shift+P menu
  2. Inspect the scopes of each heading

Expected behavior

Since H1 is assigned markup.heading.atx.1 and H2 is assigned markup.heading.atx.2, H3 - H6 should be assigned markup.heading.atx.3, markup.heading.atx.4, and so on.

Actual behavior

H3 - H6 are all assigned markup.heading.atx.2.

Affected runtime and version

Not sure, it's in the most recent version of VSCode

Affected package manager and version

No response

Affected OS and version

Ubuntu 22.04.5 LTS

Build and bundle tools

No response

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Nov 11, 2024
@icorbrey
Copy link
Author

A visual example of what's occurring, I'm writing a theme that differentiates each heading level with color and this is currently broken on MDX. Expected:
image

Actual:
image

@icorbrey
Copy link
Author

Looks like the issue is upstream in https://github.com/wooorm/markdown-tm-language, should be an easy fix? H3 - H6 are already targeted in the TMLanguage file, just not assigned the right IDs

@icorbrey
Copy link
Author

PR is in to hopefully fix this at wooorm/markdown-tm-language#12

@wooorm
Copy link
Member

wooorm commented Nov 11, 2024

Hi!

Have you tested that your changes work with GitHub and some common/default VS Code themes?

Often, themes don’t support all scopes.

@wooorm
Copy link
Member

wooorm commented Nov 11, 2024

I think for GH this will be fine. That’s just looking at markup.heading.

@remcohaszing
Copy link
Member

It might be interesting to know of this breaks any things. But even if this breaks a theme, this change makes sense IMO. Themes can adapt.

@icorbrey
Copy link
Author

Is there a reason H3 - H6 were assigned H2 scopes?

@wooorm
Copy link
Member

wooorm commented Nov 11, 2024

I don’t remember. Hence the question.

It might be interesting to know of this breaks any things. But even if this breaks a theme, this change makes sense IMO. Themes can adapt.

GH doesn’t. There are several workarounds for GH. That’s the most important.

As for VS Code defaults: that’s chicken and egg. At this point this grammar is not too often used. So it is less likely that VS Code changes things. But theoretically I’d agree with you.

Also: this can be used in many places. TextMate. Sublime. Anywhere. Some compatibility between existing things is nice. We can’t expect every software to change.

@icorbrey
Copy link
Author

Okay so after looking a fair amount I can't really see anything that is explicitly relying on H3 - H6 all having H2 scope, but some that already rely on headings having the correct scope that get broken in MDX. For example, Catppuccin Frappe from Catppuccin for VSCode

image

Searching for files not using deeper headings shows that pretty much repo that references H2 scopes and not any deeper ones is just bundling https://github.com/wooorm/markdown-tm-language instead of doing something meaningful with it.

Now I can't speak on GitHub, as I don't know where I'd even begin to look for their implementations let alone if they're source available, but from my surface level lookover I can't see anywhere that they'd be relying on this behavior. They seem to not care what level a heading is wherever I look.

@wooorm
Copy link
Member

wooorm commented Nov 11, 2024

For GH, see my earlier comment #483 (comment). As for source, see e.g. https://github.com/primer/github-syntax-dark/blob/master/lib/github-dark.css.

Thanks for checking!

@icorbrey
Copy link
Author

Ah, I think I missed that comment. Thanks!

This comment has been minimized.

@remcohaszing remcohaszing added 🐛 type/bug This is a problem 📦 area/deps This affects dependencies 👶 semver/patch This is a backwards-compatible fix 💪 phase/solved Post is done labels Dec 22, 2024
@github-actions github-actions bot removed the 🤞 phase/open Post is being triaged manually label Dec 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 area/deps This affects dependencies 💪 phase/solved Post is done 👶 semver/patch This is a backwards-compatible fix 🐛 type/bug This is a problem
Development

No branches or pull requests

3 participants