Fix: Indent multiline fixes (fixes #120) #124
Conversation
If the replacement text of an autofix contains newlines, all lines but the first in the replacement text will unexpectedly have 0 indentation if a code block is indented. I guess not only the _range_ of fixes but also the _text_ of fixes need to be adjusted. If the code block is placed within a blockquote, a `>` character also needs to be inserted. This issue was found while investigating lydell/eslint-plugin-simple-import-sort#22. That plugin several lines of imports in one go. This means that most of the imports will have too little indentation after autofix if the code block is indented (such as when placed in a list).
| " ```js", | ||
| " console.log(\"Hello, world!\")", | ||
| " console.log(\"Hello, \\", | ||
| " world!\")", |
There was a problem hiding this comment.
I’m not 100% sure how leading whitespace works in weird code blocks like this, but didn’t we cause one more space to appear in the JS string here?
There was a problem hiding this comment.
I just pushed 80edd59 that adds comments explaining why this occurs for the benefit of future readers of these tests.
When a line inside a fenced code block is indented less than the opening fence, Markdown treats it as having 0 indent, even though its indent in the source is negative. The parser doesn't provide any information to distinguish lines that begin at the same level as the opening fence from those that are underindented. Whenever one of those lines gets autofixed, it gets shifted to align with the opening fence. Lines that that have positive indentation inside the code fence retain their original indentation because those are representable.
There was a problem hiding this comment.
I checked this case in https://spec.commonmark.org/dingus/ and the actual and expected code snippets produce the same rendered JS, with the same amount of spaces between "Hello," and "world!".
| ">", | ||
| "> > ```js", | ||
| "> > console.log(\"Hello, \\", | ||
| "> > world!\")", |
There was a problem hiding this comment.
Same thing here? Is this 1–2 too many spaces?
There was a problem hiding this comment.
Yep, same cause. I added comments in 80edd59 and explained further in #124 (comment).
There was a problem hiding this comment.
I checked this case in https://spec.commonmark.org/dingus/ and the actual and expected code snippets produce the same rendered JS, with the same amount of spaces between "Hello," and "world!".
|
Btw, I’d be fine not bothering with the two potential edge cases above. It’s better to fix the bug in the common case, and leave the very uncommon case for later. |
|
Looks like this is ready to go, then? |
|
As mentioned in the inline comments above, after checking on https://spec.commonmark.org/dingus/ there seems to be no problem with the current implementation! 🎉 |
Thanks to @lydell for discovering this bug and writing the tests to reproduce it in #120.
If I lint the following Markdown source text containing a JS code block inside a blockquote:
What ESLint sees is this:
The fix text sent to the processor will contain the string, now with double quotes, and preserving the newline as-is:
The fix would then be applied incorrectly because it didn't account for the leading
>in the Markdown source text:This change tracks the leading text that precedes the first line of any fenced code block and inserts the same prefix after any newline in a fix, so the fix text in the above example would become: