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

RTE should insert a newline before code block #5674

Closed
pafcu opened this issue Nov 23, 2017 · 5 comments
Closed

RTE should insert a newline before code block #5674

pafcu opened this issue Nov 23, 2017 · 5 comments
Labels
A-Timeline P2 S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect

Comments

@pafcu
Copy link
Contributor

pafcu commented Nov 23, 2017

Description

Markdown requires that the code block triple-backticks start on a new line. However, the code button in the RTE does not add a newline, which may lead to "broken" markdown.

Steps to reproduce

  • Press the "Show Text Formatting Toolbar" button in the RTE
  • Write "foo" in RTE
  • Press the '</>' button
  • Write "bar"
  • Press enter

I expect "bar" to be inside a code block. Instead I get an empty code block at the end (Markdown probably gets confused and thinks the last backticks actually start a codeblock rather than end it).

Actual result:
image

Expected result:
image

Version information

Riot web c28d9e5-react-c002d3ff9967-js-36ff0ad0193e on FF57

@lampholder
Copy link
Member

If I'm understanding http://spec.commonmark.org/0.28/#fenced-code-blocks correctly, fenced code blocks can interrupt paragraphs:

A fenced code block may interrupt a paragraph, and does not require a blank line either before or after.

Indented code blocks explictly cannot interrupt a paragraph.

However, also from the commonmark spec:

Unclosed code blocks are closed by the end of the document (or the enclosing block quote or list item)

So your example does highlight a bug in Riot's handling of the markdown - it should close the code block with the end of the document and render as per your expected result 😄

@lampholder lampholder added T-Defect S-Major Severely degrades major functionality or product features, with no satisfactory workaround P2 type:rte A-Timeline labels Nov 23, 2017
@pafcu
Copy link
Contributor Author

pafcu commented Nov 23, 2017

Ah, so that's different from e.g. GFM where fenced code blocks can't interrupt a paragraph. The joys of Markdown ...```
This should be in a code block

@pafcu
Copy link
Contributor Author

pafcu commented Nov 23, 2017

Actually the reference implementation at http://spec.commonmark.org/dingus/ also fails so either the reference implementation is wrong, the spec is wrong, or the interpretation of the spec is wrong.

image

@pafcu
Copy link
Contributor Author

pafcu commented Nov 24, 2017

A newline should also be inserted when pressing the quote button (if not pressed at the beginning of a line)

@aaronraimist
Copy link
Collaborator

This seems to work properly in the new composer (you have to select the text that you want to be in the code block now to see the RTE rather than being a button to open a code block like it was previously)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Timeline P2 S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect
Projects
None yet
Development

No branches or pull requests

3 participants