#633: Error compiling file `url.replace is not a function #676
Conversation
hkollmann
commented
May 6, 2020
hkollmann
commented
- markdown error as warning
- add more tests
- add more tests
| @@ -99,7 +99,7 @@ qx.Class.define("qx.tool.compiler.jsdoc.Parser", { | |||
| try { | |||
| cmd.body = converter.makeHtml(cmd.body); | |||
| } catch (e) { | |||
| qx.tool.compiler.Console.error(`Markdown conversion error: "${e.message}" found in \n${cmd.body.trim()}`); | |||
| qx.tool.compiler.Console.warn(`Markdown conversion: "${e.message}" found in \n${cmd.body.trim()}`); | |||
There was a problem hiding this comment.
Isn't the exception being raised a bug in the "showdown" module? EG what is wrong with having a comment like
/**
* --------------------------------------------------------------------------
* [Constructor]
* --------------------------------------------------------------------------
*/
construct: function()
Even if we decided to enforce strict rules on comment formatting, it should not say something inexplicable just url.replace is not a function.
There was a problem hiding this comment.
Maybe the simplest thing would be making the warning more explicative:
Markdown conversion problem: Error "${e.message}" was thrown parsing "${cmd.body.trim()}".
Please review your doc comments for compatibility with Markdown syntax.
There was a problem hiding this comment.
I don't think there should be anything in a comment that causes it to barf. If it's understandable as markdown, then treat it as such. If not, don't tell the user how to write a comment. (A warning that can be turned off via command-line option or @ignore or whatever is ok; just not an error.)
- add verbose property to console so that the verbose flag can be used elsewhere
|
Implemented both suggestions from @cboulanger and @derrell |
|
BTW: this |
|
and what I've understood from that bug is, that every markdown named like a javascript reserved word could cause the exception, like e.g. constructor |
There was a problem hiding this comment.
i dont think this is the right approach, we should ditch showdown altogether - if this is a two year old bug, it doesnt sound like it's maintained very well.
Even if the markdown is not valid, the warning "url.replace..." is meaningless and very misleading - it also suggests that one typo anywhere in a long comment could break the entire markdown process for that comment.
|
@johnspackman as I've understood it will only break for links, but its bad anyway |
|
Do you know an alternative to showdown? |
|
Not personally but there are several out there. Github use https://github.com/github/cmark-gfm, but that's a C library so there is this nodejs wrapper: https://github.com/killagu/node-cmark For a pure JS approach there is https://github.com/markdown-it/markdown-it "Markdown parser done right. Fast and easy to extend." |
|
@hkollmann Can you document why did you closed this? Was the bug resolved with another PR? |
|
was an accident |
|
@johnspackman You are right, but can we accept this workaround now and deal with switching the markdown parser later, since this would push the release date again? The current system is working except this bug. I'll create a separate issue for v7 so we don't forget. Ok? |
|
Thank you @hkollmann |
