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

Remove extra inner linebreak from code blocks #1266

Merged
merged 1 commit into from
May 11, 2018
Merged

Remove extra inner linebreak from code blocks #1266

merged 1 commit into from
May 11, 2018

Conversation

remyrylan
Copy link
Contributor

@remyrylan remyrylan commented May 11, 2018

Description

  • Fixes
    Currently, marked adds a line break before inserting </code></pre>, some highlighting and parsing tools I've ran into actually interpret this line break (since it's wrapped in a <pre>) as an extra line break that should be displayed in the output.

Essentially the following HTML from marked:

<pre><code class="language-js">const foo = 'bar'
</code></pre>

Will produce rendered output with some highlighters as having an extra trailing inner line break:

const foo = 'bar'

Expectation

(no trailing inner line break)

<pre><code class="language-js">const foo = 'bar'</code></pre>

Then highlighters will render correctly as:

const foo = 'bar'

Contributor

  • Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,

Committer

In most cases, this should be a different person than the contributor.

  • Draft GitHub release notes have been updated.
  • CI is green (no forced merge required).
  • Merge PR

Copy link
Member

@UziTech UziTech left a comment

Choose a reason for hiding this comment

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

LGTM

@remyrylan
Copy link
Contributor Author

Forced a new commit that removed marked.min.js from my changes.

Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Nice!

@styfle
Copy link
Member

styfle commented May 11, 2018

@remyrylan Thanks for the contribution!

@styfle styfle merged commit 0218945 into markedjs:master May 11, 2018
@remyrylan remyrylan deleted the remove-extra-linebreak branch May 11, 2018 17:26
@hail2u
Copy link
Contributor

hail2u commented Sep 2, 2018

I think both CommonMark spec and GFM spec say last line break should be left as it is. It is a highlighter bug, not marked or spec bug. It must be reverted.

@styfle
Copy link
Member

styfle commented Sep 2, 2018

@hail2u Which example number in the spec are you referencing?

@hail2u
Copy link
Contributor

hail2u commented Sep 2, 2018

Example 109 is most clear. This 1 line fenced code block has line break. Also commonmark.js dingus can reproduce this behavior.

20180902221003

@styfle
Copy link
Member

styfle commented Sep 2, 2018

This is a bit strange since the spec doesn't seem to call it out directly. Only the example html output (and consequently the dingus) show this "feature".

Example 99 seems to avoid adding a newline.

Example 109 seems to add a newline.

Our tests show both 99 and 109 as passing but I would expect one to pass and one to fail.

@UziTech Is this due to the way html-differ works?

@UziTech
Copy link
Member

UziTech commented Sep 2, 2018

Most likely an html-differ bug.

@hail2u why is this a problem, that the last line break is missing? Did you run into a specific case where adding the line break would be beneficial?

@hail2u
Copy link
Contributor

hail2u commented Sep 2, 2018

To be honest, there is no big problem here. Some syntax highlighter shows character just before line break like GUI text editors. With an HTML rendered by marked v0.5.0, they failed to show last because there is no last line break. But this is a little problem.

I think the problem mentioned in this issue is also a little problem. The original markdown, CommonMark, GFM, and marked prior to v0.4.0 leave the last line break as it is. It would be a reasonable and a safest behavior, isn’t it? Markdown has been used for a long time, there might be a tool that depends on this behavior.


I find a clear rule about this line break from the CommonMark spec - 4.4 Indented code blocks:

... The contents of the code block are the literal contents of the lines, including trailing line endings, ...

@Feder1co5oave
Copy link
Contributor

The passing tests are because of this bug in html-differ.
My fork should fix this. Try it out and see what goes wrong.

zhenalexfan pushed a commit to zhenalexfan/MarkdownHan that referenced this pull request Nov 8, 2021
Remove extra inner linebreak from code blocks
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.

5 participants