graphql-markdown icon indicating copy to clipboard operation
graphql-markdown copied to clipboard

Fixing issue where newline cause errors in Markdown files while rende…

Open jaygurnani opened this issue 5 years ago • 7 comments

…ring

Errors while rendering

Some markdown engines have errors when trying to render html tables that have new lines in them. Specfically, new lines in

element cause an issue By removing them we are fixing this issue. @exogen Are you the right person to review this?

jaygurnani avatar Nov 18 '20 04:11 jaygurnani

Coverage Status

Coverage remained the same at 77.429% when pulling 5b197ef371d56afc53dfa7fe26326332f2c12eed on jaygurnani:master into a543fd8eac9c44f2f865e7abb9fca215dfa50756 on exogen:master.

coveralls avatar Nov 18 '20 05:11 coveralls

To my knowledge, this would break the rendering in Github Flavored Markdown, but I haven't tested it yet.

Historically at least, GFM requires the newlines in order to treat the HTML content as Markdown again (vs. raw HTML).

e.g.

<td>
some content here <-- not parsed as Markdown
</td>
<td>

some content here <-- parsed as Markdown

</td>

Will have to look into whether this is still the case. What Markdown engines is it a problem for?

exogen avatar Nov 26 '20 08:11 exogen

To my knowledge, this would break the rendering in Github Flavored Markdown, but I haven't tested it yet.

Looks like this is still the case – see this example: https://gist.github.com/exogen/2db509b6926f672225a58c4aa21b7829 (source)

Perhaps we could add an option to support different Markdown engines?

exogen avatar Nov 26 '20 08:11 exogen

Perhaps we could add an option to support different Markdown engines?

Yeah I am happy with this approach. The markdown rendering engine I am using is blackfriday which is being wrapped around by another rendering engine called Hugo

Unfortunately there is no easy way to show you the error you get since there are quite a few layers involved, but from the documentation of blackfriday, you can see explicitly that elements inside <td> do not have a newline separator - see here https://github.com/russross/blackfriday/blob/master/testdata/Markdown%20Documentation%20-%20Syntax.text line 102

For example, to add an HTML table to a Markdown article:

    This is a regular paragraph.

    <table>
        <tr>
            <td>Foo</td>
        </tr>
    </table>

    This is another regular paragraph.

Note that Markdown formatting syntax is not processed within block-level
HTML tags. E.g., you can't use Markdown-style `*emphasis*` inside an
HTML block.

Rendered version of the syntax guide: https://test-staticman.netlify.app/archive/content-organisation/blackfriday-markdown/

A new line character in between the <td> simply causes the web server to return a 500, while removing the new line character allows the markdown engine to render correctly.

jaygurnani avatar Nov 30 '20 00:11 jaygurnani

Note that Markdown formatting syntax is not processed within block-level HTML tags. E.g., you can't use Markdown-style *emphasis* inside an HTML block.

What should happen to the field & argument descriptions in this case? Let's say someone does put a newline or other Markdown in their argument or field description, would they just not be able to use this output mode, or if they need it, keep that limitation in mind when writing their schema doc comments?

A couple other options would be to render their Markdown for them (so it gets turned into HTML and thus can be stuck in the <td>, or strip Markdown formatting from it to force it to text only – but both of those involve adding a Markdown renderer as a dependency, which I'd like to avoid. (One nice property of the current setup is that we just stick their Markdown into the template literally, there's no need to actually process it.)

exogen avatar Nov 30 '20 03:11 exogen

Maybe the most flexible would be to have a hook where they could supply a function to process the Markdown content however they'd like? Then the default could be what happens now – the \n${field.description}\n behavior – but if they want to either strip certain characters or syntax, or render it with their Markdown renderer of choice into HTML, they could do that?

exogen avatar Nov 30 '20 03:11 exogen

Maybe the most flexible would be to have a hook where they could supply a function to process the Markdown content however they'd like? Then the default could be what happens now – the \n${field.description}\n behavior – but if they want to either strip certain characters or syntax, or render it with their Markdown renderer of choice into HTML, they could do that?

Yeah, this approach works as well, it would definitely solve the above problem.

jaygurnani avatar Nov 30 '20 03:11 jaygurnani