fix(markdown): remove a fence's indentation before scanning its body - #72
Merged
Merged
Conversation
CommonMark takes up to as many spaces as a fenced code block's opener is indented off each line of the block's content. The scanner handed the body to the embedded language with that indentation still on, so a heredoc terminator written at the fence's indentation, as under a list item, was not one, and the page was reported as invalid syntax with exit status 2. The Rust scanner now scans an indented fence's body over a de-indented MappedBytes copy, keeping the child's offset, and maps every comment and diagnostic span back to the bytes on the page, so fix edits the right bytes. A tab that crosses the opener's column leaves the rest of its width as spaces, which is what `commonmark` 0.31.2 and `markdown-it` 15.0.2 both do. The OCaml reference applies the same rule over its own mapped_bytes. Three fixtures record what CommonMark and bash do with such a block: an indented fence at the top level, one under a list item, and a tab in the indentation.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
A fenced code block indented under a list item is now scanned the way CommonMark reads it.
Before, the page below failed with
unterminated shell heredocand exit status 2, while the same block at the top level passed:scan_markdownaccepts a fence opener indented up to three spaces, butscan_markdown_fencehanded the body to the embedded scanner with that indentation still on, so the child sawEOF.CommonMark 4.5 takes up to as many spaces as the opener is indented off each line of the content.
The same hit Ruby's
<<EOF, whose terminator also has to start its line.MappedBytescopy.The child keeps its offset, so classification is unchanged, and
merge_mappednow takes the copy's origin and maps every comment and diagnostic span back to the bytes on the page.A fence at column 0 takes the old path and makes no copy.
without_fence_indentationbuilds its ownmapped_bytes, and the fence maps spans back throughmapped_span.commonmark0.31.2 andmarkdown-it15.0.2 both read\tEOFunder a three-space opener asEOF.Three fixtures in
hazards.jsonrecord the external facts, each checked againstcommonmark0.31.2,markdown-it15.0.2 and bash:markdown-indented-fence-takes-its-indentation-off-each-line: a line indented more than the opener keeps the rest, and one indented less loses all it has.markdown-fence-under-a-list-item-loses-the-item-indentation: the reported case.markdown-indented-fence-counts-a-tab-to-its-stop:\tEOFunder a three-space opener isEOF, which<<-does not strip.Compatibility and safety
Spans still name bytes on the page, so
fixedits the same places it did.The tests show that a comment in an indented fence is removed where it stands, and that a block comment across fence lines comes back as one span with its inner indentation and is removed as it would be from a C file of its own.
Only fences indented one to three spaces change behaviour, and only for languages where leading whitespace is lexically significant.
The scanner still does not model list items, so a fence under
10.sits four columns in and stays an opaque indented code block, as it was before.Verification
cargo test --manifest-path rust/Cargo.toml --workspace: all passed.cargo clippy --manifest-path rust/Cargo.toml --workspace --all-targets --locked -- -D warnings: clean.cargo fmt --all --manifest-path rust/Cargo.toml -- --check: clean.opam exec -- dune runtest --root ocaml: 30 tests passed, including the newmarkdown-indented-fence.opam exec -- cargo xtask differential: 599 fixtures passed, 599 against a recorded expectation.python3 tools/validate_schemas.py(run in a venv withjsonschema),check_embedded_specs.py,check_directives.py --binary,gen_docs.py --binary --check: all passed.ocommentover this repository: 0 findings.cargo xtask preflight(full): 30 of 30 steps passed.Two pre-existing issues this change does not touch:
# -*- coding: utf-8 -*-on the first line of apythonorrubyfence, OCaml classifies it asencodingand Rust asline.That happens at column 0 as well, because the OCaml child scans its body from offset 0.
a_reflowed_source_is_still_the_same_commentsfailed once on a random seed: Rust////: |\n////reflows into a///doc comment.It reproduces on
origin/main, and it passed on the reruns.Checklist