-
Notifications
You must be signed in to change notification settings - Fork 1
Add plans 87 and 88: GitHub Alerts and TOC directive rules #139
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
Merged
Merged
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
28cc348
plan: add 87 (GitHub Alerts) and 88 (TOC migration)
claude c878ad9
plan 88: clarify catalog use-case, fix error message
claude 08665c7
plan 88: conform error style, handle [TOC] link-ref ambiguity
claude eb3513a
plan 88: correct failure description and error message style
claude 0031023
plan 88: backtick <?catalog?> in example error message
claude 7c7a4d0
plan 88: make error message template explicit for all tokens
claude b7b5f00
plans 87, 88: tighten failure-mode and CommonMark wording
claude 417a0cb
ci: bump merge-queue-action to v0.5.0
claude File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,153 @@ | ||
| --- | ||
| id: 87 | ||
| title: Flavor validation for GitHub Alerts | ||
| status: "🔲" | ||
| summary: >- | ||
| Extend MDS034 to detect GitHub Alerts syntax | ||
| (`> [!NOTE]` blockquote prefix) as a GFM-only | ||
| feature with auto-fix that demotes the alert to | ||
| a plain blockquote when the target flavor does | ||
| not support it. | ||
| --- | ||
| # Flavor validation for GitHub Alerts | ||
|
|
||
| Extends [plan 86](86_markdown-flavor-validation.md) | ||
| (MDS034, flavor validation). Add one feature — | ||
| GitHub Alerts — to the MDS034 feature enum. | ||
|
|
||
| Depends on: plan 86 lands first (provides the | ||
| dual parser, feature enum, fix pipeline). | ||
|
|
||
| ## Goal | ||
|
|
||
| MDS034 flags `> [!NOTE]`-style alert blockquotes | ||
| when the target flavor is `commonmark` or | ||
| `goldmark`. `gfm` accepts them. Auto-fix demotes | ||
| the alert marker so the blockquote still renders | ||
| on non-GFM renderers. | ||
|
|
||
| ## Context | ||
|
|
||
| GitHub added Alerts to GFM in December 2023 | ||
| (see the `github.blog` changelog entry for | ||
| `new-syntax-for-alerts-on-github`). Five tokens | ||
| are recognized: `[!NOTE]`, `[!TIP]`, | ||
| `[!IMPORTANT]`, `[!WARNING]`, `[!CAUTION]`. | ||
| Obsidian callouts use the same prefix and accept | ||
| extra tokens, but only these five are standard | ||
| GFM. | ||
|
|
||
| On CommonMark / goldmark-default, the marker | ||
| renders as literal text inside a blockquote: | ||
|
|
||
| ```markdown | ||
| > [!NOTE] | ||
| > Something to remember. | ||
| ``` | ||
|
|
||
| becomes a blockquote whose first line is the | ||
| literal string `[!NOTE]`. The author intended a | ||
| styled callout; the reader sees unstyled text. | ||
| This is a silent failure. | ||
|
jeduden marked this conversation as resolved.
Outdated
|
||
|
|
||
| ### Why not a generic container rule | ||
|
|
||
| The research spike evaluated four other | ||
| container syntaxes (Pandoc `:::` fenced divs, | ||
| MyST `:::{note}`, markdown-it-container, MkDocs | ||
| `!!! note`). None are mutually compatible and no | ||
| linter in the comparison covers them. GitHub | ||
| Alerts are the only variant with a standardized | ||
| spec, broad renderer support, and a clear | ||
| failure mode — so this plan covers them alone. | ||
|
|
||
| ## Design | ||
|
|
||
| ### Detection | ||
|
|
||
| GitHub Alerts need no new goldmark extension. | ||
| The syntax is a plain Blockquote. Its first | ||
| paragraph text must match | ||
| `^\[!(NOTE|TIP|IMPORTANT|WARNING|CAUTION)\]\s*$` | ||
| (case-sensitive per GFM). | ||
|
|
||
| Detection is an AST walk over `ast.Blockquote` | ||
| nodes on the dual parser's tree. The same walk | ||
| pattern the other 12 features use. | ||
|
|
||
| ### Configuration | ||
|
|
||
| No new settings. GitHub Alerts join the existing | ||
| feature enum in | ||
| `internal/rules/markdownflavor/features.go` as | ||
| feature 13 (`GitHubAlerts`). Flavor support: | ||
|
|
||
| | Flavor | GitHub Alerts | | ||
| |------------|---------------| | ||
| | commonmark | unsupported | | ||
| | gfm | supported | | ||
| | goldmark | unsupported | | ||
|
|
||
|
jeduden marked this conversation as resolved.
|
||
| ### Auto-fix | ||
|
|
||
| Remove the `[!TOKEN]` marker line, keeping the | ||
| rest of the blockquote intact: | ||
|
|
||
| ```markdown | ||
| > [!NOTE] > Something to | ||
| > Something to → > remember. | ||
| > remember. | ||
| ``` | ||
|
|
||
| If the alert marker is the only line in the | ||
| blockquote, remove the whole blockquote. The | ||
| marker line has no meaningful content once the | ||
| token is gone. | ||
|
|
||
| ### Error message | ||
|
|
||
| `github alerts are not supported by {flavor}` | ||
|
|
||
| Severity: `warning`, matching the other MDS034 | ||
| features. | ||
|
|
||
| ## Tasks | ||
|
|
||
| 1. Add `GitHubAlerts` to the feature enum in | ||
| `internal/rules/markdownflavor/features.go` | ||
| 2. Add flavor support table entry: supported in | ||
| `gfm`, unsupported in `commonmark` and | ||
| `goldmark` | ||
| 3. Implement an AST detector that walks | ||
| `ast.Blockquote` nodes and matches the five | ||
| GFM tokens on the first paragraph child | ||
| 4. Implement the fix: strip the marker line; | ||
| drop the blockquote if empty afterward | ||
| 5. Add unit tests: each of the five tokens, | ||
| lower-case tokens (should not match), mixed | ||
| content after the marker, marker as the only | ||
| line | ||
| 6. Add good/bad fixtures under | ||
| `internal/rules/MDS034-markdown-flavor/alerts/` | ||
| 7. Update the MDS034 README to list GitHub | ||
| Alerts as the 13th feature | ||
|
|
||
| ## Acceptance Criteria | ||
|
|
||
| - [ ] `flavor: commonmark` flags all five alert | ||
| tokens | ||
| - [ ] `flavor: goldmark` flags all five alert | ||
| tokens | ||
| - [ ] `flavor: gfm` accepts all five tokens | ||
| - [ ] `mdsmith fix` removes the marker line, | ||
| preserves remaining blockquote content | ||
| - [ ] `mdsmith fix` removes the whole blockquote | ||
| when the marker was its only line | ||
| - [ ] Lower-case or unknown tokens (e.g. | ||
| `[!note]`, `[!INFO]`) produce no | ||
| diagnostic — they are ordinary blockquote | ||
| text | ||
| - [ ] Nested blockquotes are checked recursively | ||
| - [ ] All tests pass: `go test ./...` | ||
| - [ ] `go tool golangci-lint run` reports no | ||
| issues | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,235 @@ | ||
| --- | ||
| id: 88 | ||
| title: TOC directive migration aid | ||
| status: "🔲" | ||
| summary: >- | ||
| New rule MDS035 that flags renderer-specific | ||
| table-of-contents directives (`[TOC]`, | ||
| `[[_TOC_]]`, `[[toc]]`, `${toc}`) which render | ||
| as literal text on CommonMark / goldmark | ||
| instead of expanding into a TOC. The | ||
| diagnostic points authors at mdsmith's | ||
| `<?catalog?>` directive for the file-index use | ||
| case; heading-level TOCs have no direct | ||
| mdsmith equivalent. | ||
| --- | ||
| # TOC directive migration aid | ||
|
|
||
| ## Goal | ||
|
|
||
| Catch renderer-specific TOC directives that do | ||
| not expand into a TOC on CommonMark or | ||
| goldmark. The diagnostic tells authors which | ||
| use case has a mdsmith equivalent and which | ||
| does not. | ||
|
|
||
| ## Context | ||
|
|
||
| Four TOC directive variants appear in the wild: | ||
|
|
||
| - `[TOC]` — Python-Markdown, MultiMarkdown, | ||
| Pandoc (with `--toc`) | ||
| - `[[_TOC_]]` — GitLab Flavored Markdown, | ||
| Azure DevOps | ||
| - `[[toc]]` — markdown-it-toc-done-right, | ||
| VitePress | ||
| - `${toc}` — some VitePress configurations | ||
|
|
||
| None are part of CommonMark, GFM, or goldmark. | ||
| On those renderers the directive does not | ||
| expand into a TOC; it renders as literal text. | ||
|
jeduden marked this conversation as resolved.
Outdated
|
||
| The exact output depends on the pattern: | ||
|
|
||
| - `[TOC]` without a matching link reference | ||
| definition renders as the literal string | ||
| `[TOC]` (goldmark emits a "no matching link | ||
| reference" fallback, which is verbatim text) | ||
| - `[[_TOC_]]` renders as `[[_TOC_]]` inside a | ||
| paragraph | ||
| - `[[toc]]` renders as `[[toc]]` inside a | ||
| paragraph | ||
| - `${toc}` renders as `${toc}` inside a | ||
| paragraph | ||
|
|
||
| The author intended a generated table of | ||
| contents; the reader sees the directive token | ||
| instead. This is a visible failure, not a | ||
| silent one, but it is still a failure worth | ||
| catching at lint time. | ||
|
|
||
| ### Heading TOC vs file index | ||
|
|
||
| The flagged directives and | ||
| [`<?catalog?>`][catalog] solve different | ||
| problems: | ||
|
|
||
| | Directive | Generates | Input | | ||
| |----------------|-------------------------------------------|--------------------| | ||
| | `[TOC]` et al. | Table of **headings in the current file** | Current doc | | ||
| | `<?catalog?>` | Table of **other files** matching a glob | Glob + frontmatter | | ||
|
|
||
|
jeduden marked this conversation as resolved.
|
||
| [catalog]: ../internal/rules/MDS019-catalog/README.md | ||
|
|
||
| `<?catalog?>` is the right replacement only | ||
| when a directive is used on an index page to | ||
| list sibling or child documents (e.g. a wiki | ||
| homepage with `[[_TOC_]]` listing all pages in | ||
| the space). For in-document heading TOCs — the | ||
| more common case — mdsmith has no built-in | ||
| generator; the author must either drop the | ||
| directive or maintain a manual list. | ||
|
|
||
| ### Why this rule, not MDS034 | ||
|
|
||
| MDS034 ([plan 86](86_markdown-flavor-validation.md)) | ||
| validates syntax support against a declared | ||
| flavor. TOC directives are not "flavor features" | ||
| — they are per-renderer conventions with no | ||
| canonical spec and no fix path that applies to | ||
| every call site. A dedicated opt-in rule with a | ||
| diagnostic tailored to the use-case distinction | ||
| above is a better fit than folding them into | ||
| MDS034's fix pipeline. | ||
|
|
||
| ### Scope | ||
|
|
||
| Flag only the four directives above. Do not try | ||
| to auto-generate a `<?catalog?>` block — the | ||
| right glob and front-matter fields depend on the | ||
| project and are not knowable from the TOC call | ||
| site. The diagnostic is informational and names | ||
| both the file-index case (points to MDS019) and | ||
| the heading-TOC case (no equivalent). | ||
|
|
||
| ## Design | ||
|
|
||
| ### Detection | ||
|
|
||
| Line-level regex on the raw source, scoped to | ||
| paragraph nodes (skip code blocks, HTML blocks, | ||
| and inline code spans). Four patterns: | ||
|
|
||
| - `^\[TOC\]\s*$` | ||
| - `^\[\[_TOC_\]\]\s*$` | ||
| - `^\[\[toc\]\]\s*$` | ||
| - `^\$\{toc\}\s*$` | ||
|
|
||
| Goldmark parses `[TOC]` as a link reference node | ||
| and `[[_TOC_]]` / `[[toc]]` as text inside a | ||
| paragraph. AST detection would require | ||
| per-variant walkers; raw-line regex is simpler | ||
| and avoids false positives by restricting the | ||
| match to paragraph-only regions. | ||
|
|
||
| #### Link reference exception for `[TOC]` | ||
|
|
||
| `[TOC]` is syntactically a valid CommonMark | ||
| shortcut reference link. If the document | ||
| contains a matching link reference definition | ||
| (`[TOC]: <url>`), `[TOC]` resolves to a | ||
| legitimate link and must not be flagged. | ||
|
|
||
| Before emitting a diagnostic for the `[TOC]` | ||
| pattern, consult the goldmark parser context's | ||
| link reference map for a definition with the | ||
| label `TOC` (case-insensitive, per the | ||
| [CommonMark matching rules][cm-refs]). If one | ||
| exists, suppress the diagnostic. | ||
|
|
||
| [cm-refs]: https://spec.commonmark.org/0.31.2/#matches | ||
|
|
||
| The other three patterns do not have this | ||
| ambiguity: `[[_TOC_]]`, `[[toc]]`, and `${toc}` | ||
| do not form valid link references in CommonMark | ||
| and always render as literal text in a | ||
| paragraph. No exception handling is needed for | ||
| them. | ||
|
|
||
| ### Configuration | ||
|
|
||
| Rule `toc-directive`, category `meta`, disabled | ||
| by default (opt-in) — consistent with MDS034's | ||
| opt-in posture. No settings. | ||
|
|
||
| ### Error message | ||
|
|
||
| Templated on the matched directive token, so | ||
| each variant produces its own message. Given a | ||
| matched token `{directive}`: | ||
|
|
||
| ```text | ||
| unsupported TOC directive `{directive}`; mdsmith has no heading TOC equivalent; use `<?catalog?>` for file indexes (MDS019) | ||
| ``` | ||
|
|
||
| Example messages the rule would emit for each | ||
| of the four patterns: | ||
|
|
||
| ```text | ||
| unsupported TOC directive `[TOC]`; mdsmith has no heading TOC equivalent; use `<?catalog?>` for file indexes (MDS019) | ||
| unsupported TOC directive `[[_TOC_]]`; mdsmith has no heading TOC equivalent; use `<?catalog?>` for file indexes (MDS019) | ||
| unsupported TOC directive `[[toc]]`; mdsmith has no heading TOC equivalent; use `<?catalog?>` for file indexes (MDS019) | ||
| unsupported TOC directive `${toc}`; mdsmith has no heading TOC equivalent; use `<?catalog?>` for file indexes (MDS019) | ||
| ``` | ||
|
jeduden marked this conversation as resolved.
|
||
|
|
||
| The leading word is lowercase. No trailing | ||
| punctuation, per [CLAUDE.md](../CLAUDE.md). | ||
| Both the matched token and the replacement | ||
| `<?catalog?>` are backticked so they read as | ||
| quoted directives, not prose. | ||
|
|
||
| Severity: `warning`. | ||
|
|
||
| ### No auto-fix | ||
|
|
||
| The rule is detection-only. Whether the right | ||
| replacement is `<?catalog?>`, a manually | ||
| maintained list, or deletion depends on intent | ||
| that is not recoverable from the directive | ||
| alone. | ||
|
|
||
| ## Tasks | ||
|
|
||
| 1. Create `internal/rules/MDS035-toc-directive/` | ||
| with `rule.go`, `README.md` | ||
| 2. Implement paragraph-scoped line scanning for | ||
| the four directive patterns | ||
| 3. For the `[TOC]` pattern, consult the goldmark | ||
| parser context's link reference definition | ||
| map; suppress the diagnostic when a label | ||
| `TOC` (case-insensitive) is defined | ||
| 4. Implement `rule.Defaultable` with | ||
| `EnabledByDefault` returning `false` | ||
| 5. Register as MDS035 in category `meta` | ||
| 6. Add good/bad fixtures with front-matter | ||
| specifying the expected diagnostics, including | ||
| a good fixture that has `[TOC]: https://x` as | ||
| a reference definition alongside a `[TOC]` | ||
| line | ||
| 7. Document the rule in the flavor comparison | ||
| table in | ||
| [docs/background/markdown-linters.md](../docs/background/markdown-linters.md) | ||
|
|
||
| ## Acceptance Criteria | ||
|
|
||
| - [ ] `[TOC]` on its own line produces a | ||
| diagnostic that names both the heading-TOC | ||
| gap and the `<?catalog?>` file-index | ||
| alternative | ||
| - [ ] `[[_TOC_]]` on its own line produces the | ||
| same diagnostic | ||
| - [ ] `[[toc]]` on its own line produces the | ||
| same diagnostic | ||
| - [ ] `${toc}` on its own line produces the | ||
| same diagnostic | ||
| - [ ] `[TOC]` inside a fenced code block | ||
| produces no diagnostic | ||
| - [ ] `[TOC]` inside an inline code span | ||
| produces no diagnostic | ||
| - [ ] `[TOC]` used as legitimate link text | ||
| (with a matching `[TOC]: url` definition) | ||
| produces no diagnostic | ||
|
jeduden marked this conversation as resolved.
|
||
| - [ ] Rule is disabled by default (opt-in) | ||
| - [ ] No auto-fix is applied | ||
| - [ ] All tests pass: `go test ./...` | ||
| - [ ] `go tool golangci-lint run` reports no | ||
| issues | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.