From 209cdf457fcb7bd16f20c134106063564d5bfa0b Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Wed, 25 Oct 2023 11:24:30 -0700 Subject: [PATCH 1/4] Improve display text for libraries --- lib/src/element_type.dart | 6 + lib/src/generator/generator_frontend.dart | 2 +- .../templates.aot_renderers_for_html.dart | 22 ++- .../templates.runtime_renderers.dart | 169 ++++++++++++++---- lib/src/model/library.dart | 106 ++++++----- lib/src/model/nameable.dart | 6 + lib/src/model/package_graph.dart | 6 + lib/src/render/model_element_renderer.dart | 3 +- lib/src/validator.dart | 2 +- lib/templates/html/_head.html | 12 +- lib/templates/html/library.html | 12 +- 11 files changed, 252 insertions(+), 94 deletions(-) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index dbe38e6036..b50da3796e 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -64,6 +64,12 @@ abstract class ElementType /// Name with generics and nullability indication. String get nameWithGenerics; + @override + String get displayName => throw UnimplementedError(); + + @override + String get breadcrumbName => throw UnimplementedError(); + DartType get instantiatedType; Iterable get typeArguments; diff --git a/lib/src/generator/generator_frontend.dart b/lib/src/generator/generator_frontend.dart index 30c7a612e2..e1585027d9 100644 --- a/lib/src/generator/generator_frontend.dart +++ b/lib/src/generator/generator_frontend.dart @@ -74,7 +74,7 @@ class GeneratorFrontEnd implements Generator { for (var lib in filterNonDocumented(package.libraries)) { if (!multiplePackages) { - logInfo('Generating docs for library ${lib.name} from ' + logInfo('Generating docs for library ${lib.breadcrumbName} from ' '${lib.element.source.uri}...'); } if (!lib.isAnonymous && !lib.hasDocumentation) { diff --git a/lib/src/generator/templates.aot_renderers_for_html.dart b/lib/src/generator/templates.aot_renderers_for_html.dart index 96cd48855a..863e2dfef5 100644 --- a/lib/src/generator/templates.aot_renderers_for_html.dart +++ b/lib/src/generator/templates.aot_renderers_for_html.dart @@ -1109,17 +1109,25 @@ String renderLibrary(LibraryTemplateData context0) { var context1 = context0.self; buffer.writeln(); buffer.write(''' -
'''); +
+ '''); buffer.write(_renderLibrary_partial_source_link_1(context1)); - buffer.write('''

'''); - buffer.write(context1.name); - buffer.write(''' '''); + buffer.writeln(); + buffer.write(''' +

+ '''); + buffer.write(context1.displayName); + buffer.write(''' + '''); buffer.writeEscaped(context1.kind.toString()); buffer.write(' '); buffer.write(_renderLibrary_partial_feature_set_2(context1)); buffer.write(' '); buffer.write(_renderLibrary_partial_categorization_3(context1)); - buffer.write('''

'''); + buffer.writeln(); + buffer.write(''' + +
'''); buffer.writeln(); var context2 = context0.library; buffer.write('\n '); @@ -3579,7 +3587,7 @@ String _deduplicated_lib_templates_html__head_html(TemplateDataBase context0) {
  • '''); - buffer.writeEscaped(context4.name); + buffer.writeEscaped(context4.breadcrumbName); buffer.write('''
    • '''); } var context5 = context0.navLinksWithGenerics; @@ -3589,7 +3597,7 @@ String _deduplicated_lib_templates_html__head_html(TemplateDataBase context0) {
  • '''); - buffer.writeEscaped(context6.name); + buffer.writeEscaped(context6.breadcrumbName); if (context6.hasGenericParameters == true) { buffer.write(''''''); buffer.write(context6.genericParameters); diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart index e5b769af8b..ecb08d0869 100644 --- a/lib/src/generator/templates.runtime_renderers.dart +++ b/lib/src/generator/templates.runtime_renderers.dart @@ -4399,6 +4399,50 @@ class _Renderer_ElementType extends RendererBase { ..._Renderer_CommentReferable.propertyMap(), ..._Renderer_Nameable.propertyMap(), ..._Renderer_ModelBuilder.propertyMap(), + 'breadcrumbName': Property( + getValue: (CT_ c) => c.breadcrumbName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable( + self.getValue(c) as String, + nextProperty, + [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => false, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.breadcrumbName, ast, r.template, sink, + parent: r); + }, + ), + 'displayName': Property( + getValue: (CT_ c) => c.displayName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable( + self.getValue(c) as String, + nextProperty, + [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => false, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.displayName, ast, r.template, sink, + parent: r); + }, + ), 'instantiatedType': Property( getValue: (CT_ c) => c.instantiatedType, renderVariable: (CT_ c, Property self, @@ -8276,6 +8320,28 @@ class _Renderer_Library extends RendererBase { parent: r); }, ), + 'breadcrumbName': Property( + getValue: (CT_ c) => c.breadcrumbName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable( + self.getValue(c) as String, + nextProperty, + [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => false, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.breadcrumbName, ast, r.template, sink, + parent: r); + }, + ), 'characterLocation': Property( getValue: (CT_ c) => c.characterLocation, renderVariable: (CT_ c, Property self, @@ -8351,6 +8417,28 @@ class _Renderer_Library extends RendererBase { _render_String(c.dirName, ast, r.template, sink, parent: r); }, ), + 'displayName': Property( + getValue: (CT_ c) => c.displayName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable( + self.getValue(c) as String, + nextProperty, + [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => false, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.displayName, ast, r.template, sink, + parent: r); + }, + ), 'element': Property( getValue: (CT_ c) => c.element, renderVariable: (CT_ c, Property self, @@ -8594,28 +8682,6 @@ class _Renderer_Library extends RendererBase { _render_String(c.name, ast, r.template, sink, parent: r); }, ), - 'nameFromPath': Property( - getValue: (CT_ c) => c.nameFromPath, - renderVariable: - (CT_ c, Property self, List remainingNames) { - if (remainingNames.isEmpty) { - return self.getValue(c).toString(); - } - var name = remainingNames.first; - var nextProperty = - _Renderer_String.propertyMap().getValue(name); - return nextProperty.renderVariable( - self.getValue(c) as String, - nextProperty, - [...remainingNames.skip(1)]); - }, - isNullValue: (CT_ c) => false, - renderValue: (CT_ c, RendererBase r, - List ast, StringSink sink) { - _render_String(c.nameFromPath, ast, r.template, sink, - parent: r); - }, - ), 'package': Property( getValue: (CT_ c) => c.package, renderVariable: @@ -8673,19 +8739,6 @@ class _Renderer_Library extends RendererBase { parent: r); }, ), - 'prefixToLibrary': Property( - getValue: (CT_ c) => c.prefixToLibrary, - renderVariable: (CT_ c, Property self, - List remainingNames) => - self.renderSimpleVariable( - c, remainingNames, 'Map>'), - isNullValue: (CT_ c) => false, - renderValue: (CT_ c, RendererBase r, - List ast, StringSink sink) { - renderSimple(c.prefixToLibrary, ast, r.template, sink, - parent: r, getters: _invisibleGetters['Map']!); - }, - ), 'properties': Property( getValue: (CT_ c) => c.properties, renderVariable: (CT_ c, Property self, @@ -11544,6 +11597,50 @@ class _Renderer_Nameable extends RendererBase { CT_, () => { ..._Renderer_Object.propertyMap(), + 'breadcrumbName': Property( + getValue: (CT_ c) => c.breadcrumbName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable( + self.getValue(c) as String, + nextProperty, + [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => false, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.breadcrumbName, ast, r.template, sink, + parent: r); + }, + ), + 'displayName': Property( + getValue: (CT_ c) => c.displayName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable( + self.getValue(c) as String, + nextProperty, + [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => false, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.displayName, ast, r.template, sink, + parent: r); + }, + ), 'fullyQualifiedName': Property( getValue: (CT_ c) => c.fullyQualifiedName, renderVariable: @@ -16886,10 +16983,12 @@ const _invisibleGetters = { 'allLibraries', 'allLibrariesAdded', 'allLocalModelElements', + 'breadcrumbName', 'config', 'dartCoreObject', 'defaultPackage', 'defaultPackageName', + 'displayName', 'documentedExtensions', 'documentedPackages', 'extensions', diff --git a/lib/src/model/library.dart b/lib/src/model/library.dart index bebc6577c2..add5060b16 100644 --- a/lib/src/model/library.dart +++ b/lib/src/model/library.dart @@ -182,7 +182,7 @@ class Library extends ModelElement /// Map of each import prefix ('import "foo" as prefix;') to the set of /// libraries which are imported via that prefix. - Map> get prefixToLibrary { + Map> get _prefixToLibrary { var prefixToLibrary = >{}; // It is possible to have overlapping prefixes. for (var i in element.libraryImports) { @@ -197,9 +197,40 @@ class Library extends ModelElement return prefixToLibrary; } - late final String dirName = (isAnonymous ? nameFromPath : name) - .replaceAll(':', '-') - .replaceAll('/', '_'); + /// An identifier for this library based on its location. + /// + /// This provides filename collision-proofing for anonymous libraries by + /// incorporating more from the location of the anonymous library into the + /// name calculation. Simple cases (such as an anonymous library in 'lib/') + /// are the same, but this will include slashes and possibly colons + /// for anonymous libraries in subdirectories or other packages. + late final String dirName = () { + String nameFromPath; + if (isAnonymous) { + assert(!_restoredUri.startsWith('file:'), + '"$_restoredUri" must not start with "file:"'); + // Strip the package prefix if the library is part of the default package + // or if it is being documented remotely. + var packageToHide = package.documentedWhere == DocumentLocation.remote + ? package.packageMeta + : package.packageGraph.packageMeta; + var schemaToHide = 'package:$packageToHide/'; + + nameFromPath = _restoredUri; + if (nameFromPath.startsWith(schemaToHide)) { + nameFromPath = + nameFromPath.substring(schemaToHide.length, nameFromPath.length); + } + if (nameFromPath.endsWith('.dart')) { + const dartExtensionLength = '.dart'.length; + nameFromPath = nameFromPath.substring( + 0, nameFromPath.length - dartExtensionLength); + } + } else { + nameFromPath = name; + } + return nameFromPath.replaceAll(':', '-').replaceAll('/', '_'); + }(); /// Libraries are not enclosed by anything. @override @@ -281,15 +312,36 @@ class Library extends ModelElement return baseName; } - /// Generate a name for this library based on its location. - /// - /// nameFromPath provides filename collision-proofing for anonymous libraries - /// by incorporating more from the location of the anonymous library into - /// the name calculation. Simple cases (such as an anonymous library in - /// 'lib') are the same, but this will include slashes and possibly colons - /// for anonymous libraries in subdirectories or other packages. - late final String nameFromPath = - _getNameFromPath(element, package, _restoredUri); + @override + String get displayName { + var fullName = breadcrumbName; + if (fullName.endsWith('.dart')) { + const dartExtensionLength = '.dart'.length; + return fullName.substring(0, fullName.length - dartExtensionLength); + } + return fullName; + } + + @override + String get breadcrumbName { + var source = element.source; + if (source.uri.isScheme('dart')) { + return name; + } + + return _importPath; + } + + /// The path portion of this library's import URI as a 'package:' URI. + String get _importPath { + // This code should not be used for Dart SDK libraries. + assert(!element.source.uri.isScheme('dart')); + var relativePath = pathContext.relative(element.source.fullName, + from: package.packagePath); + assert(relativePath.startsWith('lib/')); + const libDirectoryLength = 'lib/'.length; + return relativePath.substring(libDirectoryLength); + } /// The name of the package we were defined in. String get packageName => packageMeta?.name ?? ''; @@ -342,32 +394,6 @@ class Library extends ModelElement return variables; } - /// Reverses URIs if needed to get a package URI. - /// - /// Not the same as [PackageGraph.name] because there we always strip all - /// path components; this function only strips the package prefix if the - /// library is part of the default package or if it is being documented - /// remotely. - static String _getNameFromPath( - LibraryElement element, Package package, String restoredUri) { - assert(!restoredUri.startsWith('file:'), - '"$restoredUri" must not start with "file:"'); - var hidePackage = package.documentedWhere == DocumentLocation.remote - ? package.packageMeta - : package.packageGraph.packageMeta; - var defaultPackagePrefix = 'package:$hidePackage/'; - - var name = restoredUri; - if (name.startsWith(defaultPackagePrefix)) { - name = name.substring(defaultPackagePrefix.length, name.length); - } - if (name.endsWith('.dart')) { - name = name.substring(0, name.length - '.dart'.length); - } - assert(!name.startsWith('file:')); - return name; - } - /// A mapping of all [Element]s in this library to the [ModelElement]s which /// represent them in dartdoc. // Note: Keep this a late final field; converting to a getter (without further @@ -409,7 +435,7 @@ class Library extends ModelElement // refer to hidden members via the prefix, because that can be // ambiguous. dart-lang/dartdoc#2683. for (var MapEntry(key: prefix, value: libraries) - in prefixToLibrary.entries) { + in _prefixToLibrary.entries) { referenceChildrenBuilder.putIfAbsent(prefix, () => libraries.first); } return referenceChildrenBuilder; diff --git a/lib/src/model/nameable.dart b/lib/src/model/nameable.dart index 902d381dad..177036fd3a 100644 --- a/lib/src/model/nameable.dart +++ b/lib/src/model/nameable.dart @@ -16,6 +16,12 @@ abstract mixin class Nameable { ...name.split(locationSplitter).where((s) => s.isNotEmpty) }; + /// The name to use as text in the rendered documentation. + String get displayName => name; + + /// The name to use in breadcrumbs in the rendered documentation. + String get breadcrumbName => name; + /// Utility getter/cache for `_MarkdownCommentReference._getResultsForClass`. // TODO(jcollins-g): This should really be the same as 'name', but isn't // because of accessors and operators. diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index 73a9f7efd4..65c49ceb08 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -59,6 +59,12 @@ class PackageGraph with CommentReferable, Nameable, ModelBuilder { @override String get name => ''; + @override + String get displayName => throw UnimplementedError(); + + @override + String get breadcrumbName => throw UnimplementedError(); + /// Adds [resolvedLibrary] to the package graph, adding it to [allLibraries], /// and to the [Package] which is created from the [PackageMeta] for the /// library. diff --git a/lib/src/render/model_element_renderer.dart b/lib/src/render/model_element_renderer.dart index 127743249b..382e705ad6 100644 --- a/lib/src/render/model_element_renderer.dart +++ b/lib/src/render/model_element_renderer.dart @@ -31,7 +31,8 @@ class ModelElementRendererHtml extends ModelElementRenderer { @override String renderLinkedName(ModelElement modelElement) { var cssClass = modelElement.isDeprecated ? ' class="deprecated"' : ''; - return '${modelElement.name}'; + return '' + '${modelElement.displayName}'; } @override diff --git a/lib/src/validator.dart b/lib/src/validator.dart index ce0ea76f15..46b74226eb 100644 --- a/lib/src/validator.dart +++ b/lib/src/validator.dart @@ -36,7 +36,7 @@ class Validator { /// Don't call this method more than once, and only after you've /// generated all docs for the Package. void validateLinks() { - logInfo('Validating...'); + logInfo('Validating links...'); runtimeStats.resetAccumulators({ 'readCountForLinkValidation', 'readCountForIndexValidation', diff --git a/lib/templates/html/_head.html b/lib/templates/html/_head.html index fe93a83a45..496b883576 100644 --- a/lib/templates/html/_head.html +++ b/lib/templates/html/_head.html @@ -40,12 +40,12 @@
    menu