Skip to content

eliminate markdown generation when unneeded #1446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/src/html/html_generator_instance.dart
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class HtmlGeneratorInstance implements HtmlOptions {
generatePackage();

for (var lib in package.libraries) {
//if (lib.name != 'flutter_test') continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to commit this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gah. will fix.

generateLibrary(package, lib);

for (var clazz in lib.allClasses) {
Expand Down
208 changes: 170 additions & 38 deletions lib/src/markdown_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@
library dartdoc.markdown_processor;

import 'dart:convert';
import 'dart:io';
import 'dart:math';

import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/dart/element/member.dart' show Member;
import 'package:html/parser.dart' show parse;
import 'package:markdown/markdown.dart' as md;
import 'package:tuple/tuple.dart';

import 'model.dart';

Expand Down Expand Up @@ -134,9 +136,6 @@ final RegExp isConstructor = new RegExp(r'^new[\s]+', multiLine: true);
// Covers anything with leading digits/symbols, empty string, weird punctuation, spaces.
final RegExp notARealDocReference = new RegExp(r'''(^[^\w]|^[\d]|[,"'/]|^$)''');

// We don't emit warnings currently: #572.
const List<String> _oneLinerSkipTags = const ["code", "pre"];

final List<md.InlineSyntax> _markdown_syntaxes = [
new _InlineCodeSyntax(),
new _AutolinkWithoutScheme()
Expand All @@ -152,6 +151,22 @@ class MatchingLinkResult {
MatchingLinkResult(this.element, this.label, {this.warn: true});
}

class IterableBlockParser extends md.BlockParser {
IterableBlockParser(lines, document) : super(lines, document);

Iterable<md.Node> parseLinesGenerator() sync* {
while (!isDone) {
for (var syntax in blockSyntaxes) {
if (syntax.canParse(this)) {
md.Node block = syntax.parse(this);
if (block != null) yield (block);
break;
}
}
}
}
}

// Calculate a class hint for findCanonicalModelElementFor.
ModelElement _getPreferredClass(ModelElement modelElement) {
if (modelElement is EnclosedElement &&
Expand Down Expand Up @@ -667,23 +682,14 @@ String _linkDocReference(String codeRef, Documentable documentable,
}
}

String _renderMarkdownToHtml(Documentable element) {
NodeList<CommentReference> commentRefs = _getCommentRefs(element);
md.Node _linkResolver(String name) {
return new md.Text(_linkDocReference(name, element, commentRefs));
}

String text = element.documentation;
_showWarningsForGenericsOutsideSquareBracketsBlocks(text, element);
return md.markdownToHtml(text,
inlineSyntaxes: _markdown_syntaxes, linkResolver: _linkResolver);
}

// Maximum number of characters to display before a suspected generic.
const maxPriorContext = 20;
// Maximum number of characters to display after the beginning of a suspected generic.
const maxPostContext = 30;

final RegExp allBeforeFirstNewline = new RegExp(r'^.*\n', multiLine: true);
final RegExp allAfterLastNewline = new RegExp(r'\n.*$', multiLine: true);

// Generics should be wrapped into `[]` blocks, to avoid handling them as HTML tags
// (like, [Apple<int>]). @Hixie asked for a warning when there's something, that looks
// like a non HTML tag (a generic?) outside of a `[]` block.
Expand All @@ -697,10 +703,8 @@ void _showWarningsForGenericsOutsideSquareBracketsBlocks(
"${text.substring(max(position - maxPriorContext, 0), position)}";
String postContext =
"${text.substring(position, min(position + maxPostContext, text.length))}";
priorContext =
priorContext.replaceAll(new RegExp(r'^.*\n', multiLine: true), '');
postContext =
postContext.replaceAll(new RegExp(r'\n.*$', multiLine: true), '');
priorContext = priorContext.replaceAll(allBeforeFirstNewline, '');
postContext = postContext.replaceAll(allAfterLastNewline, '');
String errorMessage = "$priorContext$postContext";
// TODO(jcollins-g): allow for more specific error location inside comments
element.warn(PackageWarning.typeAsHtml, message: errorMessage);
Expand Down Expand Up @@ -740,19 +744,24 @@ List<int> findFreeHangingGenericsPositions(String string) {
return results;
}

class Documentation {
final String raw;
final String asHtml;
final String asOneLiner;

factory Documentation.forElement(Documentable element) {
String tempHtml = _renderMarkdownToHtml(element);
return new Documentation._internal(element.documentation, tempHtml);
}

Documentation._(this.raw, this.asHtml, this.asOneLiner);

factory Documentation._internal(String markdown, String rawHtml) {
class MarkdownDocument extends md.Document {
MarkdownDocument(
{Iterable<md.BlockSyntax> blockSyntaxes,
Iterable<md.InlineSyntax> inlineSyntaxes,
md.ExtensionSet extensionSet,
linkResolver,
imageLinkResolver})
: super(
blockSyntaxes: blockSyntaxes,
inlineSyntaxes: inlineSyntaxes,
extensionSet: extensionSet,
linkResolver: linkResolver,
imageLinkResolver: imageLinkResolver);

/// Returns a tuple of longHtml, shortHtml. longHtml is NULL if [processFullDocs] is true.
static Tuple2<String, String> _renderNodesToHtml(
List<md.Node> nodes, bool processFullDocs) {
var rawHtml = new md.HtmlRenderer().render(nodes);
var asHtmlDocument = parse(rawHtml);
for (var s in asHtmlDocument.querySelectorAll('script')) {
s.remove();
Expand All @@ -775,16 +784,139 @@ class Documentation {
// Assume the user intended Dart if there are no other classes present.
if (!specifiesLanguage) pre.classes.add('language-dart');
}
String asHtml;
String asOneLiner;

// `trim` fixes issue with line ending differences between mac and windows.
var asHtml = asHtmlDocument.body.innerHtml?.trim();
var asOneLiner = asHtmlDocument.body.children.isEmpty
if (processFullDocs) {
// `trim` fixes issue with line ending differences between mac and windows.
asHtml = asHtmlDocument.body.innerHtml?.trim();
}
asOneLiner = asHtmlDocument.body.children.isEmpty
? ''
: asHtmlDocument.body.children.first.innerHtml;
if (!asOneLiner.startsWith('<p>')) {
asOneLiner = '<p>$asOneLiner</p>';

return new Tuple2(asHtml, asOneLiner);
}

// From package:markdown/src/document.dart
// TODO(jcollins-g): consider making this a public method in markdown package
void _parseInlineContent(List<md.Node> nodes) {
for (int i = 0; i < nodes.length; i++) {
var node = nodes[i];
if (node is md.UnparsedContent) {
List<md.Node> inlineNodes =
new md.InlineParser(node.textContent, this).parse();
nodes.removeAt(i);
nodes.insertAll(i, inlineNodes);
i += inlineNodes.length - 1;
} else if (node is md.Element && node.children != null) {
_parseInlineContent(node.children);
}
}
}

/// Returns a tuple of longHtml, shortHtml (longHtml is NULL if !processFullDocs)
Tuple3<String, String, bool> renderLinesToHtml(
List<String> lines, bool processFullDocs) {
bool hasExtendedDocs = false;
md.Node firstNode;
List<md.Node> nodes = [];
for (md.Node node
in new IterableBlockParser(lines, this).parseLinesGenerator()) {
if (firstNode != null) {
hasExtendedDocs = true;
if (!processFullDocs) break;
}
firstNode ??= node;
nodes.add(node);
}
return new Documentation._(markdown, asHtml, asOneLiner);
_parseInlineContent(nodes);

String shortHtml;
String longHtml;
if (processFullDocs) {
Tuple2 htmls = _renderNodesToHtml(nodes, processFullDocs);
longHtml = htmls.item1;
shortHtml = htmls.item2;
} else {
if (firstNode != null) {
Tuple2 htmls = _renderNodesToHtml([firstNode], processFullDocs);
shortHtml = htmls.item2;
} else {
shortHtml = '';
}
}
return new Tuple3<String, String, bool>(
longHtml, shortHtml, hasExtendedDocs);
}
}

class Documentation {
final Documentable _element;
Documentation.forElement(this._element) {}

bool _hasExtendedDocs;
bool get hasExtendedDocs {
if (_hasExtendedDocs == null) {
_renderHtmlForDartdoc(_element.isCanonical && _asHtml == null);
}
return _hasExtendedDocs;
}

String _asHtml;
String get asHtml {
if (_asHtml == null) {
assert(_asOneLiner == null || _element.isCanonical);
_renderHtmlForDartdoc(true);
}
return _asHtml;
}

String _asOneLiner;
String get asOneLiner {
if (_asOneLiner == null) {
assert(_asHtml == null);
_renderHtmlForDartdoc(_element.isCanonical);
}
return _asOneLiner;
}

NodeList<CommentReference> _commentRefs;
NodeList<CommentReference> get commentRefs {
if (_commentRefs == null) _commentRefs = _getCommentRefs(_element);
return _commentRefs;
}

String get raw => _element.documentation;

void _renderHtmlForDartdoc(bool processAllDocs) {
Tuple3<String, String, bool> renderResults =
_renderMarkdownToHtml(processAllDocs);
if (processAllDocs) {
_asHtml = renderResults.item1;
}
if (_asOneLiner == null) {
_asOneLiner = renderResults.item2;
}
if (_hasExtendedDocs != null) {
assert(_hasExtendedDocs == renderResults.item3);
}
_hasExtendedDocs = renderResults.item3;
}

/// Returns a tuple of longHtml, shortHtml, hasExtendedDocs
/// (longHtml is NULL if !processFullDocs)
Tuple3<String, String, bool> _renderMarkdownToHtml(bool processFullDocs) {
md.Node _linkResolver(String name) {
return new md.Text(_linkDocReference(name, _element, commentRefs));
}

String text = _element.documentation;
_showWarningsForGenericsOutsideSquareBracketsBlocks(text, _element);
MarkdownDocument document = new MarkdownDocument(
inlineSyntaxes: _markdown_syntaxes, linkResolver: _linkResolver);
List<String> lines = text.replaceAll('\r\n', '\n').split('\n');
return document.renderLinesToHtml(lines, processFullDocs);
}
}

Expand Down
11 changes: 11 additions & 0 deletions lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ class Accessor extends ModelElement

ModelElement get enclosingCombo => _enclosingCombo;

@override
bool get isCanonical => enclosingCombo.isCanonical;

@override
void warn(PackageWarning kind, {String message, Locatable referredFrom}) {
if (enclosingCombo != null) {
Expand Down Expand Up @@ -903,6 +906,7 @@ abstract class Documentable implements Warnable {
String get documentation;
String get documentationAsHtml;
bool get hasDocumentation;
bool get hasExtendedDocumentation;
String get oneLineDoc;
Documentable get overriddenDocumentedElement;
Package get package;
Expand Down Expand Up @@ -2092,6 +2096,10 @@ abstract class ModelElement implements Comparable, Nameable, Documentable {
bool get hasDocumentation =>
documentation != null && documentation.isNotEmpty;

@override
bool get hasExtendedDocumentation =>
href != null && _documentation.hasExtendedDocs;

bool get hasParameters => parameters.isNotEmpty;

/// If canonicalLibrary (or canonicalEnclosingElement, for Inheritable
Expand Down Expand Up @@ -2963,6 +2971,9 @@ class Package implements Nameable, Documentable {
@override
Documentable get documentationFrom => this;

@override
bool get hasExtendedDocumentation => documentation.isNotEmpty;

final Map<Element, Library> _elementToLibrary = {};
String _docsAsHtml;
final Map<String, String> _macros = {};
Expand Down
2 changes: 1 addition & 1 deletion pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ packages:
name: markdown
url: "https://pub.dartlang.org"
source: hosted
version: "0.11.2"
version: "0.11.3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, not using any new APIs.

matcher:
description:
name: matcher
Expand Down
14 changes: 11 additions & 3 deletions test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ void main() {
expect(
fakeLibrary.oneLineDoc,
equals(
'<p>WOW FAKE PACKAGE IS <strong>BEST</strong> <a href="http://example.org">PACKAGE</a></p>'));
'WOW FAKE PACKAGE IS <strong>BEST</strong> <a href="http://example.org">PACKAGE</a>'));
});

test('has properties', () {
Expand Down Expand Up @@ -427,14 +427,22 @@ void main() {
contains("['hello from dart']"));
});

test('class without additional docs', () {
expect(specialList.hasExtendedDocumentation, equals(false));
});

test('class with additional docs', () {
expect(Apple.hasExtendedDocumentation, equals(true));
});

test('oneLine doc references in inherited methods should not have brackets',
() {
Method add =
specialList.allInstanceMethods.firstWhere((m) => m.name == 'add');
expect(
add.oneLineDoc,
equals(
'<p>Adds <code>value</code> to the end of this list,\nextending the length by one.</p>'));
'Adds <code>value</code> to the end of this list,\nextending the length by one.'));
});

test(
Expand Down Expand Up @@ -515,7 +523,7 @@ void main() {
expect(
testingCodeSyntaxInOneLiners.oneLineDoc,
equals(
'<p>These are code syntaxes: <code>true</code> and <code>false</code></p>'));
'These are code syntaxes: <code>true</code> and <code>false</code>'));
});

test('doc comments to parameters are marked as code', () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ <h2>Functions</h2>
</span>
</dt>
<dd>
<p></p>


</dd>
</dl>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ <h2>Functions</h2>
</span>
</dt>
<dd>
<p></p>


</dd>
</dl>
Expand Down
2 changes: 1 addition & 1 deletion testing/test_package_docs/css/css-library.html
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ <h2>Properties</h2>
</span>
</dt>
<dd>
<p></p>

<div class="features">read / write</div>
</dd>
</dl>
Expand Down
Loading