Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Markdown: Preserve multiple spaces in inline code #13590

Merged
merged 22 commits into from
Oct 14, 2022
Merged

Markdown: Preserve multiple spaces in inline code #13590

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f94f5a8
Markdown: Preserve multiple spaces in inline code
kachkaev Oct 8, 2022
c32ebba
Add changelog entry
kachkaev Oct 8, 2022
afd962e
Improve newline handling
kachkaev Oct 8, 2022
15aee10
Add more CommonMark examples to tests
kachkaev Oct 8, 2022
d5fdb2c
Move commonmark-0.30-example-331.md to `remark-bug`
kachkaev Oct 8, 2022
e0f2ed2
Tweak changelog entry
kachkaev Oct 8, 2022
8d23538
Keep leading & trailing spaces
kachkaev Oct 9, 2022
4fc4693
Update blog post
kachkaev Oct 9, 2022
dbe851d
Merge remote-tracking branch 'origin/next' into inline-code-spacing
kachkaev Oct 9, 2022
6af20a1
Handle options.proseWrap, add test case
kachkaev Oct 9, 2022
feee9dd
Move test file
kachkaev Oct 9, 2022
17f18ad
Merge remote-tracking branch 'origin/next' into inline-code-spacing
kachkaev Oct 10, 2022
9a7cd17
Add spaces in print, not in preprocess
thorn0 Oct 13, 2022
e2abead
Fix adding padding for string with backticks
thorn0 Oct 13, 2022
5c7d4e1
Update changelog_unreleased/markdown/13590.md
thorn0 Oct 13, 2022
6489a4d
Add tests for proseWrap: preserve
thorn0 Oct 13, 2022
c5677d0
Refactor
thorn0 Oct 13, 2022
0856e6a
Follow spec more closely
thorn0 Oct 13, 2022
0b42cdf
Follow spec even more closely
thorn0 Oct 13, 2022
f939b6d
Add a test
thorn0 Oct 13, 2022
33d7a43
Fix one more case
thorn0 Oct 13, 2022
5350af8
Remove preprocess, move logic to print
thorn0 Oct 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions changelog_unreleased/markdown/13590.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#### Preserve multiple spaces in inline code (#13590 by @kachkaev)
thorn0 marked this conversation as resolved.
Show resolved Hide resolved

