Update: Replace Markdown parser (fixes #125, fixes #186) #188
Conversation
This file contains 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
Future versions of the Markdown processor don't include per-line indent information. Thankfully, we're already calculating it in `getBlockRangeMap()` because we need it to correctly map fix ranges for lines that are under-indented to the left of their opening code fence. This includes the indent in the `RangeMap` for each line and uses that in `adjustBlock()` instead. This uncovered an incorrect assertion in one of the tests, which combined an indented code block, a message on a line with mixed spaces and tabs indentation, and an under-indented final line. The message column was incorrectly asserted as being in the whitespace indentation before the line.
The previous parser, `remark-parse` v7, included a transitive dependency on an npm package with a security vulnerability. Newer versions of `remark-parse` are wrappers around a new underlying parser, `mdast-util-from-markdown`, so we can use that directly. The previous parser also failed to preserve `\r\n` line endings, replacing them all with `\n`. The new parser correctly preserves `\r\n` line endings, finally providing a fix for the failing test case I cherry-picked in the previous commit. The improved behavior also uncovered an incorrect line ending test assertion that this commit corrects. While this change is in theory fully compatible, containing just bug fixes, I'm tagging it `Update:` in case there are compatibility changes in the new parser. This is consistent with #175, which upgraded `remark-parse` v5 to v7 in a semver-minor `Update:` change.
This comment has been minimized.
This comment has been minimized.
btmills
changed the title
nzakas
approved these changes
May 25, 2021
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.
The previous parser,
remark-parsev7, included a transitive dependency on an npm package with a security vulnerability (see #186). Newer versions ofremark-parseare wrappers around a new underlying parser,mdast-util-from-markdown, so we can use that directly.I had thought the upgrade was going to be more difficult because the new parser no longer supplies per-line indent information, but we're already calculating it in
getBlockRangeMap(), so I included the indentation in the each line'sRangeMapand read from there instead.The previous parser also failed to preserve
\r\nline endings, replacing them all with\n. The new parser correctly preserves\r\nline endings, finally providing a fix for the failing test case I cherry-picked from #125. The improved behavior also uncovered an incorrect line ending test assertion that this commit corrects.While this change is in theory fully compatible, containing just bug fixes, I'm tagging it
Update:in case there are compatibility changes in the new parser. This is consistent with #175, which upgradedremark-parsev5 to v7 in a semver-minorUpdate:change.