[H] Cleanup (#21542)

* Improve documentation and clean up code.

* Remove "Note that".

The phrase "note that" is basically meaningless as a prefix to an
otherwise fine sentence.

dev/bots/README.md

@@ -151,14 +151,17 @@ components need to be updated or installed, follow the steps below:
 
 ## Flutter codelabs build test
 
-The Flutter codelabs exercise Material Components in the form of a demo application. Note that the 
+The Flutter codelabs exercise Material Components in the form of a
-code for the codelabs is similar to but distinct from the code for the Shrine demo app in Flutter Gallery.
+demo application. The code for the codelabs is similar to, but
-The Flutter codelabs build test ensures that the final version of
+distinct from, the code for the Shrine demo app in Flutter Gallery.
-[Material Components for Flutter Codelabs](https://github.com/material-components/material-components-flutter-codelabs)
+
-can be built. This test serves as a smoke test for the Flutter framework and should not fail. Please
+The Flutter codelabs build test ensures that the final version of the
-address the issue from within your PR and rerun the test. If you feel that the test failing is not a
+[Material Components for Flutter
-direct result of changes made in your PR or that breaking this test is absolutely necessary, escalate this issue by
+Codelabs](https://github.com/material-components/material-components-flutter-codelabs)
-[submitting an issue](https://github.com/material-components/material-components-flutter-codelabs/issues/new?title=%5BURGENT%5D%20Flutter%20Framework%20breaking%20PR)
+can be built. This test serves as a smoke test for the Flutter
+framework and should not fail. Please address the issue from within
+your PR and rerun the test. If you feel that the test failing is not a
+direct result of changes made in your PR or that breaking this test is
+absolutely necessary, escalate this issue by [submitting an
+issue](https://github.com/material-components/material-components-flutter-codelabs/issues/new?title=%5BURGENT%5D%20Flutter%20Framework%20breaking%20PR)
 to the MDC-Flutter Team.
-
-

dev/bots/prepare_package.dart

@@ -567,8 +567,8 @@ class ArchivePublisher {
 /// packages, and the flutter cache in bin/cache with the appropriate
 /// dependencies and snapshots.
 ///
-/// Note that archives contain the executables and customizations for the
+/// Archives contain the executables and customizations for the platform that
-/// platform that they are created on.
+/// they are created on.
 Future<Null> main(List<String> argList) async {
   final ArgParser argParser = ArgParser();
   argParser.addOption(

dev/bots/test/fake_process_manager.dart

@@ -81,7 +81,7 @@ class FakeProcessManager extends Mock implements ProcessManager {
   }
 
   void _setupMock() {
-    // Note that not all possible types of invocations are covered here, just the ones
+    // Not all possible types of invocations are covered here, just the ones
     // expected to be called.
     // TODO(gspencer): make this more general so that any call will be captured.
     when(start(

examples/flutter_gallery/lib/demo/calculator/logic.dart

@@ -112,11 +112,11 @@ enum ExpressionState {
 
 /// An expression that can be displayed in a calculator. It is the result
 /// of a sequence of user entries. It is represented by a sequence of tokens.
-/// Note that the tokens are not in one to one correspondence with the
+///
-/// key taps because we use one token per number, not one token per digit.
+/// The tokens are not in one to one correspondence with the key taps because we
-/// A CalcExpression is immutable. The append* methods return a new
+/// use one token per number, not one token per digit. A [CalcExpression] is
-/// CalcExpression that represents the appropriate expression when one
+/// immutable. The `append*` methods return a new [CalcExpression] that
-/// additional key tap occurs.
+/// represents the appropriate expression when one additional key tap occurs.
 class CalcExpression {
   CalcExpression(this._list, this.state);
 

packages/flutter/lib/src/material/dialog.dart

@@ -101,9 +101,13 @@ class Dialog extends StatelessWidget {
 /// displayed below the content.
 ///
 /// If the content is too large to fit on the screen vertically, the dialog will
-/// display the title and the actions and let the content overflow. Consider
+/// display the title and the actions and let the content overflow, which is
-/// using a scrolling widget, such as [ListView], for [content] to avoid
+/// rarely desired. Consider using a scrolling widget for [content], such as
-/// overflow.
+/// [SingleChildScrollView], to avoid overflow. (However, be aware that since
+/// [AlertDialog] tries to size itself using the intrinsic dimensions of its
+/// children, widgets such as [ListView], [GridView], and [CustomScrollView],
+/// which use lazy viewports, will not work. If this is a problem, consider
+/// using [Dialog] directly.)
 ///
 /// For dialogs that offer the user a choice between several options, consider
 /// using a [SimpleDialog].

packages/flutter/lib/src/material/progress_indicator.dart

@@ -463,7 +463,7 @@ class _RefreshProgressIndicatorPainter extends _CircularProgressIndicatorPainter
 
   void paintArrowhead(Canvas canvas, Size size) {
     // ux, uy: a unit vector whose direction parallels the base of the arrowhead.
-    // Note that ux, -uy points in the direction the arrowhead points.
+    // (So ux, -uy points in the direction the arrowhead points.)
     final double arcEnd = arcStart + arcSweep;
     final double ux = math.cos(arcEnd);
     final double uy = math.sin(arcEnd);

packages/flutter/lib/src/material/reorderable_list.dart

@@ -11,23 +11,28 @@ import 'debug.dart';
 import 'material.dart';
 import 'material_localizations.dart';
 
+// Examples can assume:
+// class MyDataObject { }
+
 /// The callback used by [ReorderableListView] to move an item to a new
 /// position in a list.
 ///
 /// Implementations should remove the corresponding list item at [oldIndex]
 /// and reinsert it at [newIndex].
 ///
-/// Note that if [oldIndex] is before [newIndex], removing the item at [oldIndex]
+/// If [oldIndex] is before [newIndex], removing the item at [oldIndex] from the
-/// from the list will reduce the list's length by one. Implementations used
+/// list will reduce the list's length by one. Implementations used by
-/// by [ReorderableListView] will need to account for this when inserting before
+/// [ReorderableListView] will need to account for this when inserting before
 /// [newIndex].
 ///
+/// ## Sample code
+///
 /// Example implementation:
 ///
 /// ```dart
 /// final List<MyDataObject> backingList = <MyDataObject>[/* ... */];
 ///
-/// void onReorder(int oldIndex, int newIndex) {
+/// void handleReorder(int oldIndex, int newIndex) {
 ///   if (oldIndex < newIndex) {
 ///     // removing the item at oldIndex will shorten the list by 1.
 ///     newIndex -= 1;
@@ -36,7 +41,7 @@ import 'material_localizations.dart';
 ///   backingList.insert(newIndex, element);
 /// }
 /// ```
-typedef OnReorderCallback = void Function(int oldIndex, int newIndex);
+typedef ReorderCallback = void Function(int oldIndex, int newIndex);
 
 /// A list whose items the user can interactively reorder by dragging.
 ///
@@ -84,7 +89,7 @@ class ReorderableListView extends StatefulWidget {
   ///
   /// This [ReorderableListView] calls [onReorder] after a list child is dropped
   /// into a new position.
-  final OnReorderCallback onReorder;
+  final ReorderCallback onReorder;
 
   @override
   _ReorderableListViewState createState() => _ReorderableListViewState();
@@ -148,7 +153,7 @@ class _ReorderableListContent extends StatefulWidget {
   final List<Widget> children;
   final Axis scrollDirection;
   final EdgeInsets padding;
-  final OnReorderCallback onReorder;
+  final ReorderCallback onReorder;
 
   @override
   _ReorderableListContentState createState() => _ReorderableListContentState();

packages/flutter/lib/src/material/scaffold.dart

@@ -83,9 +83,10 @@ class ScaffoldPrelayoutGeometry {
   /// keeping it above the [BottomSheet], the [Scaffold.bottomNavigationBar],
   /// or the keyboard.
   ///
-  /// Note that [Scaffold.body] is laid out with respect to [minInsets] already.
+  /// The [Scaffold.body] is laid out with respect to [minInsets] already. This
-  /// This means that a [FloatingActionButtonLocation] does not need to factor
+  /// means that a [FloatingActionButtonLocation] does not need to factor in
-  /// in [minInsets.bottom] when aligning a [FloatingActionButton] to [contentBottom].
+  /// [minInsets.bottom] when aligning a [FloatingActionButton] to
+  /// [contentBottom].
   final double contentBottom;
 
   /// The vertical distance from the [Scaffold]'s origin to the top of
@@ -95,9 +96,9 @@ class ScaffoldPrelayoutGeometry {
   /// place the [FloatingActionButton] at the top of the screen, while
   /// keeping it below the [Scaffold.appBar].
   ///
-  /// Note that [Scaffold.body] is laid out with respect to [minInsets] already.
+  /// The [Scaffold.body] is laid out with respect to [minInsets] already. This
-  /// This means that a [FloatingActionButtonLocation] does not need to factor
+  /// means that a [FloatingActionButtonLocation] does not need to factor in
-  /// in [minInsets.top] when aligning a [FloatingActionButton] to [contentTop].
+  /// [minInsets.top] when aligning a [FloatingActionButton] to [contentTop].
   final double contentTop;
 
   /// The minimum padding to inset the [FloatingActionButton] by for it

packages/flutter/lib/src/painting/image_provider.dart

@@ -698,8 +698,7 @@ class MemoryImage extends ImageProvider<MemoryImage> {
 ///    - packages/fancy_backgrounds/backgrounds/background1.png
 /// ```
 ///
-/// Note that the `lib/` is implied, so it should not be included in the asset
+/// The `lib/` is implied, so it should not be included in the asset path.
-/// path.
 ///
 /// See also:
 ///

packages/flutter/lib/src/painting/image_resolution.dart

@@ -115,8 +115,7 @@ const String _kAssetManifestFileName = 'AssetManifest.json';
 ///    - packages/fancy_backgrounds/backgrounds/background1.png
 /// ```
 ///
-/// Note that the `lib/` is implied, so it should not be included in the asset
+/// The `lib/` is implied, so it should not be included in the asset path.
-/// path.
 ///
 /// See also:
 ///

packages/flutter/lib/src/widgets/automatic_keep_alive.dart

@@ -323,14 +323,19 @@ class KeepAliveHandle extends ChangeNotifier {
   }
 }
 
-/// A mixin with convenience methods for clients of [AutomaticKeepAlive].
+/// A mixin with convenience methods for clients of [AutomaticKeepAlive]. Used
+/// with [State] subclasses.
 ///
 /// Subclasses must implement [wantKeepAlive], and their [build] methods must
-/// call `super.build` (which will always return null).
+/// call `super.build` (the return value will always return null, and should be
+/// ignored).
 ///
 /// Then, whenever [wantKeepAlive]'s value changes (or might change), the
 /// subclass should call [updateKeepAlive].
 ///
+/// The type argument `T` is the type of the [StatefulWidget] subclass of the
+/// [State] into which this class is being mixed.
+///
 /// See also:
 ///
 ///  * [AutomaticKeepAlive], which listens to messages from this mixin.

packages/flutter/lib/src/widgets/media_query.dart

@@ -315,8 +315,8 @@ class MediaQueryData {
   String toString() {
     return '$runtimeType('
              'size: $size, '
-             'devicePixelRatio: $devicePixelRatio, '
+             'devicePixelRatio: ${devicePixelRatio.toStringAsFixed(1)}, '
-             'textScaleFactor: $textScaleFactor, '
+             'textScaleFactor: ${textScaleFactor.toStringAsFixed(1)}, '
              'padding: $padding, '
              'viewInsets: $viewInsets, '
              'alwaysUse24HourFormat: $alwaysUse24HourFormat, '

packages/flutter/lib/src/widgets/sliver.dart

@@ -1009,6 +1009,21 @@ class SliverFillRemaining extends SingleChildRenderObjectWidget {
 ///
 /// This widget is for use in [SliverMultiBoxAdaptorWidget]s, such as
 /// [SliverGrid] or [SliverList].
+///
+/// This widget is rarely used directly. The [SliverChildBuilderDelegate] and
+/// [SliverChildListDelegate] delegates, used with [SliverList] and
+/// [SliverGrid], as well as the scroll view counterparts [ListView] and
+/// [GridView], have an `addAutomaticKeepAlives` feature, which is enabled by
+/// default, and which causes [AutomaticKeepAlive] widgets to be inserted around
+/// each child, causing [KeepAlive] widgets to be automatically added and
+/// configured in response to [KeepAliveNotification]s.
+///
+/// Therefore, to keep a widget alive, it is more common to use those
+/// notifications than to directly deal with [KeepAlive] widgets.
+///
+/// In practice, the simplest way to deal with these notifications is to mix
+/// [AutomaticKeepAliveClientMixin] into one's [State]. See the documentation
+/// for that mixin class for details.
 class KeepAlive extends ParentDataWidget<SliverMultiBoxAdaptorWidget> {
   /// Marks a child as needing to remain alive.
   ///

packages/flutter/test/material/ink_paint_test.dart

@@ -115,7 +115,7 @@ void main() {
       );
     }
 
-    // Initially the ripple's center is where the tap occurred. Note that
+    // Initially the ripple's center is where the tap occurred;
     // ripplePattern always add a translation of tapDownOffset.
     expect(box, ripplePattern(Offset.zero, 30.0, 0));
 

packages/flutter/test/material/will_pop_test.dart

@@ -248,10 +248,10 @@ void main() {
     await tester.pump(const Duration(seconds: 1)); // Wait until it has finished.
     expect(find.text('Sample Form'), findsOneWidget);
 
-    // Do it again. Note that each time the Alert is shown and dismissed
+    // Do it again.
-    // the FormState's didChangeDependencies() method runs. We're making sure
+    // Each time the Alert is shown and dismissed the FormState's
-    // that the didChangeDependencies() method doesn't add an extra willPop
+    // didChangeDependencies() method runs. We're making sure that the
-    // callback.
+    // didChangeDependencies() method doesn't add an extra willPop callback.
     await tester.tap(find.byTooltip('Back'));
     await tester.pump(); // Start the pop "back" operation.
     await tester.pump(); // Call willPop which will show an Alert.

packages/flutter_tools/lib/src/base/build.dart

@@ -159,7 +159,7 @@ class AOTSnapshotter {
 
     if (platform == TargetPlatform.android_arm || iosArch == IOSArch.armv7) {
       // Use softfp for Android armv7 devices.
-      // Note that this is the default for armv7 iOS builds, but harmless to set.
+      // This is the default for armv7 iOS builds, but harmless to set.
       // TODO(cbracken): eliminate this when we fix https://github.com/flutter/flutter/issues/17489
       genSnapshotArgs.add('--no-sim-use-hardfp');
 

packages/flutter_tools/lib/src/runner/flutter_command.dart

@@ -310,7 +310,7 @@ abstract class FlutterCommand extends Command<Null> {
         } finally {
           final DateTime endTime = clock.now();
           printTrace('"flutter $name" took ${getElapsedAsMilliseconds(endTime.difference(startTime))}.');
-          // Note that this is checking the result of the call to 'usagePath'
+          // This is checking the result of the call to 'usagePath'
           // (a Future<String>), and not the result of evaluating the Future.
           if (usagePath != null) {
             final List<String> labels = <String>[];

packages/flutter_tools/lib/src/runner/flutter_command_runner.dart

@@ -254,7 +254,7 @@ class FlutterCommandRunner extends CommandRunner<Null> {
         os.zip(tempDir, zipFile);
         printStatus(
           'Bug report written to ${zipFile.basename}.\n'
-          'Note that this bug report contains local paths, device '
+          'Warning: this bug report contains local paths, device '
           'identifiers, and log snippets.'
         );
       }, ShutdownStage.POST_PROCESS_RECORDING);

packages/flutter_tools/lib/src/test/flutter_platform.dart

@@ -193,10 +193,9 @@ enum _TestResult { crashed, harnessBailed, testBailed }
 typedef _Finalizer = Future<Null> Function();
 
 class _CompilationRequest {
+  _CompilationRequest(this.path, this.result);
   String path;
   Completer<String> result;
-
-  _CompilationRequest(this.path, this.result);
 }
 
 // This class is a wrapper around compiler that allows multiple isolates to

packages/flutter_tools/test/protocol_discovery_test.dart

@@ -30,8 +30,8 @@ void main() {
       /// This also exists for cases where our initialization requires access to
       /// a `Context` object, which is only set up inside the zone.
       ///
-      /// Note that these issues do not pertain to real code and are a test-only
+      /// These issues do not pertain to real code and are a test-only concern,
-      /// concern, since in real code, the zone is set-up in `main()`.
+      /// because in real code, the zone is set up in `main()`.
       ///
       /// See also: [runZoned]
       void initialize() {

packages/fuchsia_remote_debug_protocol/lib/src/common/logging.dart

@@ -13,8 +13,7 @@ enum LoggingLevel {
   /// Logs no logs.
   none,
 
-  /// Logs severe messages at the most (note that severe messages are always
+  /// Logs severe messages at the most (severe messages are always logged).
-  /// logged).
   ///
   /// Severe means that the process has encountered a critical level of failure
   /// in which it cannot recover and will terminate as a result.

packages/fuchsia_remote_debug_protocol/lib/src/fuchsia_remote_connection.dart

@@ -96,8 +96,8 @@ class DartVmEvent {
 /// Provides affordances to observe and connect to Flutter views, isolates, and
 /// perform actions on the Fuchsia device's various VM services.
 ///
-/// Note that this class can be connected to several instances of the Fuchsia
+/// This class can be connected to several instances of the Fuchsia device's
-/// device's Dart VM at any given time.
+/// Dart VM at any given time.
 class FuchsiaRemoteConnection {
   FuchsiaRemoteConnection._(this._useIpV6Loopback, this._sshCommandRunner)
       : _pollDartVms = false;
@@ -168,10 +168,10 @@ class FuchsiaRemoteConnection {
   /// Throws an [ArgumentError] if the supplied `address` is not valid IPv6 or
   /// IPv4.
   ///
-  /// Note that if `address` is ipv6 link local (usually starts with fe80::),
+  /// If `address` is IPv6 link local (usually starts with `fe80::`), then
-  /// then `interface` will probably need to be set in order to connect
+  /// `interface` will probably need to be set in order to connect successfully
-  /// successfully (that being the outgoing interface of your machine, not the
+  /// (that being the outgoing interface of your machine, not the interface on
-  /// interface on the target machine).
+  /// the target machine).
   ///
   /// Attempts to set `address` via the environment variable
   /// `FUCHSIA_DEVICE_URL` in the event that the argument is not passed.

packages/fuchsia_remote_debug_protocol/lib/src/runners/ssh_command_runner.dart

@@ -42,9 +42,8 @@ class SshCommandRunner {
   /// undefined.
   ///
   /// [ArgumentError] is thrown in the event that `address` is neither valid
-  /// IPv4 nor IPv6. Note that when connecting to a link local address (fe80::
+  /// IPv4 nor IPv6. When connecting to a link local address (`fe80::` is
-  /// is usually at the start of the address), then an interface should be
+  /// usually at the start of the address), an interface should be supplied.
-  /// supplied.
   SshCommandRunner({
     this.address,
     this.interface = '',