-
Notifications
You must be signed in to change notification settings - Fork 86
feat: support Math #617
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
feat: support Math #617
Changes from all commits
fb34a6d
cd1193e
53a3d96
6d2d38e
0816846
f4808f1
1de5acc
2b9e418
2158620
33896ea
8e6bf91
7a0ab0c
728baca
49cb5b7
d678bb7
12f1766
23304cb
f429df4
6759ea1
bfb129b
c7c38ed
29ba15a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
|
|
||
| /** | ||
| * @import { Heading, Paragraph, TableCell, Html, Image, ImageReference, InlineCode, LinkReference } from "mdast"; | ||
| * @import { InlineMath } from "mdast-util-math"; | ||
| * @import { MarkdownRuleDefinition } from "../types.js"; | ||
| * @typedef {"reversedSyntax"} NoReversedMediaSyntaxMessageIds | ||
| * @typedef {[]} NoReversedMediaSyntaxOptions | ||
|
|
@@ -65,12 +66,12 @@ export default { | |
| nodeStartOffset = node.position.start.offset; | ||
| }, | ||
|
|
||
| ":matches(heading, paragraph, tableCell) :matches(html, image, imageReference, inlineCode, linkReference)"( | ||
| /** @type {Html | Image | ImageReference | InlineCode | LinkReference} */ node, | ||
| ":matches(heading, paragraph, tableCell) :matches(html, image, imageReference, inlineCode, linkReference, inlineMath)"( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR introduces two node types: However, the |
||
| /** @type {Html | Image | ImageReference | InlineCode | LinkReference | InlineMath} */ node, | ||
| ) { | ||
| const [startOffset, endOffset] = sourceCode.getRange(node); | ||
|
|
||
| // Mask the content of `html`, `image`, `imageReference`, `inlineCode`, and `linkReference` nodes with whitespaces. | ||
| // Mask the content of `html`, `image`, `imageReference`, `inlineCode`, `linkReference`, and `inlineMath` nodes with whitespaces. | ||
| for (let i = startOffset; i < endOffset; i++) { | ||
| buffer[i - nodeStartOffset] = " "; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,18 +16,7 @@ import assert from "node:assert"; | |
|
|
||
| describe("MarkdownLanguage", () => { | ||
| describe("validateLanguageOptions()", () => { | ||
| it("should throw an error if `frontmatter` is not `false`, `'yaml'`, `'toml'`, or `'json'`", () => { | ||
| const language = new MarkdownLanguage(); | ||
|
|
||
| assert.throws(() => { | ||
| language.validateLanguageOptions({ frontmatter: "invalid" }); | ||
| }, /Invalid language option value/u); | ||
| assert.throws(() => { | ||
| language.validateLanguageOptions({ frontmatter: 123 }); | ||
| }, /Invalid language option value/u); | ||
| }); | ||
|
|
||
| it("should not throw an error when `frontmatter` is not provided", () => { | ||
| it("should not throw an error when `frontmatter` or `math` is not provided", () => { | ||
| const language = new MarkdownLanguage(); | ||
|
|
||
| assert.doesNotThrow(() => { | ||
|
|
@@ -38,13 +27,39 @@ describe("MarkdownLanguage", () => { | |
| }); | ||
| }); | ||
|
|
||
| it("should not throw an error when `frontmatter` is not provided and other keys are present", () => { | ||
| it("should not throw an error when `frontmatter` or `math` is not provided and other keys are present", () => { | ||
| const language = new MarkdownLanguage(); | ||
| assert.doesNotThrow(() => { | ||
| language.validateLanguageOptions({ foo: "bar" }); | ||
| }); | ||
| }); | ||
|
|
||
| // Validation tests for the `frontmatter` option | ||
| it("should throw an error if `frontmatter` is not `false`, `'yaml'`, `'toml'`, or `'json'`", () => { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous test logic for In summary, this diff here mainly restructures the test suites for improved clarity. |
||
| const language = new MarkdownLanguage(); | ||
|
|
||
| assert.throws( | ||
| () => { | ||
| language.validateLanguageOptions({ | ||
| frontmatter: "invalid", | ||
| }); | ||
| }, | ||
| { | ||
| message: | ||
| 'Invalid language option value `invalid` for frontmatter. Expected one of `false`, `"yaml"`, `"toml"`, or `"json"`.', | ||
| }, | ||
| ); | ||
| assert.throws( | ||
| () => { | ||
| language.validateLanguageOptions({ frontmatter: 123 }); | ||
| }, | ||
| { | ||
| message: | ||
| 'Invalid language option value `123` for frontmatter. Expected one of `false`, `"yaml"`, `"toml"`, or `"json"`.', | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| it("should not throw an error when `frontmatter` has a correct value in commonmark mode", () => { | ||
| const language = new MarkdownLanguage({ mode: "commonmark" }); | ||
|
|
||
|
|
@@ -78,6 +93,52 @@ describe("MarkdownLanguage", () => { | |
| language.validateLanguageOptions({ frontmatter: "json" }); | ||
| }); | ||
| }); | ||
|
|
||
| // Validation tests for the `math` option | ||
| it("should throw an error if `math` is not `true` or `false`", () => { | ||
| const language = new MarkdownLanguage(); | ||
|
|
||
| assert.throws( | ||
| () => { | ||
| language.validateLanguageOptions({ math: "invalid" }); | ||
| }, | ||
| { | ||
| message: | ||
| "Invalid language option value `invalid` for math. Expected a boolean.", | ||
| }, | ||
| ); | ||
| assert.throws( | ||
| () => { | ||
| language.validateLanguageOptions({ math: 123 }); | ||
| }, | ||
| { | ||
| message: | ||
| "Invalid language option value `123` for math. Expected a boolean.", | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| it("should not throw an error when `math` has a correct value in commonmark mode", () => { | ||
| const language = new MarkdownLanguage({ mode: "commonmark" }); | ||
|
|
||
| assert.doesNotThrow(() => { | ||
| language.validateLanguageOptions({ math: true }); | ||
| }); | ||
| assert.doesNotThrow(() => { | ||
| language.validateLanguageOptions({ math: false }); | ||
| }); | ||
| }); | ||
|
|
||
| it("should not throw an error when `math` has a correct value in gfm mode", () => { | ||
| const language = new MarkdownLanguage({ mode: "gfm" }); | ||
|
|
||
| assert.doesNotThrow(() => { | ||
| language.validateLanguageOptions({ math: true }); | ||
| }); | ||
| assert.doesNotThrow(() => { | ||
| language.validateLanguageOptions({ math: false }); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe("parse()", () => { | ||
|
|
@@ -270,6 +331,77 @@ describe("MarkdownLanguage", () => { | |
| assert.strictEqual(result.ast.children[1].type, "heading"); | ||
| assert.strictEqual(result.ast.children[2].type, "paragraph"); | ||
| }); | ||
|
|
||
| it("should not parse math by default", () => { | ||
| const language = new MarkdownLanguage(); | ||
| const result = language.parse({ | ||
| body: "Inline math: $E=mc^2$\n\nBlock math:\n\n$$\nE=mc^2\n$$", | ||
| path: "test.md", | ||
| }); | ||
|
|
||
| assert.strictEqual(result.ok, true); | ||
| assert.strictEqual(result.ast.type, "root"); | ||
| assert.strictEqual(result.ast.children[0].type, "paragraph"); | ||
| assert.strictEqual(result.ast.children[0].children[0].type, "text"); | ||
| assert.strictEqual(result.ast.children[1].type, "paragraph"); | ||
| assert.strictEqual(result.ast.children[1].children[0].type, "text"); | ||
| assert.strictEqual(result.ast.children[2].type, "paragraph"); | ||
| assert.strictEqual(result.ast.children[2].children[0].type, "text"); | ||
| }); | ||
|
|
||
| it("should parse math in commonmark mode when `math: true` is set", () => { | ||
| const language = new MarkdownLanguage({ mode: "commonmark" }); | ||
| const result = language.parse( | ||
| { | ||
| body: "Inline math: $E=mc^2$\n\nBlock math:\n\n$$\nE=mc^2\n$$", | ||
| path: "test.md", | ||
| }, | ||
| { | ||
| languageOptions: { | ||
| math: true, | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| assert.strictEqual(result.ok, true); | ||
| assert.strictEqual(result.ast.type, "root"); | ||
| assert.strictEqual(result.ast.children[0].type, "paragraph"); | ||
| assert.strictEqual(result.ast.children[0].children[0].type, "text"); | ||
| assert.strictEqual( | ||
| result.ast.children[0].children[1].type, | ||
| "inlineMath", | ||
| ); | ||
| assert.strictEqual(result.ast.children[1].type, "paragraph"); | ||
| assert.strictEqual(result.ast.children[1].children[0].type, "text"); | ||
| assert.strictEqual(result.ast.children[2].type, "math"); | ||
| }); | ||
|
|
||
| it("should parse math in gfm mode when `math: true` is set", () => { | ||
| const language = new MarkdownLanguage({ mode: "gfm" }); | ||
| const result = language.parse( | ||
| { | ||
| body: "Inline math: $E=mc^2$\n\nBlock math:\n\n$$\nE=mc^2\n$$", | ||
| path: "test.md", | ||
| }, | ||
| { | ||
| languageOptions: { | ||
| math: true, | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| assert.strictEqual(result.ok, true); | ||
| assert.strictEqual(result.ast.type, "root"); | ||
| assert.strictEqual(result.ast.children[0].type, "paragraph"); | ||
| assert.strictEqual(result.ast.children[0].children[0].type, "text"); | ||
| assert.strictEqual( | ||
| result.ast.children[0].children[1].type, | ||
| "inlineMath", | ||
| ); | ||
| assert.strictEqual(result.ast.children[1].type, "paragraph"); | ||
| assert.strictEqual(result.ast.children[1].children[0].type, "text"); | ||
| assert.strictEqual(result.ast.children[2].type, "math"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("createSourceCode()", () => { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question:
Are the
undefinedchecks (e.g.mathOption !== undefined,frontmatterOption !== undefined) and the optional chaining (languageOptions?.math,languageOptions?.frontmatter) still necessary? (I initially implemented it to check forundefined.)markdown/src/language/markdown-language.js
Line 171 in 3a8169c
markdown/src/language/markdown-language.js
Line 162 in 3a8169c
Initially, the motivation for introducing
frontmatterOption !== undefinedwas in #328. At that time, theREADME.mdstated that the minimum compatible ESLint version was any v9 release, so it made sense.Later, we discovered that rules used
defaultOptions, and we raised the minimum ESLint version to v9.15.0 (ref: #537).The issue is that
defaultLanguageOptionswas introduced in ESLint v9.13.0. Since we now expect users to be running ESLint v9.15.0 or later, thefrontmatterandmathoptions can't beundefined.markdown/src/language/markdown-language.js
Lines 134 to 136 in 3a8169c
However, currently there are no
peerDependenciesor other restrictions on the language plugins, so checking forundefinedstill seems sensible: some users might not depend on the rule and could only be usingMarkdownSourceCodeorMarkdownLanguagefeatures with ESLint versions older than v9.13.0.To summarize, for safety, would it be better to keep the
undefinedchecks and optional chaining, or is it safe to remove them? If opening a separate issue would help, I'm happy to do that and work on it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that we should keep the optional chaining on
languageOptionsandundefinedchecks until the next major version.