yzhang-gh / vscode-markdown

Markdown All in One
https://marketplace.visualstudio.com/items?itemName=yzhang.markdown-all-in-one
MIT License
2.93k stars 325 forks source link

Math expressions are recognized as setext headings #911

Open randallwc opened 3 years ago

randallwc commented 3 years ago

Problem

'=' causes line above to be a header in TOC when in math mode '-' doesn't show up in TOC when used as a secondary header syntax

How to reproduce

https://www.markdownguide.org/basic-syntax/

here is the TOC created from a simple equation

- [one](#one)
- [x](#x)
- [two](#two)
- [three](#three)

# one

$$
x
=
y
$$

# two

$$
x
-
y
$$

three
=

four
-

what it should be is

- [one](#one)
- [two](#two)
- [three](#three)
  - [four](#four)

# one

$$
x
=
y
$$

# two

$$
x
-
y
$$

three
=

four
-

Error message in the console

(node:18136) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
[Extension Host] No character metrics for '' in style 'Main-Regular' and mode 'text'
DevTools failed to load SourceMap: Could not load content for https://ticino.blob.core.windows.net/sourcemaps/f30a9b73e8ffc278e71575118b6bf568f04587c8/core/vs/workbench/workbench.desktop.main.js.map: Load canceled due to load timeout
yzhang-gh commented 3 years ago

This looks to be a regression because we chose to use a markdown-it engine to parse the document, which is principled in theory. The problem is Math is not in the CommonMark spec.

cc @Lemmingh.

randallwc commented 3 years ago

This post is essentially two issues. I probably should have separated them from the start.

First, is the math mode syntax being registered as a heading (which is a bad thing) but not displayed as one in preview (which is a good thing).

Second, is that the syntax of using a minus sign below any word should make it a heading 2 which it does in the preview but it doesn't add it to the TOC.

I feel like the first issue might be a regression but I think the second issue is about TOC. But I am primarily focused on the first issue because this weird syntax of - and = is kinda not useful in markdown but others might use it.

yzhang-gh commented 3 years ago

Second, is that the syntax of using a minus sign below any word should make it a heading 2 which it does in the preview but it doesn't add it to the TOC.

I'm using the latest dev build and it works "as expected"

Lemmingh commented 3 years ago

A very old problem, for years.

For now, we can move all TOC operations onto mdEngine.

A final fix won't land until we finish our language service.


For TOC operations in our product, we are mimicking the default configuration of VS Code's markdown-language-features: Load markdown-it-front-matter.

So other syntax, including math, is not considered.

As mentioned above, the fix could be loading markdown-it-front-matter in mdEngine, which already loads other markdown-it plugins, in the same way:

https://github.com/microsoft/vscode/blob/5d80c30e5b6ce8b2f5336ed55ad043490b0b818f/extensions/markdown-language-features/src/markdownEngine.ts#L96-L109

Then adapt getAllTocEntry().

Lemmingh commented 3 years ago

In short, there are a few mismatches between components.

yzhang-gh commented 3 years ago

In the long term, I think we will solve this (a certain level of compatibility is needed). But there is no easy fix given the current state. In contrast, we can simply avoid this issue if we don't write equations like that.

Lemmingh commented 3 years ago

Yeah. Just had a deeper look. Even a short term fix could take months.

Not only is the change big, but also the wait chain is long.


Major work items:

  1. Introduce utility types for handling async tasks.
  2. Migrate completion to a new model. (#905)
  3. Clean up internal data structures of toc.
  4. Migrate toc to a completely async model.
  5. Move heading recognition onto mdEngine. (this)

Such a long list is just caused by the asynchronous nature of mdEngine. 😂