Previously, multiple whitespace characters in inline code were collapsed into a single space. This is no longer happening to match [CommonMark spec](https://spec.commonmark.org/0.30/#backtick-string).

<!-- prettier-ignore -->
```markdown
<!-- Input -->
` foo bar baz `

<!-- Prettier stable -->
` foo bar baz `

<!-- Prettier main -->
` foo bar baz `
```
6 changes: 5 additions & 1 deletion src/language-markdown/clean.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ function clean(ast, newObj, parent) {
}

if (ast.type === "inlineCode") {
newObj.value = ast.value.replace(/[\t\n ]+/g, " ");
const value = ast.value.replace(/\n/g, " ");
newObj.value =
value.startsWith(" ") && value.endsWith(" ") && value.trim().length > 0
? " " + value + " "
: value;
}

if (ast.type === "wikiLink") {
Expand Down
15 changes: 13 additions & 2 deletions src/language-markdown/print-preprocess.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,22 @@ function transformImportExport(ast) {

function transformInlineCode(ast, options) {
return mapAst(ast, (node) => {
if (node.type !== "inlineCode" || options.proseWrap === "preserve") {
if (node.type !== "inlineCode") {
return node;
}

return { ...node, value: node.value.replace(/\s+/g, " ") };
const value =
options.proseWrap === "preserve"
? node.value
: node.value.replace(/\n/g, " ");

return {
...node,
value:
value.startsWith(" ") && value.endsWith(" ") && value.trim().length > 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something doesn't look right here. Is this part really needed? Isn't this check and processing done by the parser already?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I see why you did this. But this should be done in print, not in preprocess.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for the fix! 🙌

Do you know why we still need newObj.value = ast.value.replace(/\n/g, " "); in clean.js? This transform is applied regardless of proseWrap. I vaguely remember trying to remove it but tests failed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those tests are run only with proseWrap: "always", that's why it works. Need to think what to do.

Copy link
Member

@thorn0 thorn0 Oct 13, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I was wrong. It's some other tests that fail if I add

run_spec(import.meta, ["markdown"], { proseWrap: "preserve" });
run_spec(import.meta, ["markdown"], { proseWrap: "never" });

to jsfmt.spec.js, not related to code spans.

As for clean.js, see how the clean function (AKA massageAST) is used:

prettier/src/cli/format.js

Lines 137 to 169 in 955553b

if (context.argv.debugCheck) {
const pp = prettier.format(input, opt);
const pppp = prettier.format(pp, opt);
if (pp !== pppp) {
throw new errors.DebugError(
"prettier(input) !== prettier(prettier(input))\n" + diff(pp, pppp)
);
} else {
const stringify = (obj) => JSON.stringify(obj, null, 2);
const ast = stringify(
prettier.__debug.parse(input, opt, /* massage */ true).ast
);
const past = stringify(
prettier.__debug.parse(pp, opt, /* massage */ true).ast
);
/* istanbul ignore next */
if (ast !== past) {
const MAX_AST_SIZE = 2097152; // 2MB
const astDiff =
ast.length > MAX_AST_SIZE || past.length > MAX_AST_SIZE
? "AST diff too large to render"
: diff(ast, past);
throw new errors.DebugError(
"ast(input) !== ast(prettier(input))\n" +
astDiff +
"\n" +
diff(input, pp)
);
}
}
return { formatted: pp, filepath: opt.filepath || "(stdin)\n" };
}

It's applied to both the original AST and to the AST built from Prettier's output, and then the ASTs are checked for deep equality. So it makes sense that this replacement is needed. After it, no matter what proseWrap is, the values in the two ASTs are equal.

? " " + value + " "
: value,
};
});
}

Expand Down
2 changes: 1 addition & 1 deletion tests/format/js/multiparser-markdown/__snapshots__/jsfmt.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ markdown\`

=====================================output=====================================
markdown\`
const cssString = css\\\` background-color: \\$\\{color('base')\\}\\\`;
const cssString = css\\\` background-color: \\$\\{color('base')\\}\\\`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this call markdown -> js -> css and remove the leading space? Because it's not valid css?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a crazy test. const cssString = ... is Markdown here, not JS. The Markdown-in-JS embedder also includes logic for ignoring indentation (no idea what's the story behind that). That's why the output is like that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you explained before... forgot. Sorry.

\`;

markdown\`
Expand Down
50 changes: 50 additions & 0 deletions tests/format/markdown/inlineCode/__snapshots__/jsfmt.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,56 @@ proseWrap: "always"
================================================================================
`;

exports[`inline-code-multiple-spaces.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
\` three spaces everywhere \`

\` three spaces
everywhere \`

\` three spaces
everywhere \`

=====================================output=====================================
\` three spaces everywhere \`

\` three spaces everywhere \`

\` three spaces everywhere \`

================================================================================
`;

exports[`inline-code-newline.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod \`tempor
incididunt\` ut labore et dolore magna aliqua. Ut enim ad minim veniam, \`quis
nostrud\` exercitation ullamco laboris nisi ut aliquip ex ea commodo \`consequat.
Duis\` aute irure dolor in reprehenderit in voluptate velit esse cillum dolore \`eu
fugiat\` nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in
culpa qui officia deserunt mollit anim id est laborum.

=====================================output=====================================
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
\`tempor incididunt\` ut labore et dolore magna aliqua. Ut enim ad minim veniam,
\`quis nostrud\` exercitation ullamco laboris nisi ut aliquip ex ea commodo
\`consequat. Duis\` aute irure dolor in reprehenderit in voluptate velit esse
cillum dolore \`eu fugiat\` nulla pariatur. Excepteur sint occaecat cupidatat non
proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

================================================================================
`;

exports[`long.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
Expand Down
7 changes: 7 additions & 0 deletions tests/format/markdown/inlineCode/inline-code-multiple-spaces.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
` three spaces everywhere `

` three spaces
everywhere `

` three spaces
everywhere `
6 changes: 6 additions & 0 deletions tests/format/markdown/inlineCode/inline-code-newline.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod `tempor
incididunt` ut labore et dolore magna aliqua. Ut enim ad minim veniam, `quis
nostrud` exercitation ullamco laboris nisi ut aliquip ex ea commodo `consequat.
Duis` aute irure dolor in reprehenderit in voluptate velit esse cillum dolore `eu
fugiat` nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in
culpa qui officia deserunt mollit anim id est laborum.
161 changes: 159 additions & 2 deletions tests/format/markdown/spec/__snapshots__/jsfmt.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,162 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`commonmark-0.30-example-328.md - {"proseWrap":"always"} format 1`] = `
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context: #13591

====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
\`foo\`

=====================================output=====================================
\`foo\`

================================================================================
`;

exports[`commonmark-0.30-example-329.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
\`\` foo \` bar \`\`

=====================================output=====================================
\`\` foo \` bar \`\`

================================================================================
`;

exports[`commonmark-0.30-example-330.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
\` \`\` \`

=====================================output=====================================
\` \`\` \`

================================================================================
`;

exports[`commonmark-0.30-example-331.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
\` \`\` \`

=====================================output=====================================
\` \`\` \`

================================================================================
`;

exports[`commonmark-0.30-example-332.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
\` a\`

=====================================output=====================================
\` a\`

================================================================================
`;

exports[`commonmark-0.30-example-333.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
\` b \`

=====================================output=====================================
\` b \`

================================================================================
`;

exports[`commonmark-0.30-example-334.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
\` \`
\` \`

=====================================output=====================================
\` \` \` \`

================================================================================
`;

exports[`commonmark-0.30-example-335.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
\`\`
foo
bar
baz
\`\`

=====================================output=====================================
\`foo bar baz\`

================================================================================
`;

exports[`commonmark-0.30-example-336.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
\`\`
foo
\`\`

=====================================output=====================================
\`foo \`

================================================================================
`;

exports[`commonmark-0.30-example-337.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
printWidth: 80
proseWrap: "always"
| printWidth
=====================================input======================================
\`foo bar
baz\`
=====================================output=====================================
\`foo bar baz\`

================================================================================
`;

exports[`example-1.md - {"proseWrap":"always"} format 1`] = `
====================================options=====================================
parsers: ["markdown"]
Expand Down Expand Up @@ -6012,7 +6169,7 @@ proseWrap: "always"
baz\`

=====================================output=====================================
\`foo bar baz\`
\`foo bar baz\`

================================================================================
`;
Expand Down Expand Up @@ -10593,7 +10750,7 @@ proseWrap: "always"
span\`

=====================================output=====================================
\`code span\`
\`code span\`

================================================================================
`;
Expand Down
1 change: 1 addition & 0 deletions tests/format/markdown/spec/commonmark-0.30-example-328.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`foo`
1 change: 1 addition & 0 deletions tests/format/markdown/spec/commonmark-0.30-example-329.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`` foo ` bar ``
1 change: 1 addition & 0 deletions tests/format/markdown/spec/commonmark-0.30-example-330.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
` `` `
1 change: 1 addition & 0 deletions tests/format/markdown/spec/commonmark-0.30-example-331.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
` `` `
1 change: 1 addition & 0 deletions tests/format/markdown/spec/commonmark-0.30-example-332.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
` a`
1 change: 1 addition & 0 deletions tests/format/markdown/spec/commonmark-0.30-example-333.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
` b `
2 changes: 2 additions & 0 deletions tests/format/markdown/spec/commonmark-0.30-example-334.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
` `
` `
5 changes: 5 additions & 0 deletions tests/format/markdown/spec/commonmark-0.30-example-335.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
``
foo
bar
baz
``
3 changes: 3 additions & 0 deletions tests/format/markdown/spec/commonmark-0.30-example-336.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
``
foo
``
2 changes: 2 additions & 0 deletions tests/format/markdown/spec/commonmark-0.30-example-337.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
`foo bar
baz`