command line cleanup and link-to-remote by default (#2187)

* Change default

* Link to remote, spinner, doc fixes

* Fix dartdoc sanity check

* Update README for new outpu

* Fix grinder tests

Author: jcollins-g

README.md

@@ -20,25 +20,17 @@ For issues/details related to hosted Dart API docs, see
 
 ## Generating docs
 
-Run `dartdoc` from the root directory of package. For example:
+Run `dartdoc` from the root directory of a package.  Here is an example of dartdoc documenting itself:
 
 ```
 $ dartdoc
-Generating documentation for 'server_code_lab' into <path-to-server-code-lab>/server_code_lab/doc/api/
-
-parsing lib/client/piratesapi.dart...
-parsing lib/common/messages.dart...
-parsing lib/common/utils.dart...
-parsing lib/server/piratesapi.dart...
-Parsed 4 files in 8.1 seconds.
-
-generating docs for library pirate.messages from messages.dart...
-generating docs for library pirate.server from piratesapi.dart...
-generating docs for library pirate.utils from utils.dart...
-generating docs for library server_code_lab.piratesApi.client from piratesapi.dart...
-Documented 4 libraries in 9.6 seconds.
-
-Success! Docs generated into <path-to-server-code-lab>/server_code_lab/doc/api/index.html
+Documenting dartdoc...
+Initialized dartdoc with 766 libraries in 63.9 seconds
+Generating docs for library dartdoc from package:dartdoc/dartdoc.dart...
+Validating docs...
+no issues found
+Documented 1 public library in 17.9 seconds
+Success! Docs generated into <path to dartdoc>/doc/api
 ```
 
 By default, the documentation is generated to the `doc/api` directory as static

lib/dartdoc.dart

@@ -189,7 +189,7 @@ class Dartdoc extends PackageBuilder {
   }
 
   Future<DartdocResults> generateDocs() async {
-    logPrint("Documenting ${config.topLevelPackageMeta}...");
+    logInfo("Documenting ${config.topLevelPackageMeta}...");
 
     DartdocResults dartdocResults = await generateDocsBase();
     if (dartdocResults.packageGraph.localPublicLibraries.isEmpty) {

lib/src/dartdoc_options.dart

@@ -205,8 +205,8 @@ class Snapshot {
 
   bool _needsSnapshot = true;
 
-  /// Will return true precisely once, unless [toolPath] was already a snapshot.
-  /// In that case, will always return false.
+  /// Will return true precisely once, unless [snapshotFile] was already a
+  /// snapshot.  In that case, will always return false.
   bool get needsSnapshot {
     if (_needsSnapshot == true) {
       _needsSnapshot = false;
@@ -1525,18 +1525,14 @@ Future<List<DartdocOption>> createDartdocOptions() async {
         mustExist: true),
     DartdocOptionSet('linkTo')
       ..addAll([
-        DartdocOptionArgOnly<Map<String, String>>(
-            'hosted',
-            {
-              'pub.dartlang.org':
-                  'https://pub.dartlang.org/documentation/%n%/%v%'
-            },
+        DartdocOptionArgOnly<Map<String, String>>('hosted',
+            {'pub.dartlang.org': 'https://pub.dev/documentation/%n%/%v%'},
             help: 'Specify URLs for hosted pub packages'),
         DartdocOptionArgOnly<Map<String, String>>(
           'sdks',
           {
-            'Dart': 'https://api.dartlang.org/%b%/%v%',
-            'Flutter': 'https://docs.flutter.io/flutter',
+            'Dart': 'https://api.dart.dev/%b%/%v%',
+            'Flutter': 'https://api.flutter.dev/flutter',
           },
           help: 'Specify URLs for SDKs.',
         ),
@@ -1558,7 +1554,7 @@ Future<List<DartdocOption>> createDartdocOptions() async {
           }
           return '';
         }, help: 'Url to use for this particular package.'),
-        DartdocOptionArgOnly<bool>('remote', false,
+        DartdocOptionArgOnly<bool>('remote', true,
             help: 'Allow links to be generated for packages outside this one.',
             negatable: true),
       ]),

lib/src/generator/generator_frontend.dart

@@ -36,7 +36,6 @@ class GeneratorFrontEnd implements Generator {
       List<Indexable> indexAccumulator) {
     if (packageGraph == null) return;
 
-    logInfo('documenting ${packageGraph.defaultPackage.name}');
     _generatorBackend.generatePackage(
         writer, packageGraph, packageGraph.defaultPackage);
 

lib/src/logging.dart

@@ -2,6 +2,7 @@ import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 
+import 'package:cli_util/cli_logging.dart' show Ansi;
 import 'package:dartdoc/src/dartdoc_options.dart';
 // Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
@@ -29,6 +30,10 @@ void logInfo(Object message) {
   _logger.log(Level.INFO, message);
 }
 
+void logDebug(Object message) {
+  _logger.log(Level.FINE, message);
+}
+
 void logProgress(Object message) {
   _logger.log(progressLevel, message);
 }
@@ -74,41 +79,47 @@ void startLogging(LoggingContext config) {
     // Used to track if we're printing `...` to show progress.
     // Allows unified new-line tracking
     var writingProgress = false;
+    Ansi ansi = Ansi(Ansi.terminalSupportsAnsi);
+    int spinnerIndex = 0;
+    final List<String> spinner = ['-', r'\', '|', '/'];
 
     Logger.root.onRecord.listen((record) {
       if (record.level == progressLevel) {
-        if (config.showProgress && stopwatch.elapsed.inMilliseconds > 250) {
+        if (!config.quiet &&
+            config.showProgress &&
+            stopwatch.elapsed.inMilliseconds > 125) {
+          if (writingProgress = false) {
+            stdout.write(' ');
+          }
           writingProgress = true;
-          stdout.write('.');
+          stdout.write('${ansi.backspace}${spinner[spinnerIndex]}');
+          spinnerIndex = (spinnerIndex + 1) % spinner.length;
           stopwatch.reset();
         }
         return;
       }
 
       stopwatch.reset();
       if (writingProgress) {
-        // print a new line after progress dots...
-        print('');
-        writingProgress = false;
+        stdout.write('${ansi.backspace} ${ansi.backspace}');
       }
       var message = record.message;
       assert(message == message.trimRight());
       assert(message.isNotEmpty);
 
       if (record.level < Level.WARNING) {
         if (!config.quiet) {
-          if (config.showProgress && message.endsWith('...')) {
-            // Assume there may be more progress to print, so omit the trailing
-            // newline
-            writingProgress = true;
-            stdout.write(message);
-          } else {
-            print(message);
-          }
+          print(message);
         }
       } else {
-        stderr.writeln(message);
+        if (writingProgress) {
+          // Some console implementations, like IntelliJ, apparently need
+          // the backspace to occur for stderr as well.
+          stderr.write('${ansi.backspace} ${ansi.backspace}');
+        }
+        stderr.write('${message}\n');
       }
+      writingProgress = false;
     });
   }
 }
@@ -124,9 +135,9 @@ Future<List<DartdocOption>> createLoggingOptions() async {
     DartdocOptionArgOnly<bool>('json', false,
         help: 'Prints out progress JSON maps. One entry per line.',
         negatable: true),
-    DartdocOptionArgOnly<bool>('showProgress', false,
-        help: 'Display progress indications to console stdout',
-        negatable: false),
+    DartdocOptionArgOnly<bool>('showProgress', Ansi.terminalSupportsAnsi,
+        help: 'Display progress indications to console stdout.',
+        negatable: true),
     DartdocOptionArgSynth<bool>('quiet',
         (DartdocSyntheticOption option, Directory dir) {
       if (option.root['generateDocs']?.valueAt(dir) == false) {

lib/src/model/class.dart

@@ -91,7 +91,7 @@ class Class extends Container
 
   Map<String, List<ModelElement>> _allModelElementsByNamePart;
 
-  /// Helper for [_MarkdownCommentReference._getResultsForClass].
+  /// Helper for `_MarkdownCommentReference._getResultsForClass`.
   Map<String, List<ModelElement>> get allModelElementsByNamePart {
     if (_allModelElementsByNamePart == null) {
       _allModelElementsByNamePart = {};
@@ -435,8 +435,6 @@ class Class extends Container
 
   List<Field> _fields;
 
-  /// Internal only because subclasses are allowed to override how
-  /// these are mapped to [allInheritedFields] and so forth.
   @override
   List<Field> get allFields {
     if (_fields == null) {

lib/src/model/inheritable.dart

@@ -16,7 +16,7 @@ import 'package:dartdoc/src/special_elements.dart';
 /// namespace, that's the one we should treat as canonical and implementors
 /// of this class can use that knowledge to determine canonicalization.
 ///
-/// We pick the class closest to the [definingEnclosingElement] so that all
+/// We pick the class closest to the [definingEnclosingContainer] so that all
 /// children of that class inheriting the same member will point to the same
 /// place in the documentation, and we pick a canonical class because that's
 /// the one in the public namespace that will be documented.

lib/src/model/locatable.dart

@@ -2,6 +2,8 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'package:analyzer/dart/element/element.dart' show Element;
+
 /// Something that can be located for warning purposes.
 abstract class Locatable {
   List<Locatable> get documentationFrom;

lib/src/model/model_element.dart

@@ -744,6 +744,7 @@ abstract class ModelElement extends Canonicalization
 
   @override
   bool get isCanonical {
+    if (!isPublic) return false;
     if (library == canonicalLibrary) {
       if (this is Inheritable) {
         Inheritable i = (this as Inheritable);

lib/src/model/nameable.dart

@@ -24,7 +24,7 @@ abstract class Nameable {
 
   String _namePart;
 
-  /// Utility getter/cache for [_MarkdownCommentReference._getResultsForClass].
+  /// Utility getter/cache for `_MarkdownCommentReference._getResultsForClass`.
   String get namePart {
     // TODO(jcollins-g): This should really be the same as 'name', but isn't
     // because of accessors and operators.

lib/src/model/never.dart

@@ -9,7 +9,7 @@ class NeverType extends ModelElement {
   NeverType(Element element, PackageGraph packageGraph)
       : super(element, null, packageGraph, null);
 
-  /// [never] is not a real object, and so we can't document it, so there
+  /// `Never` is not a real object, and so we can't document it, so there
   /// can be nothing canonical for it.
   @override
   ModelElement get canonicalModelElement => null;

lib/src/model/package.dart

@@ -147,9 +147,9 @@ class Package extends LibraryContainer
 
   bool _isLocal;
 
-  /// Return true if this is the default package, this is part of an embedder SDK,
-  /// or if [config.autoIncludeDependencies] is true -- but only if the package
-  /// was not excluded on the command line.
+  /// Return true if this is the default package, this is part of an embedder
+  /// SDK, or if [DartdocOptionContext.autoIncludeDependencies] is true -- but
+  /// only if the package was not excluded on the command line.
   bool get isLocal {
     if (_isLocal == null) {
       _isLocal = (

lib/src/model/package_builder.dart

@@ -163,7 +163,7 @@ class PackageBuilder {
           null,
           sourceFactory,
           options);
-      driver.results.listen((_) {});
+      driver.results.listen((_) => logProgress(''));
       driver.exceptions.listen((_) {});
       scheduler.start();
     }
@@ -243,11 +243,12 @@ class PackageBuilder {
       // Be careful here not to accidentally stack up multiple
       // ResolvedLibraryResults, as those eat our heap.
       for (String f in files) {
+        logProgress(f);
         ResolvedLibraryResult r = await processLibrary(f);
         if (r != null &&
             !libraries.contains(r.element) &&
             isLibraryIncluded(r.element)) {
-          logInfo('parsing ${f}...');
+          logDebug('parsing ${f}...');
           libraryAdder(r);
           libraries.add(r.element);
         }

lib/src/model/package_graph.dart

@@ -14,6 +14,7 @@ import 'package:analyzer/src/generated/source.dart';
 import 'package:analyzer/src/generated/source_io.dart';
 import 'package:collection/collection.dart';
 import 'package:dartdoc/src/dartdoc_options.dart';
+import 'package:dartdoc/src/logging.dart';
 import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/model_utils.dart' as utils;
 import 'package:dartdoc/src/package_meta.dart' show PackageMeta;
@@ -103,11 +104,13 @@ class PackageGraph {
             !precachedElements.contains(d)) {
           precachedElements.add(d);
           yield d.precacheLocalDocs();
+          logProgress(d.name);
           // TopLevelVariables get their documentation from getters and setters,
           // so should be precached if either has a template.
           if (m is TopLevelVariable && !precachedElements.contains(m)) {
             precachedElements.add(m);
             yield m.precacheLocalDocs();
+            logProgress(d.name);
           }
         }
       }

lib/src/model/top_level_container.dart

@@ -6,7 +6,7 @@ import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/model_utils.dart' as model_utils;
 
 /// A set of [Class]es, [Enum]s, [TopLevelVariable]s, [ModelFunction]s,
-/// [Property]s, and [Typedef]s, possibly initialized after construction by
+/// [Field]s, and [Typedef]s, possibly initialized after construction by
 /// accessing private member variables.  Do not call any methods or members
 /// excepting [name] and the private Lists below before finishing initialization
 /// of a [TopLevelContainer].

pubspec.yaml

@@ -10,6 +10,7 @@ dependencies:
   analyzer: ^0.39.6
   args: '>=1.5.0 <2.0.0'
   collection: ^1.2.0
+  cli_util: ^0.1.3+2
   crypto: ^2.0.6
   html: '>=0.12.1 <0.15.0'
   # We don't use http_parser directly; this dep exists to ensure that we get at

test/dartdoc_test.dart

@@ -211,8 +211,7 @@ void main() {
     });
 
     test('basic interlinking test', () async {
-      Dartdoc dartdoc =
-          await buildDartdoc(['--link-to-remote'], testPackageDir, tempDir);
+      Dartdoc dartdoc = await buildDartdoc([], testPackageDir, tempDir);
       DartdocResults results = await dartdoc.generateDocs();
       PackageGraph p = results.packageGraph;
       Package meta = p.publicPackages.firstWhere((p) => p.name == 'meta');
@@ -228,9 +227,9 @@ void main() {
       expect(
           useSomethingInAnotherPackage.modelType.linkedName,
           matches(
-              '<a href=\"https://pub.dartlang.org/documentation/meta/[^\"]*/meta/Required-class.html\">Required</a>'));
+              '<a href=\"https://pub.dev/documentation/meta/[^\"]*/meta/Required-class.html\">Required</a>'));
       RegExp stringLink = RegExp(
-          'https://api.dartlang.org/(dev|stable|edge|be)/${Platform.version.split(' ').first}/dart-core/String-class.html">String</a>');
+          'https://api.dart.dev/(dev|stable|edge|be|beta)/${Platform.version.split(' ').first}/dart-core/String-class.html">String</a>');
       expect(useSomethingInTheSdk.modelType.linkedName, contains(stringLink));
     });
 

test/src/utils.dart

@@ -28,29 +28,42 @@ PackageMeta sdkPackageMeta = PackageMeta.fromDir(sdkDir);
 
 final _testPackageGraphMemo = AsyncMemoizer<PackageGraph>();
 Future<PackageGraph> get testPackageGraph => _testPackageGraphMemo.runOnce(() =>
-    bootBasicPackage('testing/test_package', ['css', 'code_in_comments']));
+    bootBasicPackage('testing/test_package', ['css', 'code_in_comments'],
+        additionalArguments: ['--no-link-to-remote']));
 
 final _testPackageGraphExperimentsMemo = AsyncMemoizer<PackageGraph>();
 Future<PackageGraph> get testPackageGraphExperiments =>
     _testPackageGraphExperimentsMemo.runOnce(() => bootBasicPackage(
-        'testing/test_package_experiments', [],
-        additionalArguments: ['--enable-experiment', 'non-nullable']));
+            'testing/test_package_experiments', [], additionalArguments: [
+          '--enable-experiment',
+          'non-nullable',
+          '--no-link-to-remote'
+        ]));
 
 final _testPackageGraphGinormousMemo = AsyncMemoizer<PackageGraph>();
 Future<PackageGraph> get testPackageGraphGinormous =>
     _testPackageGraphGinormousMemo.runOnce(() => bootBasicPackage(
-        'testing/test_package', ['css', 'code_in_commnets', 'excluded'],
-        additionalArguments: ['--auto-include-dependencies']));
+            'testing/test_package', [
+          'css',
+          'code_in_commnets',
+          'excluded'
+        ], additionalArguments: [
+          '--auto-include-dependencies',
+          '--no-link-to-remote'
+        ]));
 
 final _testPackageGraphSmallMemo = AsyncMemoizer<PackageGraph>();
-Future<PackageGraph> get testPackageGraphSmall => _testPackageGraphSmallMemo
-    .runOnce(() => bootBasicPackage('testing/test_package_small', []));
+Future<PackageGraph> get testPackageGraphSmall =>
+    _testPackageGraphSmallMemo.runOnce(() => bootBasicPackage(
+        'testing/test_package_small', [],
+        additionalArguments: ['--no-link-to-remote']));
 
 final _testPackageGraphErrorsMemo = AsyncMemoizer<PackageGraph>();
 Future<PackageGraph> get testPackageGraphErrors =>
     _testPackageGraphErrorsMemo.runOnce(() => bootBasicPackage(
         'testing/test_package_doc_errors',
-        ['css', 'code_in_comments', 'excluded']));
+        ['css', 'code_in_comments', 'excluded'],
+        additionalArguments: ['--no-link-to-remote']));
 
 final _testPackageGraphSdkMemo = AsyncMemoizer<PackageGraph>();
 Future<PackageGraph> get testPackageGraphSdk =>