Ignore all descriptions for rendered images

[pengjiz]

First of all, thanks for creating and maintaining this awesome tool! It is incredibly handy and useful to me.

Back to the PR. I was bitten by #285 as well so I decided to have a look at it.

According to the CommonMark Spec, all inline elements are allowed in the image description part. However, previously we only handled plain text elements (by ignoring them). So in this PR we change to ignore all elements other than image elements.

To correctly handle nested images inside the image description, here we push dummy RenderedImage states to the stack. An alternative solution would be maintaining a counter for nested image level and checking the counter when dealing with end of image events, but that feels less tasteful to me.

Note that this is a bit more lenient than the spec because it allows block elements as well. However, this behaviour aligns with the upstream library, and the more correct version is a bit tedious to implement because we have to list all inline tags ourselves.

By the way, this issue should be already covered in the 'commonmark-spec' test cases (e.g., 573-images.md). However, because the image files for those fixtures are missing, the images are never rendered as images but links (golden/iterm2/commonmark-spec/573-images). So those problems are not detected in testing. I think we perhaps need to address this as well, but I do not have a good solution. What do you think?

Re: #285, #194.

[swsnr]

@pengjiz Thanks. Would you take a look at the Github actions failure?

Wrt to testing, I'm not sure we can easily rely on the spec test cases here, given that those reference non-existing images, but we have additional test cases in pulldown-cmark-mdcat/tests/render/md/samples, so perhaps create one for nested markup in image descriptions?

Add a markdown file, and some kind of dummy image (a 1x1px white PNG image would suffice), and then regenerate the golden files (see test_with_golden_file in pulldown-cmark-mdcat/tests/render.rs).

[pengjiz]

Sebastian Wiesner ***@***.***> writes:

@pengjiz Thanks. Would you take a look at the Github actions failure?

Kinda strange that I did not get such failures with 'cargo fmt' in my local environment. Perhaps I am using a different version of rustfmt from GHA. Anyway, I have reformatted the file and those failures should be fixed now. Please see 9844fd2.

Wrt to testing, I'm not sure we can easily rely on the spec test cases here, given that those reference non-existing images, but we have additional test cases in pulldown-cmark-mdcat/tests/render/md/samples, so perhaps create one for nested markup in image descriptions? Add a markdown file, and some kind of dummy image (a 1x1px white PNG image would suffice), and then regenerate the golden files (see test_with_golden_file in pulldown-cmark-mdcat/tests/render.rs).

I added a new test in 'samples/images.md'. Please have a look at 8107719. I chose to reuse the existing image file instead of a new one, to avoid adding a new binary blob into the repository ^v^. Thanks!

[swsnr]

Thanks 🙏 rebased and merged, I'll make a bugfix release soon.

[pengjiz]

Actually I wasn't expecting such a quick release. Many thanks!