From 094f93dfcf0b35739f47476441a51b483f4a68dc Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Wed, 7 Nov 2018 08:29:14 -0800 Subject: [PATCH] Fixes several bugs in samples, quotes HTML properly, and pre-compiles snippet tool. (#24020) When converting all of the samples to use the snippet tool, I encountered some bugs/shortcomings: 1. The document production took 90 minutes, since the snippet tool was being invoked from the command line each time. I fixed this by snapshotting the executable before running, so it's down to 7 minutes. 2. The sample code was not being properly escaped by the snippet tool, so generics were causing issues in the HTML output. It is now quoted. 3. Code examples that used languages other than Dart were not supported. Anything that highlight.js was compiled for dartdoc with is now supported. 4. The comment color for highlight.js was light grey on white, which was pretty unreadable. It's now dark green and bold. --- dartdoc_options.yaml | 6 ++- dev/bots/analyze-sample-code.dart | 13 +++--- dev/docs/assets/overrides.css | 7 ++++ .../config/skeletons/application.html | 4 +- dev/snippets/config/skeletons/sample.html | 13 +++--- dev/snippets/lib/configuration.dart | 30 ++++++------- dev/snippets/lib/main.dart | 14 ++++++- dev/snippets/lib/snippets.dart | 42 ++++++++++++------- dev/snippets/test/configuration_test.dart | 20 ++++----- dev/snippets/test/snippets_test.dart | 23 ++++------ dev/tools/dartdoc.dart | 29 +++++++++++++ 11 files changed, 127 insertions(+), 74 deletions(-) diff --git a/dartdoc_options.yaml b/dartdoc_options.yaml index d340a23062..6fc305054b 100644 --- a/dartdoc_options.yaml +++ b/dartdoc_options.yaml @@ -1,9 +1,11 @@ # This file is used by dartdoc when generating API documentation for Flutter. dartdoc: + # Before you can run dartdoc, the snippets tool needs to have a snapshot built. + # The dev/tools/dartdoc.dart script does this automatically. tools: snippet: - command: ["dev/snippets/lib/main.dart", "--type=application"] + command: ["bin/cache/dart-sdk/bin/dart", "../../bin/cache/snippets.snapshot", "--type=application"] description: "Creates application sample code documentation output from embedded documentation samples." sample: - command: ["dev/snippets/lib/main.dart", "--type=sample"] + command: ["bin/cache/dart-sdk/bin/dart", "../../bin/cache/snippets.snapshot", "--type=sample"] description: "Creates sample code documentation output from embedded documentation samples." diff --git a/dev/bots/analyze-sample-code.dart b/dev/bots/analyze-sample-code.dart index 5c9f056269..115c7c62d9 100644 --- a/dev/bots/analyze-sample-code.dart +++ b/dev/bots/analyze-sample-code.dart @@ -202,22 +202,23 @@ class SampleChecker { // Precompiles the snippets tool if _snippetsSnapshotPath isn't set yet, and // runs the precompiled version if it is set. ProcessResult _runSnippetsScript(List args) { + final String workingDirectory = path.join(_flutterRoot, 'dev', 'docs'); if (_snippetsSnapshotPath == null) { _snippetsSnapshotPath = '$_snippetsExecutable.snapshot'; return Process.runSync( - Platform.executable, + path.absolute(Platform.executable), [ '--snapshot=$_snippetsSnapshotPath', '--snapshot-kind=app-jit', - _snippetsExecutable, + path.absolute(_snippetsExecutable), ]..addAll(args), - workingDirectory: _flutterRoot, + workingDirectory: workingDirectory, ); } else { return Process.runSync( - Platform.executable, - [_snippetsSnapshotPath]..addAll(args), - workingDirectory: _flutterRoot, + path.absolute(Platform.executable), + [path.absolute(_snippetsSnapshotPath)]..addAll(args), + workingDirectory: workingDirectory, ); } } diff --git a/dev/docs/assets/overrides.css b/dev/docs/assets/overrides.css index 4aa271620b..981157b7f7 100644 --- a/dev/docs/assets/overrides.css +++ b/dev/docs/assets/overrides.css @@ -137,3 +137,10 @@ footer { font-size: 13px; padding: 12px 20px; } +/* Override the comment color for highlight.js to make it more + prominent/readable */ +.hljs-comment { + color: #128c00; + font-style: italic; + font-weight: bold; +} diff --git a/dev/snippets/config/skeletons/application.html b/dev/snippets/config/skeletons/application.html index 479a8c09c2..347b6e1332 100644 --- a/dev/snippets/config/skeletons/application.html +++ b/dev/snippets/config/skeletons/application.html @@ -15,7 +15,7 @@ onclick="copyTextToClipboard();"> assignment - 
{{code}}
+ 
{{code}}
diff --git a/dev/snippets/config/skeletons/sample.html b/dev/snippets/config/skeletons/sample.html index 9343a01f92..15a98ea571 100644 --- a/dev/snippets/config/skeletons/sample.html +++ b/dev/snippets/config/skeletons/sample.html @@ -1,19 +1,18 @@ {@inject-html} +
+ +
-
- {@end-inject-html} - {{description}} - {@inject-html} +
{@end-inject-html} +{{description}}{@inject-html}
- 
-        {{code}}
-
+ 
{{code}}
diff --git a/dev/snippets/lib/configuration.dart b/dev/snippets/lib/configuration.dart index 3f7f3ebc43..1f401b1bfd 100644 --- a/dev/snippets/lib/configuration.dart +++ b/dev/snippets/lib/configuration.dart @@ -5,7 +5,6 @@ import 'dart:io' hide Platform; import 'package:meta/meta.dart'; -import 'package:platform/platform.dart'; import 'package:path/path.dart' as path; /// What type of snippet to produce. @@ -13,6 +12,7 @@ enum SnippetType { /// Produces a snippet that includes the code interpolated into an application /// template. application, + /// Produces a nicely formatted sample code, but no application. sample, } @@ -27,29 +27,31 @@ String getEnumName(dynamic enumItem) { /// A class to compute the configuration of the snippets input and output /// locations based in the current location of the snippets main.dart. class Configuration { - const Configuration({Platform platform}) : platform = platform ?? const LocalPlatform(); + Configuration({@required this.flutterRoot}) : assert(flutterRoot != null); - final Platform platform; + final Directory flutterRoot; /// This is the configuration directory for the snippets system, containing /// the skeletons and templates. @visibleForTesting - Directory getConfigDirectory(String kind) { - final String platformScriptPath = path.dirname(platform.script.toFilePath()); - final String configPath = - path.canonicalize(path.join(platformScriptPath, '..', 'config', kind)); - return Directory(configPath); + Directory get configDirectory { + _configPath ??= Directory( + path.canonicalize(path.join(flutterRoot.absolute.path, 'dev', 'snippets', 'config'))); + return _configPath; } + Directory _configPath; + /// This is where the snippets themselves will be written, in order to be /// uploaded to the docs site. Directory get outputDirectory { - final String platformScriptPath = path.dirname(platform.script.toFilePath()); - final String docsDirectory = - path.canonicalize(path.join(platformScriptPath, '..', '..', 'docs', 'doc', 'snippets')); - return Directory(docsDirectory); + _docsDirectory ??= Directory( + path.canonicalize(path.join(flutterRoot.absolute.path, 'dev', 'docs', 'doc', 'snippets'))); + return _docsDirectory; } + Directory _docsDirectory; + /// This makes sure that the output directory exists. void createOutputDirectory() { if (!outputDirectory.existsSync()) { @@ -59,11 +61,11 @@ class Configuration { /// The directory containing the HTML skeletons to be filled out with metadata /// and returned to dartdoc for insertion in the output. - Directory get skeletonsDirectory => getConfigDirectory('skeletons'); + Directory get skeletonsDirectory => Directory(path.join(configDirectory.path,'skeletons')); /// The directory containing the code templates that can be referenced by the /// dartdoc. - Directory get templatesDirectory => getConfigDirectory('templates'); + Directory get templatesDirectory => Directory(path.join(configDirectory.path, 'templates')); /// Gets the skeleton file to use for the given [SnippetType]. File getHtmlSkeletonFile(SnippetType type) { diff --git a/dev/snippets/lib/main.dart b/dev/snippets/lib/main.dart index 30b09b1b8e..3f47559205 100644 --- a/dev/snippets/lib/main.dart +++ b/dev/snippets/lib/main.dart @@ -12,12 +12,13 @@ import 'configuration.dart'; import 'snippets.dart'; const String _kElementOption = 'element'; +const String _kHelpOption = 'help'; const String _kInputOption = 'input'; const String _kLibraryOption = 'library'; +const String _kOutputOption = 'output'; const String _kPackageOption = 'package'; const String _kTemplateOption = 'template'; const String _kTypeOption = 'type'; -const String _kOutputOption = 'output'; /// Generates snippet dartdoc output for a given input, and creates any sample /// applications needed by the snippet. @@ -73,9 +74,20 @@ void main(List argList) { defaultsTo: environment['ELEMENT_NAME'], help: 'The name of the element that this snippet belongs to.', ); + parser.addFlag( + _kHelpOption, + defaultsTo: false, + negatable: false, + help: 'Prints help documentation for this command', + ); final ArgResults args = parser.parse(argList); + if (args[_kHelpOption]) { + stderr.writeln(parser.usage); + exit(0); + } + final SnippetType snippetType = SnippetType.values .firstWhere((SnippetType type) => getEnumName(type) == args[_kTypeOption], orElse: () => null); assert(snippetType != null, "Unable to find '${args[_kTypeOption]}' in SnippetType enum."); diff --git a/dev/snippets/lib/snippets.dart b/dev/snippets/lib/snippets.dart index b2616e85b5..482b3b1df8 100644 --- a/dev/snippets/lib/snippets.dart +++ b/dev/snippets/lib/snippets.dart @@ -18,9 +18,10 @@ void errorExit(String message) { // A Tuple containing the name and contents associated with a code block in a // snippet. class _ComponentTuple { - _ComponentTuple(this.name, this.contents); + _ComponentTuple(this.name, this.contents, {String language}) : language = language ?? ''; final String name; final List contents; + final String language; String get mergedContent => contents.join('\n').trim(); } @@ -28,7 +29,9 @@ class _ComponentTuple { /// the output directory. class SnippetGenerator { SnippetGenerator({Configuration configuration}) - : configuration = configuration ?? const Configuration() { + : configuration = configuration ?? + // This script must be run from dev/docs, so the root is up two levels. + Configuration(flutterRoot: Directory(path.canonicalize(path.join('..', '..')))) { this.configuration.createOutputDirectory(); } @@ -95,11 +98,16 @@ class SnippetGenerator { /// if not a [SnippetType.application] snippet. String interpolateSkeleton(SnippetType type, List<_ComponentTuple> injections, String skeleton) { final List result = []; + const HtmlEscape htmlEscape = HtmlEscape(); + String language; for (_ComponentTuple injection in injections) { if (!injection.name.startsWith('code')) { continue; } result.addAll(injection.contents); + if (injection.language.isNotEmpty) { + language = injection.language; + } result.addAll(['', '// ...', '']); } if (result.length > 3) { @@ -109,16 +117,17 @@ class SnippetGenerator { 'description': injections .firstWhere((_ComponentTuple tuple) => tuple.name == 'description') .mergedContent, - 'code': result.join('\n'), + 'code': htmlEscape.convert(result.join('\n')), + 'language': language ?? 'dart', }..addAll(type == SnippetType.application ? { 'id': injections.firstWhere((_ComponentTuple tuple) => tuple.name == 'id').mergedContent, 'app': - injections.firstWhere((_ComponentTuple tuple) => tuple.name == 'app').mergedContent, + htmlEscape.convert(injections.firstWhere((_ComponentTuple tuple) => tuple.name == 'app').mergedContent), } : {'id': '', 'app': ''}); - return skeleton.replaceAllMapped(RegExp(r'{{(code|app|id|description)}}'), (Match match) { + return skeleton.replaceAllMapped(RegExp('{{(${substitutions.keys.join('|')})}}'), (Match match) { return substitutions[match[1]]; }); } @@ -126,31 +135,32 @@ class SnippetGenerator { /// Parses the input for the various code and description segments, and /// returns them in the order found. List<_ComponentTuple> parseInput(String input) { - bool inSnippet = false; + bool inCodeBlock = false; input = input.trim(); final List description = []; final List<_ComponentTuple> components = <_ComponentTuple>[]; - String currentComponent; + String language; + final RegExp codeStartEnd = RegExp(r'^\s*```([-\w]+|[-\w]+ ([-\w]+))?\s*$'); for (String line in input.split('\n')) { - final Match match = RegExp(r'^\s*```(dart|dart (\w+))?\s*$').firstMatch(line); - if (match != null) { - inSnippet = !inSnippet; + final Match match = codeStartEnd.firstMatch(line); + if (match != null) { // If we saw the start or end of a code block + inCodeBlock = !inCodeBlock; if (match[1] != null) { - currentComponent = match[1]; + language = match[1]; if (match[2] != null) { - components.add(_ComponentTuple('code-${match[2]}', [])); + components.add(_ComponentTuple('code-${match[2]}', [], language: language)); } else { - components.add(_ComponentTuple('code', [])); + components.add(_ComponentTuple('code', [], language: language)); } } else { - currentComponent = null; + language = null; } continue; } - if (!inSnippet) { + if (!inCodeBlock) { description.add(line); } else { - assert(currentComponent != null); + assert(language != null); components.last.contents.add(line); } } diff --git a/dev/snippets/test/configuration_test.dart b/dev/snippets/test/configuration_test.dart index 8b2e567743..41e7d2f7df 100644 --- a/dev/snippets/test/configuration_test.dart +++ b/dev/snippets/test/configuration_test.dart @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:platform/platform.dart' show FakePlatform; +import 'dart:io'; import 'package:test/test.dart' hide TypeMatcher, isInstanceOf; @@ -10,36 +10,32 @@ import 'package:snippets/configuration.dart'; void main() { group('Configuration', () { - FakePlatform fakePlatform; Configuration config; setUp(() { - fakePlatform = FakePlatform( - operatingSystem: 'linux', - script: Uri.parse('file:///flutter/dev/snippets/lib/configuration_test.dart')); - config = Configuration(platform: fakePlatform); + config = Configuration(flutterRoot: Directory('/flutter sdk')); }); test('config directory is correct', () async { - expect(config.getConfigDirectory('foo').path, - matches(RegExp(r'[/\\]flutter[/\\]dev[/\\]snippets[/\\]config[/\\]foo'))); + expect(config.configDirectory.path, + matches(RegExp(r'[/\\]flutter sdk[/\\]dev[/\\]snippets[/\\]config'))); }); test('output directory is correct', () async { expect(config.outputDirectory.path, - matches(RegExp(r'[/\\]flutter[/\\]dev[/\\]docs[/\\]doc[/\\]snippets'))); + matches(RegExp(r'[/\\]flutter sdk[/\\]dev[/\\]docs[/\\]doc[/\\]snippets'))); }); test('skeleton directory is correct', () async { expect(config.skeletonsDirectory.path, - matches(RegExp(r'[/\\]flutter[/\\]dev[/\\]snippets[/\\]config[/\\]skeletons'))); + matches(RegExp(r'[/\\]flutter sdk[/\\]dev[/\\]snippets[/\\]config[/\\]skeletons'))); }); test('templates directory is correct', () async { expect(config.templatesDirectory.path, - matches(RegExp(r'[/\\]flutter[/\\]dev[/\\]snippets[/\\]config[/\\]templates'))); + matches(RegExp(r'[/\\]flutter sdk[/\\]dev[/\\]snippets[/\\]config[/\\]templates'))); }); test('html skeleton file is correct', () async { expect( config.getHtmlSkeletonFile(SnippetType.application).path, matches(RegExp( - r'[/\\]flutter[/\\]dev[/\\]snippets[/\\]config[/\\]skeletons[/\\]application.html'))); + r'[/\\]flutter sdk[/\\]dev[/\\]snippets[/\\]config[/\\]skeletons[/\\]application.html'))); }); }); } diff --git a/dev/snippets/test/snippets_test.dart b/dev/snippets/test/snippets_test.dart index a253126f48..f621d6d616 100644 --- a/dev/snippets/test/snippets_test.dart +++ b/dev/snippets/test/snippets_test.dart @@ -5,16 +5,13 @@ import 'dart:io' hide Platform; import 'package:path/path.dart' as path; -import 'package:platform/platform.dart' show FakePlatform; - -import 'package:test_api/test_api.dart' hide TypeMatcher, isInstanceOf; +import 'package:test/test.dart' hide TypeMatcher, isInstanceOf; import 'package:snippets/configuration.dart'; import 'package:snippets/snippets.dart'; void main() { group('Generator', () { - FakePlatform fakePlatform; Configuration configuration; SnippetGenerator generator; Directory tmpDir; @@ -22,10 +19,8 @@ void main() { setUp(() { tmpDir = Directory.systemTemp.createTempSync('snippets_test'); - fakePlatform = FakePlatform( - script: Uri.file(path.join( - tmpDir.absolute.path, 'flutter', 'dev', 'snippets', 'lib', 'snippets_test.dart'))); - configuration = Configuration(platform: fakePlatform); + configuration = Configuration(flutterRoot: Directory(path.join( + tmpDir.absolute.path, 'flutter'))); configuration.createOutputDirectory(); configuration.templatesDirectory.createSync(recursive: true); configuration.skeletonsDirectory.createSync(recursive: true); @@ -67,7 +62,7 @@ A description of the snippet. On several lines. -```dart preamble +```my-dart_language my-preamble const String name = 'snippet'; ``` @@ -82,13 +77,13 @@ void main() { generator.generate(inputFile, SnippetType.application, template: 'template', id: 'id'); expect(html, contains('
HTML Bits
')); expect(html, contains('
More HTML Bits
')); - expect(html, contains("print('The actual \$name.');")); + expect(html, contains('print('The actual \$name.');')); expect(html, contains('A description of the snippet.\n')); expect( html, - contains('// A description of the snippet.\n' - '//\n' - '// On several lines.\n')); + contains('// A description of the snippet.\n' + '//\n' + '// On several lines.\n')); expect(html, contains('void main() {')); }); @@ -110,7 +105,7 @@ void main() { final String html = generator.generate(inputFile, SnippetType.sample); expect(html, contains('
HTML Bits
')); expect(html, contains('
More HTML Bits
')); - expect(html, contains("print('The actual \$name.');")); + expect(html, contains(' print('The actual \$name.');')); expect(html, contains('A description of the snippet.\n\nOn several lines.\n')); expect(html, contains('main() {')); }); diff --git a/dev/tools/dartdoc.dart b/dev/tools/dartdoc.dart index cda9793ed3..752ccd28f4 100644 --- a/dev/tools/dartdoc.dart +++ b/dev/tools/dartdoc.dart @@ -99,6 +99,7 @@ Future main(List arguments) async { createFooter('$kDocsRoot/lib/footer.html'); copyAssets(); cleanOutSnippets(); + precompileSnippetsTool(); final List dartdocBaseArgs = ['global', 'run']; if (args['checked']) { @@ -299,6 +300,34 @@ void cleanOutSnippets() { } } +File precompileSnippetsTool() { + final File snapshotPath = File(path.join('bin', 'cache', 'snippets.snapshot')); + print('Precompiling snippets tool into ${snapshotPath.absolute.path}'); + if (snapshotPath.existsSync()) { + snapshotPath.deleteSync(); + } + // In order to be able to optimize properly, we need to provide a training set + // of arguments, and an input file to process. + final Directory tempDir = Directory.systemTemp.createTempSync('dartdoc_snippet_'); + final File trainingFile = File(path.join(tempDir.path, 'snippet_training')); + trainingFile.writeAsStringSync('```dart\nvoid foo(){}\n```'); + Process.runSync(Platform.resolvedExecutable, [ + '--snapshot=${snapshotPath.absolute.path}', + '--snapshot_kind=app-jit', + path.join( + 'dev', + 'snippets', + 'lib', + 'main.dart', + ), + '--type=sample', + '--input=${trainingFile.absolute.path}', + '--output=${path.join(tempDir.absolute.path, 'training_output.txt')}', + ]); + tempDir.deleteSync(recursive: true); + return snapshotPath; +} + void sanityCheckDocs() { final List canaries = [ '$kPublishRoot/assets/overrides.css',