eliminate markdown generation when unneeded #1446
Conversation
Merge branch 'master' of github.com:dart-lang/dartdoc into partial-markdown
lib/src/markdown_processor.dart
Outdated
| hasExtendedDocs = true; | ||
| if (!processFullDocs) break; | ||
| } | ||
| if (firstNode == null) { |
There was a problem hiding this comment.
This could be firstNode ??= node
lib/src/markdown_processor.dart
Outdated
| shortHtml = ''; | ||
| } | ||
| } | ||
| if (!shortHtml.startsWith('<p>')) { |
There was a problem hiding this comment.
Why are we doing this - is the the short html version doesn't convert as cleanly/consistently to html?
There was a problem hiding this comment.
To be honest, I have no idea -- I left it in because the old code did that and some of the tests seemed to verify it. However, I can see nothing that actually requires this -- docs look and browse OK even without these tags. Removing. This will trigger a large set of meaningless test package doc changes and test changes.
There was a problem hiding this comment.
I was curious - didn't realize it was pre-existing.
lib/src/markdown_processor.dart
Outdated
| return document.renderLinesToHtml(lines, processFullDocs); | ||
| } | ||
|
|
||
| Documentation.forElement(this._element) {} |
There was a problem hiding this comment.
We generally put the ctors near the beginning of the class.
There was a problem hiding this comment.
Right, cut and pastyness got out of control. Reorged this class.
| url: "https://pub.dartlang.org" | ||
| source: hosted | ||
| version: "0.11.2" | ||
| version: "0.11.3" |
There was a problem hiding this comment.
Just want to confirm, that we're not using new API from 0.11.3? (If so, we'll also need to bump the pubspec)
There was a problem hiding this comment.
Correct, not using any new APIs.
|
lgtm! What's the performance improvement like? |
|
Performance improves another 10-15% for generation. With this change, generation time is now dominated by the link checker when building docs for flutter; parallelizing parsing the HTML could cut us down another 40% in wall time. |
| generatePackage(); | ||
|
|
||
| for (var lib in package.libraries) { | ||
| //if (lib.name != 'flutter_test') continue; |
There was a problem hiding this comment.
Did you mean to commit this?
This fixes #1417 and prepares for #1388 with a new Documentable property, hasExtendedDocumentation.