From 262f12b4a9269a77bce499926afa681b20790d6c Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Fri, 15 Feb 2019 07:48:49 -0800 Subject: [PATCH] Remove remaining "## Sample code" segments, and fix the snippet generator. (#27793) This converts all remaining "## Sample code" segments into snippets, and fixes the snippet generator to handle multiple snippets in the same dartdoc block properly. I also generated, compiled, and ran each of the existing application samples, and fixed them up to be more useful and/or just run without errors. This PR fixes these problems with examples: 1. Switching tabs in a snippet now works if there is more than one snippet in a single dartdoc block. 2. Generation of snippet code now works if there is more than one snippet. 3. Contrast of text and links in the code sample block has been improved to recommended levels. 4. Added five new snippet templates, including a "freeform" template to make it possible to show examples that need to change the app instantiation. 5. Fixed several examples to run properly, a couple by adding the "Scaffold" widget to the template, a couple by just fixing their code. 6. Fixed visual look of some of the samples when they run by placing many samples inside of a Scaffold. 7. In order to make it easier to run locally, changed the sample analyzer to remove the contents of the supplied temp directory before running, since having files that hang around is problematic (only a problem when running locally with the `--temp` argument). 8. Added a `SampleCheckerException` class, and handle sample checking exceptions more gracefully. 9. Deprecated the old "## Sample code" designation, and added enforcement for the deprecation. 10. Removed unnecessary `new` from templates (although they never appeared in the samples thanks to dartfmt, but still). Fixes #26398 Fixes #27411 --- dev/bots/analyze-sample-code.dart | 178 +++++++++++------- .../known_broken_documentation.dart | 8 +- dev/bots/test/analyze-sample-code_test.dart | 4 +- dev/docs/assets/snippets.css | 21 ++- dev/docs/assets/snippets.js | 14 +- dev/snippets/README.md | 4 +- .../config/skeletons/application.html | 16 +- dev/snippets/config/skeletons/sample.html | 2 +- dev/snippets/config/templates/README.md | 27 ++- dev/snippets/config/templates/freeform.tmpl | 11 ++ .../config/templates/stateful_widget.tmpl | 20 +- .../templates/stateful_widget_material.tmpl | 35 ++++ .../templates/stateful_widget_scaffold.tmpl | 38 ++++ .../config/templates/stateless_widget.tmpl | 23 +-- .../templates/stateless_widget_material.tmpl | 33 ++++ .../templates/stateless_widget_scaffold.tmpl | 36 ++++ dev/snippets/lib/main.dart | 15 +- dev/snippets/lib/snippets.dart | 26 ++- dev/tools/gen_keycodes/data/keyboard_key.tmpl | 4 +- .../flutter/lib/src/material/app_bar.dart | 33 ++-- .../src/material/bottom_navigation_bar.dart | 73 ++++--- packages/flutter/lib/src/material/card.dart | 82 ++++---- packages/flutter/lib/src/material/chip.dart | 4 +- .../flutter/lib/src/material/dropdown.dart | 2 +- .../flutter/lib/src/material/icon_button.dart | 34 ++-- .../lib/src/material/raised_button.dart | 62 +++--- .../flutter/lib/src/material/scaffold.dart | 92 ++++++--- .../flutter/lib/src/material/stepper.dart | 67 +++---- .../flutter/lib/src/rendering/binding.dart | 17 +- .../flutter/lib/src/semantics/semantics.dart | 31 ++- .../lib/src/services/keyboard_key.dart | 4 +- .../lib/src/services/system_chrome.dart | 69 ++++--- packages/flutter/lib/src/widgets/basic.dart | 49 +++-- .../flutter/lib/src/widgets/navigator.dart | 146 ++++++++++---- .../src/widgets/single_child_scroll_view.dart | 141 +++++++------- 35 files changed, 909 insertions(+), 512 deletions(-) create mode 100644 dev/snippets/config/templates/freeform.tmpl create mode 100644 dev/snippets/config/templates/stateful_widget_material.tmpl create mode 100644 dev/snippets/config/templates/stateful_widget_scaffold.tmpl create mode 100644 dev/snippets/config/templates/stateless_widget_material.tmpl create mode 100644 dev/snippets/config/templates/stateless_widget_scaffold.tmpl diff --git a/dev/bots/analyze-sample-code.dart b/dev/bots/analyze-sample-code.dart index a749dd2c71..6af14703f9 100644 --- a/dev/bots/analyze-sample-code.dart +++ b/dev/bots/analyze-sample-code.dart @@ -87,12 +87,42 @@ void main(List arguments) { Directory tempDirectory; if (parsedArguments.wasParsed('temp')) { - tempDirectory = Directory(parsedArguments['temp']); - if (!tempDirectory.existsSync()) { - tempDirectory.createSync(recursive: true); + tempDirectory = Directory(path.join(Directory.systemTemp.absolute.path, path.basename(parsedArguments['temp']))); + if (path.basename(parsedArguments['temp']) != parsedArguments['temp']) { + stderr.writeln('Supplied temporary directory name should be a name, not a path. Using ${tempDirectory.absolute.path} instead.'); + } + print('Leaving temporary output in ${tempDirectory.absolute.path}.'); + // Make sure that any directory left around from a previous run is cleared + // out. + if (tempDirectory.existsSync()) { + tempDirectory.deleteSync(recursive: true); + } + tempDirectory.createSync(); + } + try { + exitCode = SampleChecker(flutterPackage, tempDirectory: tempDirectory).checkSamples(); + } on SampleCheckerException catch (e) { + stderr.write(e); + exit(1); + } +} + +class SampleCheckerException implements Exception { + SampleCheckerException(this.message, {this.file, this.line}); + final String message; + final String file; + final int line; + + @override + String toString() { + if (file != null || line != null) { + final String fileStr = file == null ? '' : '$file:'; + final String lineStr = line == null ? '' : '$line:'; + return '$fileStr$lineStr Error: $message'; + } else { + return 'Error: $message'; } } - exitCode = SampleChecker(flutterPackage, tempDirectory: tempDirectory).checkSamples(); } /// Checks samples and code snippets for analysis errors. @@ -129,10 +159,10 @@ class SampleChecker { static final RegExp _dartDocSampleEndRegex = RegExp(r'{@end-tool}'); /// A RegExp that matches the start of a code block within dartdoc. - static final RegExp _codeBlockStartRegex = RegExp(r'/// ```dart.*$'); + static final RegExp _codeBlockStartRegex = RegExp(r'///\s+```dart.*$'); /// A RegExp that matches the end of a code block within dartdoc. - static final RegExp _codeBlockEndRegex = RegExp(r'/// ```\s*$'); + static final RegExp _codeBlockEndRegex = RegExp(r'///\s+```\s*$'); /// A RegExp that matches a Dart constructor. static final RegExp _constructorRegExp = RegExp(r'[A-Z][a-zA-Z0-9<>.]*\('); @@ -173,10 +203,7 @@ class SampleChecker { } static List _listDartFiles(Directory directory, {bool recursive = false}) { - return directory.listSync(recursive: recursive, followLinks: false) - .whereType() - .where((File file) => path.extension(file.path) == '.dart') - .toList(); + return directory.listSync(recursive: recursive, followLinks: false).whereType().where((File file) => path.extension(file.path) == '.dart').toList(); } /// Computes the headers needed for each sample file. @@ -218,14 +245,14 @@ class SampleChecker { } stderr.writeln('\nFound ${errors.length} sample code errors.'); } - if (!_keepTmp) { + if (_keepTmp) { + print('Leaving temporary directory ${_tempDirectory.path} around for your perusal.'); + } else { try { _tempDirectory.deleteSync(recursive: true); } on FileSystemException catch (e) { stderr.writeln('Failed to delete ${_tempDirectory.path}: $e'); } - } else { - print('Leaving temporary directory ${_tempDirectory.path} around for your perusal.'); } // If we made a snapshot, remove it (so as not to clutter up the tree). if (_snippetsSnapshotPath != null) { @@ -288,8 +315,12 @@ class SampleChecker { print('Generating snippet for ${snippet.start?.filename}:${snippet.start?.line}'); final ProcessResult process = _runSnippetsScript(args); if (process.exitCode != 0) { - throw 'Unable to create snippet for ${snippet.start.filename}:${snippet.start.line} ' - '(using input from ${inputFile.path}):\n${process.stdout}\n${process.stderr}'; + throw SampleCheckerException( + 'Unable to create snippet for ${snippet.start.filename}:${snippet.start.line} ' + '(using input from ${inputFile.path}):\n${process.stdout}\n${process.stderr}', + file: snippet.start.filename, + line: snippet.start.line, + ); } return outputFile; } @@ -304,11 +335,14 @@ class SampleChecker { final String relativeFilePath = path.relative(file.path, from: _flutterPackage.path); final List sampleLines = file.readAsLinesSync(); final List
preambleSections =
[]; + // Whether or not we're in the file-wide preamble section ("Examples can assume"). bool inPreamble = false; + // Whether or not we're in a code sample bool inSampleSection = false; + // Whether or not we're in a snippet code sample (with template) specifically. bool inSnippet = false; + // Whether or not we're in a '```dart' segment. bool inDart = false; - bool foundDart = false; int lineNumber = 0; final List block = []; List snippetArgs = []; @@ -318,7 +352,7 @@ class SampleChecker { final String trimmedLine = line.trim(); if (inSnippet) { if (!trimmedLine.startsWith(_dartDocPrefix)) { - throw '$relativeFilePath:$lineNumber: Snippet section unterminated.'; + throw SampleCheckerException('Snippet section unterminated.', file: relativeFilePath, line: lineNumber); } if (_dartDocSampleEndRegex.hasMatch(trimmedLine)) { snippets.add( @@ -342,53 +376,51 @@ class SampleChecker { preambleSections.add(_processBlock(startLine, block)); block.clear(); } else if (!line.startsWith('// ')) { - throw '$relativeFilePath:$lineNumber: Unexpected content in sample code preamble.'; + throw SampleCheckerException('Unexpected content in sample code preamble.', file: relativeFilePath, line: lineNumber); } else { block.add(line.substring(3)); } } else if (inSampleSection) { - if (!trimmedLine.startsWith(_dartDocPrefix) || trimmedLine.startsWith('$_dartDocPrefix ## ')) { + if (_dartDocSampleEndRegex.hasMatch(trimmedLine)) { if (inDart) { - throw '$relativeFilePath:$lineNumber: Dart section inexplicably unterminated.'; - } - if (!foundDart) { - throw '$relativeFilePath:$lineNumber: No dart block found in sample code section'; + throw SampleCheckerException("Dart section didn't terminate before end of sample", file: relativeFilePath, line: lineNumber); } inSampleSection = false; - } else { - if (inDart) { - if (_codeBlockEndRegex.hasMatch(trimmedLine)) { - inDart = false; - final Section processed = _processBlock(startLine, block); - if (preambleSections.isEmpty) { - sections.add(processed); - } else { - sections.add(Section.combine(preambleSections - ..toList() - ..add(processed))); - } - block.clear(); - } else if (trimmedLine == _dartDocPrefix) { - block.add(''); + } + if (inDart) { + if (_codeBlockEndRegex.hasMatch(trimmedLine)) { + inDart = false; + final Section processed = _processBlock(startLine, block); + if (preambleSections.isEmpty) { + sections.add(processed); } else { - final int index = line.indexOf(_dartDocPrefixWithSpace); - if (index < 0) { - throw '$relativeFilePath:$lineNumber: Dart section inexplicably did not ' - 'contain "$_dartDocPrefixWithSpace" prefix.'; - } - block.add(line.substring(index + 4)); + sections.add(Section.combine(preambleSections + ..toList() + ..add(processed))); } - } else if (_codeBlockStartRegex.hasMatch(trimmedLine)) { - assert(block.isEmpty); - startLine = Line( - '', - filename: relativeFilePath, - line: lineNumber + 1, - indent: line.indexOf(_dartDocPrefixWithSpace) + _dartDocPrefixWithSpace.length, - ); - inDart = true; - foundDart = true; + block.clear(); + } else if (trimmedLine == _dartDocPrefix) { + block.add(''); + } else { + final int index = line.indexOf(_dartDocPrefixWithSpace); + if (index < 0) { + throw SampleCheckerException( + 'Dart section inexplicably did not contain "$_dartDocPrefixWithSpace" prefix.', + file: relativeFilePath, + line: lineNumber, + ); + } + block.add(line.substring(index + 4)); } + } else if (_codeBlockStartRegex.hasMatch(trimmedLine)) { + assert(block.isEmpty); + startLine = Line( + '', + filename: relativeFilePath, + line: lineNumber + 1, + indent: line.indexOf(_dartDocPrefixWithSpace) + _dartDocPrefixWithSpace.length, + ); + inDart = true; } } if (!inSampleSection) { @@ -397,11 +429,7 @@ class SampleChecker { assert(block.isEmpty); startLine = Line('', filename: relativeFilePath, line: lineNumber + 1, indent: 3); inPreamble = true; - } else if (trimmedLine == '/// ## Sample code' || - trimmedLine.startsWith('/// ## Sample code:') || - trimmedLine == '/// ### Sample code' || - trimmedLine.startsWith('/// ### Sample code:') || - sampleMatch != null) { + } else if (sampleMatch != null) { inSnippet = sampleMatch != null ? sampleMatch[1] == 'snippet' : false; if (inSnippet) { startLine = Line( @@ -418,7 +446,12 @@ class SampleChecker { } } inSampleSection = !inSnippet; - foundDart = false; + } else if (RegExp(r'///\s*#+\s+[Ss]ample\s+[Cc]ode:?$').hasMatch(trimmedLine)) { + throw SampleCheckerException( + "Found deprecated '## Sample code' section: use {@tool sample}...{@end-tool} instead.", + file: relativeFilePath, + line: lineNumber, + ); } } } @@ -598,14 +631,17 @@ linter: ), ), ); - throw 'Cannot analyze dartdocs; analysis errors exist in ${file.path}: $error'; + throw SampleCheckerException( + 'Cannot analyze dartdocs; analysis errors exist: $error', + file: file.path, + line: lineNumber, + ); } if (errorCode == 'unused_element' || errorCode == 'unused_local_variable') { // We don't really care if sample code isn't used! continue; } - if (isSnippet) { addAnalysisError( file, @@ -630,14 +666,20 @@ linter: Line('', filename: file.path, line: lineNumber), ), ); - throw 'Failed to parse error message (read line number as $lineNumber; ' - 'total number of lines is ${fileContents.length}): $error'; + throw SampleCheckerException('Failed to parse error message: $error', file: file.path, line: lineNumber); } final Section actualSection = sections[file.path]; + if (actualSection == null) { + throw SampleCheckerException( + "Unknown section for ${file.path}. Maybe the temporary directory wasn't empty?", + file: file.path, + line: lineNumber, + ); + } final Line actualLine = actualSection.code[lineNumber - 1]; - if (actualLine.filename == null) { + if (actualLine?.filename == null) { if (errorCode == 'missing_identifier' && lineNumber > 1) { if (fileContents[lineNumber - 2].endsWith(',')) { final Line actualLine = sections[file.path].code[lineNumber - 2]; @@ -698,7 +740,7 @@ linter: /// into valid Dart code. Section _processBlock(Line line, List block) { if (block.isEmpty) { - throw '$line: Empty ```dart block in sample code.'; + throw SampleCheckerException('$line: Empty ```dart block in sample code.'); } if (block.first.startsWith('new ') || block.first.startsWith('const ') || block.first.startsWith(_constructorRegExp)) { _expressionId += 1; @@ -721,8 +763,8 @@ linter: // treated as a separate code block. if (block[index] == '' || block[index] == '// ...') { if (subline == null) - throw '${Line('', filename: line.filename, line: line.line + index, indent: line.indent)}: ' - 'Unexpected blank line or "// ..." line near start of subblock in sample code.'; + throw SampleCheckerException('${Line('', filename: line.filename, line: line.line + index, indent: line.indent)}: ' + 'Unexpected blank line or "// ..." line near start of subblock in sample code.'); subblocks += 1; subsections.add(_processBlock(subline, buffer)); buffer.clear(); diff --git a/dev/bots/test/analyze-sample-code-test-input/known_broken_documentation.dart b/dev/bots/test/analyze-sample-code-test-input/known_broken_documentation.dart index e624931f1e..6a00c15f9a 100644 --- a/dev/bots/test/analyze-sample-code-test-input/known_broken_documentation.dart +++ b/dev/bots/test/analyze-sample-code-test-input/known_broken_documentation.dart @@ -18,8 +18,7 @@ /// blabla 0.0, the penzance blabla is blabla not blabla at all. Bla the blabla /// 1.0, the blabla is blabla blabla blabla an blabla blabla. /// -/// ### Sample code -/// +/// {@tool sample} /// Bla blabla blabla some [Text] when the `_blabla` blabla blabla is true, and /// blabla it when it is blabla: /// @@ -29,9 +28,9 @@ /// child: const Text('Poor wandering ones!'), /// ) /// ``` +/// {@end-tool} /// -/// ## Sample code -/// +/// {@tool sample} /// Bla blabla blabla some [Text] when the `_blabla` blabla blabla is true, and /// blabla finale blabla: /// @@ -41,3 +40,4 @@ /// child: const Text('Poor wandering ones!'), /// ) /// ``` +/// {@end-tool} diff --git a/dev/bots/test/analyze-sample-code_test.dart b/dev/bots/test/analyze-sample-code_test.dart index 8d3d883cc9..0d46585a6d 100644 --- a/dev/bots/test/analyze-sample-code_test.dart +++ b/dev/bots/test/analyze-sample-code_test.dart @@ -17,9 +17,9 @@ void main() { ..removeWhere((String line) => line.startsWith('Analyzer output:')); expect(process.exitCode, isNot(equals(0))); expect(stderrLines, [ - 'known_broken_documentation.dart:27:9: new Opacity(', + 'known_broken_documentation.dart:26:9: new Opacity(', '>>> Unnecessary new keyword (unnecessary_new)', - 'known_broken_documentation.dart:39:9: new Opacity(', + 'known_broken_documentation.dart:38:9: new Opacity(', '>>> Unnecessary new keyword (unnecessary_new)', '', 'Found 1 sample code errors.', diff --git a/dev/docs/assets/snippets.css b/dev/docs/assets/snippets.css index 65a7d338cb..4fb200addb 100644 --- a/dev/docs/assets/snippets.css +++ b/dev/docs/assets/snippets.css @@ -1,6 +1,6 @@ /* Styles for handling custom code snippets */ .snippet-container { - background-color: #45aae8; + background-color: #2372a3; padding: 10px; overflow: auto; } @@ -30,8 +30,21 @@ color: white; } +.snippet-description a:link { + color: #c7fcf4; +} +.snippet-description a:visited { + color: #c7dbfc; +} +.snippet-description a:hover { + color: white; +} +.snippet-description a:active { + color: #80b0fc; +} + .snippet-buttons button { - background-color: #45aae8; + background-color: #2372a3; border-style: none; color: white; padding: 10px 24px; @@ -82,7 +95,7 @@ height: 28px; width: 28px; transition: .3s ease; - background-color: #45aae8; + background-color: #2372a3; } .copy-button { @@ -102,7 +115,7 @@ .copy-image { opacity: 0.65; - color: #45aae8; + color: #2372a3; font-size: 28px; padding-top: 4px; } diff --git a/dev/docs/assets/snippets.js b/dev/docs/assets/snippets.js index b51c96e91c..9d4da921da 100644 --- a/dev/docs/assets/snippets.js +++ b/dev/docs/assets/snippets.js @@ -2,15 +2,10 @@ * Scripting for handling custom code snippets */ -const shortSnippet = 'shortSnippet'; -const longSnippet = 'longSnippet'; -var visibleSnippet = shortSnippet; - /** - * Shows the requested snippet. Values for "name" can be "shortSnippet" or - * "longSnippet". + * Shows the requested snippet, and stores the current state in visibleSnippet. */ -function showSnippet(name) { +function showSnippet(name, visibleSnippet) { if (visibleSnippet == name) return; if (visibleSnippet != null) { var shown = document.getElementById(visibleSnippet); @@ -39,6 +34,7 @@ function showSnippet(name) { if (button != null) { button.setAttributeNode(selectedAttribute); } + return visibleSnippet; } // Finds a sibling to given element with the given id. @@ -64,8 +60,8 @@ function supportsCopying() { // Copies the text inside the currently visible snippet to the clipboard, or the // given element, if any. function copyTextToClipboard(element) { - if (element == null) { - var elementSelector = '#' + visibleSnippet + ' .language-dart'; + if (typeof element === 'string') { + var elementSelector = '#' + element + ' .language-dart'; element = document.querySelector(elementSelector); if (element == null) { console.log( diff --git a/dev/snippets/README.md b/dev/snippets/README.md index 0606877c7b..a1e7875ade 100644 --- a/dev/snippets/README.md +++ b/dev/snippets/README.md @@ -7,8 +7,9 @@ snippets. This takes code in dartdocs, like this: ```dart -/// The following is a skeleton of a stateless widget subclass called `GreenFrog`: /// {@tool snippet --template="stateless_widget"} +/// The following is a skeleton of a stateless widget subclass called `GreenFrog`. +/// ```dart /// class GreenFrog extends StatelessWidget { /// const GreenFrog({ Key key }) : super(key: key); /// @@ -17,6 +18,7 @@ This takes code in dartdocs, like this: /// return Container(color: const Color(0xFF2DBD3A)); /// } /// } +/// ``` /// {@end-tool} ``` diff --git a/dev/snippets/config/skeletons/application.html b/dev/snippets/config/skeletons/application.html index afe9c4c3d5..7d7e17bab4 100644 --- a/dev/snippets/config/skeletons/application.html +++ b/dev/snippets/config/skeletons/application.html @@ -1,26 +1,30 @@ {@inject-html}
- - + + +
-
+
{{description}} 
{{code}}
-