Conversation
john-mueller
left a comment
There was a problem hiding this comment.
👍 from me. This PR makes three additional CommonMark tests pass, without any regressions (574, 578, and 579 in the spec).
As for needing a blank line between a reference link/image and the link reference definition, the good news is that CommonMark actually does require that space. Try the following in the dingus:
[Title][url]
[url]: swiftbysundell.com
produces
<p><a href="swiftbysundell.com">Title</a></p>
but
[Title][url]
[url]: swiftbysundell.com
produces
<p>[Title][url]
[url]: swiftbysundell.com</p>
(and similarly for images).
Note that in the case with no blank line, Ink does produce different output than CommonMark, which should be dealt with eventually, but the point remains that we don't have to make it produce the same output as with the blank line.
|
Oh, that's good news @john-mueller! I've noticed that some MD parsers do respect a reference link with no blank line, but it's inconsistent. Either way, it's definitely an edge case. Thanks for taking a look at this PR! |
|
Hey @bensyverson, I'm hoping that @JohnSundell will have some time to merge these soon. I'm thinking it would be good to go ahead and rebase this commit on top of #42 (in other words, rebase your I'm thinking it would look something like my inline-images-fixup branch. After that, maybe just add a note that this PR depends on those changes. |
Summary
tl;dr: This one-line PR allows images to create new paragraphs, by treating them like links, which are also inline elements.
Overview
This PR addresses an issue with the way that Ink currently handles images. Essentially, when an image is encountered outside of an existing paragraph fragment, Ink treats it as a fragment at the same level as
Heading,HTML,Blockquote,CodeBlock,HorizontalLineorParagraph.This prevents a bare image in its own paragraph from being surrounded by
<p></p>, and probably causes some other undesirable behavior. Images should be treated the same as links, which are very similar and do have the expected behavior in Ink.Example
Same as a link, an image passed to the parser on its own should result in a surrounding paragraph:
-><p><img src="img.jpg"/></p>And the following Markdown should generate two paragraphs:
⚡️
However, currently Ink outputs:
Regression / Implications
Drafting this PR uncovered a different bug which I did not address. The unit tests in
ImageTests.swiftput their references on the next line, like so:Meanwhile, the
Linktests all separate the reference by another line:Testing against other Markdown parsers, either style should work. However, currently (and also in this PR) in Ink, the reference must be separated by more than just a newline. And by making images behave more like links, those
Imagetests stopped working.So I introduced newlines in the
Imagetests to separate the references, and they are at least at parity with links now.Next steps
I made an effort to fix the reference behavior so that the following test would pass, but couldn't immediately see how to resolve it. @JohnSundell you may be able to work it out a lot faster!
Thanks again John!