Detect empty links on markdown rendering and issue an error #884
Conversation
| @@ -242,6 +242,10 @@ pub fn markdown_to_html(content: &str, context: &RenderContext) -> Result<Render | |||
|
|
|||
| Event::Start(Tag::Image(link_type, src, title)) | |||
| } | |||
| Event::Start(Tag::Link(link_type, link, title)) if link.is_empty() => { | |||
| error = Some(Error::msg("There is a link that is missing a URL")); | |||
There was a problem hiding this comment.
Can we add the link type/title to the error message?
There was a problem hiding this comment.
I actually had it with the title in the message initially but for some reason the title variable there is an empty string. I didn't have more time to look into it. My initial guess was that maybe the Markdown parser is not properly parsing a link without a URL.
There was a problem hiding this comment.
ah interesting, I'll see if I can reproduce it and open an issue on pulldown-cmark
There was a problem hiding this comment.
@Keats did you manage to reproduce it? Is it really an issue or did I misunderstand something?
There was a problem hiding this comment.
I haven't tried yet, I'll check when I have time
There was a problem hiding this comment.
Ah nevermind, title is [I'm an inline-style link with title](https://www.google.com "Google's Homepage") Google's Homepage. It looks like you don't get the link text in the event
|
A bit late but can you add a test for that? |
Yeah, sure. I'll look into that. |
| let res = render_content(content, &context); | ||
|
|
||
| assert!(res.is_err()); | ||
| } |
There was a problem hiding this comment.
@Keats I was not terribly creative with the test. Should I test for something else other than erring out?
There was a problem hiding this comment.
You can check that the description contains the expected string
When testing for empty links detection compare the error message to make sure it's the correct error that stopped the process and not some unrelated issue.
|
Thanks! |
* Detect empty links on markdown rendering and issue an error * Add a test for empty links stopping rendering with an error * Assert error message is the expected one When testing for empty links detection compare the error message to make sure it's the correct error that stopped the process and not some unrelated issue.
Referencing #871
Sanity check:
Code changes
(Delete or ignore this section for documentation changes)
nextbranch?If the change is a new feature or adding to/changing an existing one: