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.

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

markdown: remove unnecessary blockquote > #9125

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

shisama
Copy link
Contributor

@shisama shisama commented Sep 1, 2020

Fix #9099

This removes unnecessary blockquote > when the paragraph is made a single line.
I am not confident my implementation is good. Please tell me if you know a better way to fix this.

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/pr-XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@thorn0
Copy link
Member

thorn0 commented Sep 1, 2020

This solution seems to be fighting the consequences, not the reason of the bug. > shouldn't end up in a node of the type word in the first place. There is the code in preprocess.js that takes the original remark AST and creates these word nodes. The bug must be somewhere in that code.

@kachkaev
Copy link
Member

kachkaev commented Sep 1, 2020

Once the right solution is found, it'd be also great to add changelog_unreleased/markdown/pr-9125.md file, similar to a markdown in #9078

@shisama shisama force-pushed the markdown-remove-unneed-blockquote branch from 0617bd3 to cc43e2b Compare September 2, 2020 00:54
@thorn0
Copy link
Member

thorn0 commented Sep 2, 2020

Lines shouldn't be joined when --prose-wrap is set to preserve. Also please make sure to test nested blockquotes.

Prettier pr-9125
Playground link

--parser markdown

Input:

> `1`
> `2`

Output:

> `1` `2`

Expected: same as input

@thorn0
Copy link
Member

thorn0 commented Sep 2, 2020

A solution that involves replacing > in a string can't be correct. If > has been already stringified, it's too late to meddle. It shouldn't be there. Why does it get there?

@shisama
Copy link
Contributor Author

shisama commented Sep 15, 2020

@thorn0 Thanks. I found the cause that > is included. options.originalText.slice() in the below code transforms \n \n> . So I fixed the condition of this. But I'm not confident if the solution is correct.
Please take a look. d43f64d

value:
node.value !== "*" &&
node.value !== "_" &&
node.value !== "$" && // handle these cases in printer
isSingleCharRegex.test(node.value) &&
node.position.end.offset - node.position.start.offset !==
node.value.length
? options.originalText.slice(
node.position.start.offset,
node.position.end.offset
)

And I added a test for nested blockquotes. 64f13fc

> `x` > `y`

// Prettier master
> `x` `y`
Copy link
Member

Choose a reason for hiding this comment

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

This output is incorrect with our default options(--prose-wrap=preserve ).
Can you change this to

> `x`
> `y`

?


Prettier pr-9125
Playground link

--parser markdown

Input:

> `x`
> `y`

Output:

> `x`
> `y`

Copy link
Member

@sosukesuzuki sosukesuzuki left a comment

Choose a reason for hiding this comment

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

Can you add tests for prose-wrap=never and prose-wrap=preserve with updating tests/markdown/jsfmt.spec.js?

@kachkaev
Copy link
Member

kachkaev commented Dec 11, 2023

👋 @shisama, would you be interested in updating this PR to help us close #9099? See suggestion from @sosukesuzuki above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Markdown: Blockquote > not removed correctly when wrapping adjacent markdown syntax
4 participants