Repository navigation
Ban unresolved dartdoc directives from HTML output - #62167
Conversation
fluttergithubbot
added
p: material_ui
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
The entire thing is a test for our dart docs :P |
bfdc384 to
191a77b
Compare
There was a problem hiding this comment.
Doesn't this eliminate most of the files?
There was a problem hiding this comment.
It eliminates class-level and library-level files. The majority of files are documentation for properties, which are getting checked. My hope is that we can get this fixed soon-ish on the dartdoc side.
There was a problem hiding this comment.
What about something like this instead? (and do use r'' strings).
| final RegExp _pattern = RegExp('({@[^}\n]*}?)(?![^<>]*<\/code)'); | |
| final RegExp _pattern = RegExp(r'(<code[^<>]*>)(?!.*?<code[^<>]*>).*?({@[^}\n]*}).*?(<\/code>)'); |
And why do you have the ? after the }? are macros expected to not be terminated? I mean sure, if someone is misspelling them, they can also not terminate them, but that seems unlikely.
There was a problem hiding this comment.
Added r.
Since it is essentially free, I do want to catch the error where a macro is not terminated (or has been terminated with a ) instead of a } etc).
I don't fully understand the regex suggestion. Wouldn't that match things inside a code tag?
There was a problem hiding this comment.
Yes, I wasn't thinking about the regex in the right way: my suggestion would only match inside of a code block. Please ignore it. :-)
There was a problem hiding this comment.
Given this sort of error, do we want to just search for all words beginning with "@" outside of code blocks? Or is that too many false positives?
There was a problem hiding this comment.
Unfortunately, that has too many false positives. If that becomes a common source of error, we could also match on something like (@foo} and report an error for those.
There was a problem hiding this comment.
Yeah, I figured. OK.
goderbauer
added
waiting for tree to go green
and removed
waiting for tree to go green
labels
191a77b to
4f1a086
Compare
4f1a086 to
ab6bb62
Compare
|
This pull request is not suitable for automatic merging in its current state.
|
4 participants
Description
Due to spelling mistakes (or other errors) the HTML of our dartdocs sometimes still contain unresolved dartdoc directives verbatim (e.g.
{@foo x y}). This usually indicates an error, those should not be part of the final output. This PR adds a check that runs over the generated dartdocs to ensure that no unresolved directives are included in the output. It also fixes all unresolved directives in the framework.Due to bugs in dartdoc (namely, dart-lang/dartdoc#1945 and dart-lang/dartdoc#2272) some unresolved directives will still appear in the output. The check contains exceptions for those. Once those issues are fixed, those exceptions will be removed.
Related Issues
Fixes #47652.
dart-lang/dartdoc#1945
dart-lang/dartdoc#2272
Tests
I added the following tests:
This entire thing is a test for our docs.
Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.