Plan 182: Code block convention rules (MDS065, MDS066)#372
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds two new Markdown lint rules to close markdownlint-parity gaps (Plan 182): MDS065 code-block-style (MD046) and MDS066 commands-show-output (MD014), including docs, fixtures, and rule registration wiring across the engine and integration tests.
Changes:
- Implement new rules MDS065 and MDS066 with autofix behavior and unit tests.
- Add rule READMEs + good/bad/fixed fixtures and register the rules in the “all rules” and integration suites.
- Update Plan/docs catalogs and markdownlint coverage matrix to reflect the new parity level.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| plan/182_code-block-conventions.md | Marks plan complete; adds notes about MDS065 autofix behavior. |
| PLAN.md | Updates plan 182 status in the main plan index. |
| internal/rules/MDS066-commands-show-output/README.md | Rule documentation for MDS066. |
| internal/rules/MDS066-commands-show-output/good/with-output.md | Fixture: commands + output is allowed. |
| internal/rules/MDS066-commands-show-output/good/no-prompts.md | Fixture: no $ prompts is allowed. |
| internal/rules/MDS066-commands-show-output/bad/default.md | Fixture: commands-only $ block is flagged. |
| internal/rules/MDS066-commands-show-output/fixed/default.md | Fixture: expected autofix output for prompts stripped. |
| internal/rules/MDS065-code-block-style/README.md | Rule documentation for MDS065. |
| internal/rules/MDS065-code-block-style/good/default.md | Fixture: fenced default is allowed. |
| internal/rules/MDS065-code-block-style/good/indented.md | Fixture: indented style allowed under style: indented. |
| internal/rules/MDS065-code-block-style/bad/default.md | Fixture: indented block flagged under default fenced. |
| internal/rules/MDS065-code-block-style/fixed/default.md | Fixture: expected autofix output for indented→fenced conversion. |
| internal/rules/index.md | Adds MDS065/MDS066 entries to the rule catalog. |
| internal/rules/commandsshowoutput/rule.go | Implements MDS066 rule + autofix. |
| internal/rules/commandsshowoutput/rule_test.go | Unit tests for MDS066. |
| internal/rules/codeblockstyle/rule.go | Implements MDS065 rule + autofix + config handling. |
| internal/rules/codeblockstyle/rule_test.go | Unit tests for MDS065 (check/fix/config/generator-skip). |
| internal/rules/all/all.go | Registers the new rules for normal builds. |
| internal/integration/rules_test.go | Registers the new rules for fixture-driven integration tests. |
| docs/research/markdownlint-coverage/README.md | Updates parity counts + maps MD014/MD046 to MDS066/MDS065. |
| docs/background/markdown-linters.md | Updates markdownlint gap counts and references new rules. |
0f61adc to
e691f52
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jeduden
pushed a commit
that referenced
this pull request
May 23, 2026
Address Copilot review feedback on PR #372: MDS066 (commands-show-output): - allLinesArePrompts and stripPrompt now strip leading whitespace before checking for/removing the `$ ` prompt, so fenced blocks nested in list items or blockquotes are detected and fixed correctly (the previous raw-line check missed prompts when any indent preceded them). - The fix preserves the leading whitespace so the rewritten line keeps its container indent. MDS065 (code-block-style): - Autofix now skips indented blocks whose parent is not the document root. Emitting unindented fences for blocks nested in lists or blockquotes broke the parent container; the diagnostic still fires so the author knows to convert manually. - blockInfo grows a topLevel flag and a clarified lastLine doc comment so the fenced/indented semantic difference is documented in one place. Coverage: drop unreachable `line == 0` and `ln <= 0 || ln > len(f.Lines)` guards — fencepos.OpenLine and lint.LineOfOffset both return values in range by construction, so the guards were dead code. Both packages now report 100% statement coverage.
Add two markdownlint-parity rules:
- MDS065 code-block-style (MD046): enforce fenced vs indented
consistently. Setting style ∈ {consistent, fenced, indented},
default fenced. Autofix converts indented to fenced; the
converted block carries the language tag `text` so the result
satisfies MDS011. The reverse direction is not auto-applied
to avoid dropping an existing language tag.
- MDS066 commands-show-output (MD014): flag a fenced block whose
every non-blank line starts with `$ ` and no output is shown.
Autofix strips the prompt.
Both skip include/catalog generated bodies via f.GeneratedRanges.
This closes the last markdown-style gap with markdownlint: the
coverage matrix now reports 48/52 implemented (46 fully, 2
partial), with 4 remaining scheduled in plans 172 and 181.
Address Copilot review feedback on PR #372: MDS066 (commands-show-output): - allLinesArePrompts and stripPrompt now strip leading whitespace before checking for/removing the `$ ` prompt, so fenced blocks nested in list items or blockquotes are detected and fixed correctly (the previous raw-line check missed prompts when any indent preceded them). - The fix preserves the leading whitespace so the rewritten line keeps its container indent. MDS065 (code-block-style): - Autofix now skips indented blocks whose parent is not the document root. Emitting unindented fences for blocks nested in lists or blockquotes broke the parent container; the diagnostic still fires so the author knows to convert manually. - blockInfo grows a topLevel flag and a clarified lastLine doc comment so the fenced/indented semantic difference is documented in one place. Coverage: drop unreachable `line == 0` and `ln <= 0 || ln > len(f.Lines)` guards — fencepos.OpenLine and lint.LineOfOffset both return values in range by construction, so the guards were dead code. Both packages now report 100% statement coverage.
- Split the `f == nil || f.AST == nil` guard in MDS065 and MDS066 Fix: a nil file pointer must return nil rather than dereferencing f.Source. - MDS065 Fix used a closure that linearly scanned `ranges` per source line — O(lines × blocks). Rewrite uses an advancing range pointer for one pass over f.Lines (O(lines + blocks)), matching the source-order invariant collectBlocks already produces. - Cover both new nil-file paths so the packages stay at 100%.
`allLinesArePrompts` and `stripPrompt` relied on `splitLeadingWhitespace`, which only consumes ' ' and '\t', so a fenced block inside a blockquote (raw lines like `> $ ls`) was silently skipped by both Check and Fix. Switch to a segment-offset split. `contentOffsetInLine(f, seg.Start)` returns the byte column where parser-stripped content begins on the raw line — which for goldmark includes blockquote markers (`> `) and list-item indents (` `), the same skipped prefix in both cases. `stripPromptAfter` then peels the further leading whitespace inside the content (covering nested `> $ ls` where the parser only ate `> `), checks for `$ `, and re-emits `prefix + leading + content[2:]`. Locks in three blockquote behaviors per Copilot's request: - prompts-only blockquoted block is flagged - the same block's fix preserves `> ` while stripping `$ ` - a mixed prompt/output blockquoted block is left alone Package coverage stays at 100% (the redundant `contentCol > len(line)` guard on the Check path is dropped per CLAUDE.md — only the Fix-side direct-call helper retains it, with a unit test driving the branch). https://claude.ai/code/session_019R36nGA3a3S87SJV56Kj6m
Previous indented→fenced rewrite always emitted "```text" / "```", which would prematurely close the new block whenever an indented block contained a content line of ``` (or longer) — common when the doc is teaching Markdown itself. `fenceFor(lines, first, last)` now scans the indent-stripped content for the longest run of backticks at line start, and returns a backtick string one longer (minimum 3). The per-range fence is precomputed during the indented-range collection so the rewrite loop still runs in O(lines + blocks). Tests lock in: - a ```-containing block gets a 4-backtick fence - a ````-containing block gets a 5-backtick fence Package coverage stays at 100%. README documents the sizing rule under Autofix. https://claude.ai/code/session_019R36nGA3a3S87SJV56Kj6m
9124e5d to
6fb9e6c
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Adds two markdownlint-parity rules — closing the last code-block gap (plan 182):
code-block-style(markdownlint MD046): enforces fenced vs indented code blocks consistently. Settingstyle ∈ {consistent, fenced, indented}, defaultfenced. Autofix converts indented to fenced; the converted block carries the language tagtextso the result satisfies MDS011 (fenced-code-language). The reverse direction is not auto-applied — it would drop any existing language tag. Nested blocks (inside a list item or blockquote) are diagnosed but not auto-converted, since emitting unindented fences would break the container.commands-show-output(markdownlint MD014): flags fenced code blocks whose every non-blank line starts with$and no output is shown. Autofix strips the prompt while preserving any leading container indent so the snippet stays copy-paste-friendly.Both rules skip
<?include?>and<?catalog?>generated bodies viaf.GeneratedRanges.Together with plan 181 (table structure, landed separately on main), the coverage matrix now reports 51/52 markdownlint rules implemented (49 fully, 2 partial); MD054 is the only outstanding rule, scheduled in plan 172.
Notable design choice
MDS065's autofix tags converted blocks
text(rather than leaving the info string empty) so the post-fix output satisfies MDS011. Users are expected to refine the tag to the real language after fixing. Documented in the rule README and the plan's notes section.Test plan
go test ./...passesgo tool golangci-lint runreports no issuesmdsmith check .clean$prompts stripped, mixed-output block left alone, nested blocks diagnosed without breaking containerhttps://claude.ai/code/session_017wNDiEKuCquq7rigWLZeGv
Generated by Claude Code