Skip to content
This repository has been archived by the owner on Jan 15, 2022. It is now read-only.

Better handling of markdown body #232

Closed
wants to merge 1 commit into from

Conversation

roblabla
Copy link
Contributor

@roblabla roblabla commented Aug 16, 2019

The current BODY handling for newlines has a lot of pretty bad edge-cases. For instance, if we want to use triple-quote markdown in the BODY, we have to put a newline between every code line, e.g.

// BODY: ```rust
// BODY:
// BODY: test
// BODY:
// BODY: ```

This is, obviously, extremely suboptimal. In this PR, I properly parse the markdown, and output it back as "proper" markdown that avoids the soft-break behavior of github comments. It does so by rendering the markdown as HTML and back as Markdown thanks to showdown, a bidirectional (markdown to html and html to markdown) renderer.

@JasonEtco
Copy link
Owner

Hey @roblabla - thanks for opening this! I'm not sure I understand what this change accomplishes. Can you elaborate on what the intended use is after this change? If you could add a test showing it off that'd help as well.

@roblabla
Copy link
Contributor Author

So Github comments have this annoying rule where it's not your usual markdown, it has an additional weirdness where linebreaks in the comment will cause a linebreak in the rendered version (in normal markdown, a linebreak in source will not result in a linebreak in the rendered document - you need to end the line with two spaces to get that).

To fix this annoyance, I had done a PR that used a simple heuristic to join all lines that were not separated by an empty lines. This works okay for normal text, but will break badly for code blocks - see here.

To fix that, this PR uses showdown, which can turn markdown to html and back. The interesting thing is that the markdown returned from doing this has all paragraphs on a single line with no line breaks inserted, which is exactly what we want to send to the github comment!

I'll try to add a test when I have some time.

@JasonEtco
Copy link
Owner

JasonEtco commented Sep 26, 2019

Gotcha - thanks for the explanation! The comment in the issue you linked should definitely be rendered differently, with the code block. I'm not sure that converting it to HTML then to Markdown is the only way to go though; TODO should simply be the messenger, and not alter the contents of a comment if possible. Instead, I'd investigate why those individual // BODY lines are being concatenated into one line in the first place.

In this GraphQL query, you can see that the space between the code block and the text above it only has one newline character; I believe that that's the issue we should be solving.

That's likely this line, where we call .trim(); I don't remember why that's there, but it might be worth removing.

@stale
Copy link

stale bot commented Nov 25, 2019

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Nov 25, 2019
@stale stale bot closed this Dec 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants