Should recognize generic syntax in plain text in dart docs rather than treating it as HTML

[Hixie]

The following comment:

/// This class specializes the interpolation of Tween<int> to be

...turns into the text "This class specializes the interpolation of Tween to be" because the <int> part is treated like HTML markup.

[astashov]

How do we want it to be fixed?

• We can try to not treat HTML markup in the comments at all
• We can recognize if it's a valid HTML tag - then treat it as HTML, otherwise - escape it so it will be shown as text
• We can check if there's a closing tag - if not, then it's a generic :)
• Something else?

What do you think? /cc @devoncarew

[Hixie]

Personally I'd be fine with not supporting HTML at all, except that we use it in a few places:

• Element.updateChild has some <table> markup because markdown tables weren't working. This case would be handled fine by either having markdown tables or whitelisting table, tbody, thead, tr, th, td, etc. Note that there is markdown mixed in with the markup here.
• material/icons.dart uses some crazy markup for various reasons. This case could be handled by allowing markup on any line that starts with a < (and ignoring any markdown on that line).

[Hixie]

Don't check for a closing tag, those are sometimes optional in HTML (and indeed are omitted in both of the cases above).
Checking for a whitelist of tags would work so long as none of them match class names (like "int" or "Table", though case sensitivity could help with the latter case).

[Hixie]

Hm, I just noticed that mixing markdown and markup in Element.updateChild isn't working. For tables I guess we really just want true markdown tables.

[astashov]

This case could be handled by allowing markup on any line that starts with a < (and ignoring any markdown on that line

It seems like just having this would solve the problem described in this ticket, right?

[Hixie]

Yeah that would be fine by me. We'll have to figure out what to do with the Element.updateChild case though.

[astashov]

I will look at it.

[astashov]

Actually, @Hixie, if you write it like this:

/// This class specializes the interpolation of [Tween<int>] to be

(i.e. in square brackets), it will look right. Would that solve the problem for you?

[Hixie]

What does it hyperlink to?

[astashov]

Tween class, if in scope. Nothing (will just be wrapped into <code>) - if it's not.

[Hixie]

Sounds good to me.

In that case I guess the problem boils down to us detecting cases of invalid HTML in our dartdoc markdown.

[astashov]

In that case I guess the problem boils down to us detecting cases of invalid HTML in our dartdoc markdown.

Umm, what for? Could you give an example?

[Hixie]

To catch the cases where I incorrectly did things like Tween<int> instead of [Tween<int>]. :-)

[astashov]

Oh, gotcha. So, basically it will be a whitelist of tags? And a warning in the console output if it's not in a whitelist?

[Hixie]

Oh that would be amazing, yeah. I actually just meant that we'd run some HTML validator over the site, but if you can actually catch things at build time that'd be even better.

In general my dream is to get to a point where any time there's anything wrong with the docs (including any time we use [] cross-references but they don't work), we get an error in the PR that introduces the error so that we can deal with it. So yeah, a warning (which we could then fail the PR from) would be awesome.

[astashov]

Right now I don't see any warning/error system in dartdoc, probably would be a good idea to add.
Like, if there's at least one warning, we will exit with the exit code 2, if there's at least one error, we'll exit with the exit code 1. If nothing - then 0. Or something like that.