Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Markdown highlighting issues #1597

Closed
CptPotato opened this issue Jan 28, 2022 · 6 comments · Fixed by #2401
Closed

Markdown highlighting issues #1597

CptPotato opened this issue Jan 28, 2022 · 6 comments · Fixed by #2401
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug

Comments

@CptPotato
Copy link
Contributor

CptPotato commented Jan 28, 2022

These are a few problems I've encountered with the markdown syntax highlighting while updating the everforest theme:

  1. lists appear to use punctuation for the leading -/*/1./etc. I expected them to use markup.list instead.

    • if markup.list isn't used for lists, what is it used for?
  2. there is only a single color for all headings, some themes might expect one per level (Allow separate styles for markup headings #1618):
    image

  3. changing the theme sometimes causes the list color to be different:
    image

    • after reloading the theme a few times, sometimes it's red (left) #e67e80, sometimes it's orange (right) #e69875
    • the corresponding theme keys seem to be punctuation and special
    • edit: looks like this might be the same problem as 1.
@CptPotato CptPotato added the C-bug Category: This is a bug label Jan 28, 2022
@kirawi kirawi added the A-helix-term Area: Helix term improvements label Jan 29, 2022
@archseer
Copy link
Member

  • after reloading the theme a few times, sometimes it's red (left) #e67e80, sometimes it's orange (right) #e69875

Can you reproduce on commit 83bde10 (right before incremental changes?) I had a similar issue with TODO/FIXME comment highlights when working on the incremental branch but I thought I fixed it.

@archseer
Copy link
Member

2. there is only a single color for all headings, some themes might expect one per level:

This might be doable with some changes to the queries. Right now we just match against atx_heading, but we'd need to match (atx_heading (atx_h2_marker) (heading_context @style)) etc

@CptPotato
Copy link
Contributor Author

CptPotato commented Jan 31, 2022

Can you reproduce on commit 83bde10 (right before incremental changes?) I had a similar issue with TODO/FIXME comment highlights when working on the incremental branch but I thought I fixed it.

I'm getting the same behavior on that commit, too. 😕

Edit: Currently markdown's highlights.scm uses the theme key punctuation.special for lists, which I'm not sure if it even exists.
I think this should be markup.list instead.


This might be doable with some changes to the queries. Right now we just match against atx_heading, but we'd need to match (atx_heading (atx_h2_marker) (heading_context @style)) etc

Looks like it's pretty simple. Works as you suggested (except for the missing theme keys, obviously):

(setext_heading (heading_content) @markup.heading.h1 (setext_h1_underline) @markup.heading.marker)
(setext_heading (heading_content) @markup.heading.h2 (setext_h2_underline) @markup.heading.marker)

(atx_heading (atx_h1_marker) @markup.heading.marker (heading_content) @markup.heading.h1)
(atx_heading (atx_h2_marker) @markup.heading.marker (heading_content) @markup.heading.h2)
(atx_heading (atx_h3_marker) @markup.heading.marker (heading_content) @markup.heading.h3)
(atx_heading (atx_h4_marker) @markup.heading.marker (heading_content) @markup.heading.h4)
(atx_heading (atx_h5_marker) @markup.heading.marker (heading_content) @markup.heading.h5)

Though, I'm not sure how overriding styles works, i.e. have "markup.heading" = "#..." replace all the above.

@archseer
Copy link
Member

archseer commented Feb 3, 2022

We always use the most specific style, so if you have markup.heading.h1 defined it will use that, then fall back to markup.heading and finally markup.

If it works, can you go ahead and make a PR? We can use markup.heading.1 since that seems to be what other editors used (I did a quick search for "markup.heading" on github, e.g. https://github.com/ukyo/md-anywhere/blob/5eaf5d49fc005353370fe233fc44c8a2364e2f59/ace/mode/_test/tokens_markdown.json#L1-L29)

@CptPotato
Copy link
Contributor Author

Yes, I'll take a look at it soon!
Thanks for clarifying.

@the-mikedavis
Copy link
Member

I think markup.list would make sense for lists. We may even want to go further and use markup.list.marker for the -/*/1. and markup.list.item for the text of the item.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants