-
Notifications
You must be signed in to change notification settings - Fork 125
Split sidebars out into separate HTML files #3384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a752db8
b087388
51c87e9
ef43a73
3d40e19
080569e
2dc244b
408247b
c38a515
9f688df
17cd765
8d544d8
6922b3a
8309836
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,19 +56,6 @@ class DartdocGeneratorBackendOptions implements TemplateOptions { | |
resourcesDir = context.resourcesDir; | ||
} | ||
|
||
class SidebarGenerator<T extends TemplateData> { | ||
final String Function(T context) renderFunction; | ||
final Map<Documentable, String> _renderCache = {}; | ||
|
||
SidebarGenerator(this.renderFunction); | ||
|
||
/// Retrieves the render for a specific [key], or generates it using the given | ||
/// [templateData] if needed. | ||
String getRenderFor(Documentable key, T templateData) { | ||
return _renderCache[key] ??= renderFunction(templateData); | ||
} | ||
} | ||
|
||
/// An interface for classes which are responsible for outputing the generated | ||
/// documentation. | ||
abstract class GeneratorBackend { | ||
|
@@ -142,10 +129,6 @@ abstract class GeneratorBackend { | |
abstract class GeneratorBackendBase implements GeneratorBackend { | ||
final DartdocGeneratorBackendOptions options; | ||
final Templates templates; | ||
final SidebarGenerator<TemplateDataWithLibrary<Documentable>> | ||
_sidebarForLibrary; | ||
final SidebarGenerator<TemplateDataWithContainer<Documentable>> | ||
_sidebarForContainer; | ||
|
||
@override | ||
final FileWriter writer; | ||
|
@@ -154,11 +137,7 @@ abstract class GeneratorBackendBase implements GeneratorBackend { | |
|
||
GeneratorBackendBase( | ||
this.options, this.templates, this.writer, this.resourceProvider) | ||
: _sidebarForLibrary = | ||
SidebarGenerator(templates.renderSidebarForLibrary), | ||
_sidebarForContainer = | ||
SidebarGenerator(templates.renderSidebarForContainer), | ||
_pathContext = resourceProvider.pathContext; | ||
: _pathContext = resourceProvider.pathContext; | ||
|
||
/// Binds template data and emits the content to the [writer]. | ||
void write( | ||
|
@@ -205,8 +184,7 @@ abstract class GeneratorBackendBase implements GeneratorBackend { | |
|
||
@override | ||
void generateClass(PackageGraph packageGraph, Library library, Class clazz) { | ||
var data = ClassTemplateData(options, packageGraph, library, clazz, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We no longer need to pass around these sidebar generators. |
||
_sidebarForLibrary.getRenderFor, _sidebarForContainer.getRenderFor); | ||
var data = ClassTemplateData(options, packageGraph, library, clazz); | ||
var content = templates.renderClass(data); | ||
write(writer, clazz.filePath, data, content); | ||
runtimeStats.incrementAccumulator('writtenClassFileCount'); | ||
|
@@ -220,17 +198,16 @@ abstract class GeneratorBackendBase implements GeneratorBackend { | |
@override | ||
void generateConstructor(PackageGraph packageGraph, Library library, | ||
Constructable constructable, Constructor constructor) { | ||
var data = ConstructorTemplateData(options, packageGraph, library, | ||
constructable, constructor, _sidebarForContainer.getRenderFor); | ||
var data = ConstructorTemplateData( | ||
options, packageGraph, library, constructable, constructor); | ||
var content = templates.renderConstructor(data); | ||
write(writer, constructor.filePath, data, content); | ||
runtimeStats.incrementAccumulator('writtenConstructorFileCount'); | ||
} | ||
|
||
@override | ||
void generateEnum(PackageGraph packageGraph, Library library, Enum eNum) { | ||
var data = EnumTemplateData(options, packageGraph, library, eNum, | ||
_sidebarForLibrary.getRenderFor, _sidebarForContainer.getRenderFor); | ||
var data = EnumTemplateData(options, packageGraph, library, eNum); | ||
var content = templates.renderEnum(data); | ||
write(writer, eNum.filePath, data, content); | ||
runtimeStats.incrementAccumulator('writtenEnumFileCount'); | ||
|
@@ -239,8 +216,7 @@ abstract class GeneratorBackendBase implements GeneratorBackend { | |
@override | ||
void generateExtension( | ||
PackageGraph packageGraph, Library library, Extension extension) { | ||
var data = ExtensionTemplateData(options, packageGraph, library, extension, | ||
_sidebarForLibrary.getRenderFor, _sidebarForContainer.getRenderFor); | ||
var data = ExtensionTemplateData(options, packageGraph, library, extension); | ||
var content = templates.renderExtension(data); | ||
write(writer, extension.filePath, data, content); | ||
runtimeStats.incrementAccumulator('writtenExtensionFileCount'); | ||
|
@@ -249,17 +225,15 @@ abstract class GeneratorBackendBase implements GeneratorBackend { | |
@override | ||
void generateFunction( | ||
PackageGraph packageGraph, Library library, ModelFunction function) { | ||
var data = FunctionTemplateData(options, packageGraph, library, function, | ||
_sidebarForLibrary.getRenderFor); | ||
var data = FunctionTemplateData(options, packageGraph, library, function); | ||
var content = templates.renderFunction(data); | ||
write(writer, function.filePath, data, content); | ||
runtimeStats.incrementAccumulator('writtenFunctionFileCount'); | ||
} | ||
|
||
@override | ||
void generateLibrary(PackageGraph packageGraph, Library library) { | ||
var data = LibraryTemplateData( | ||
options, packageGraph, library, _sidebarForLibrary.getRenderFor); | ||
var data = LibraryTemplateData(options, packageGraph, library); | ||
var content = templates.renderLibrary(data); | ||
write(writer, library.filePath, data, content); | ||
runtimeStats.incrementAccumulator('writtenLibraryFileCount'); | ||
|
@@ -268,17 +242,16 @@ abstract class GeneratorBackendBase implements GeneratorBackend { | |
@override | ||
void generateMethod(PackageGraph packageGraph, Library library, | ||
Container clazz, Method method) { | ||
var data = MethodTemplateData(options, packageGraph, library, clazz, method, | ||
_sidebarForContainer.getRenderFor); | ||
var data = | ||
MethodTemplateData(options, packageGraph, library, clazz, method); | ||
var content = templates.renderMethod(data); | ||
write(writer, method.filePath, data, content); | ||
runtimeStats.incrementAccumulator('writtenMethodFileCount'); | ||
} | ||
|
||
@override | ||
void generateMixin(PackageGraph packageGraph, Library library, Mixin mixin) { | ||
var data = MixinTemplateData(options, packageGraph, library, mixin, | ||
_sidebarForLibrary.getRenderFor, _sidebarForContainer.getRenderFor); | ||
var data = MixinTemplateData(options, packageGraph, library, mixin); | ||
var content = templates.renderMixin(data); | ||
write(writer, mixin.filePath, data, content); | ||
runtimeStats.incrementAccumulator('writtenMixinFileCount'); | ||
|
@@ -295,8 +268,8 @@ abstract class GeneratorBackendBase implements GeneratorBackend { | |
@override | ||
void generateProperty(PackageGraph packageGraph, Library library, | ||
Container clazz, Field field) { | ||
var data = PropertyTemplateData(options, packageGraph, library, clazz, | ||
field, _sidebarForContainer.getRenderFor); | ||
var data = | ||
PropertyTemplateData(options, packageGraph, library, clazz, field); | ||
var content = templates.renderProperty(data); | ||
write(writer, field.filePath, data, content); | ||
runtimeStats.incrementAccumulator('writtenPropertyFileCount'); | ||
|
@@ -310,8 +283,8 @@ abstract class GeneratorBackendBase implements GeneratorBackend { | |
@override | ||
void generateTopLevelProperty( | ||
PackageGraph packageGraph, Library library, TopLevelVariable property) { | ||
var data = TopLevelPropertyTemplateData(options, packageGraph, library, | ||
property, _sidebarForLibrary.getRenderFor); | ||
var data = | ||
TopLevelPropertyTemplateData(options, packageGraph, library, property); | ||
var content = templates.renderTopLevelProperty(data); | ||
write(writer, property.filePath, data, content); | ||
runtimeStats.incrementAccumulator('writtenTopLevelPropertyFileCount'); | ||
|
@@ -320,8 +293,7 @@ abstract class GeneratorBackendBase implements GeneratorBackend { | |
@override | ||
void generateTypeDef( | ||
PackageGraph packageGraph, Library library, Typedef typedef) { | ||
var data = TypedefTemplateData(options, packageGraph, library, typedef, | ||
_sidebarForLibrary.getRenderFor); | ||
var data = TypedefTemplateData(options, packageGraph, library, typedef); | ||
var content = templates.renderTypedef(data); | ||
write(writer, typedef.filePath, data, content); | ||
runtimeStats.incrementAccumulator('writtenTypedefFileCount'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,8 +12,8 @@ import 'package:dartdoc/src/generator/html_resources.g.dart' as resources; | |
import 'package:dartdoc/src/generator/resource_loader.dart'; | ||
import 'package:dartdoc/src/generator/template_data.dart'; | ||
import 'package:dartdoc/src/generator/templates.dart'; | ||
import 'package:dartdoc/src/model/package.dart'; | ||
import 'package:dartdoc/src/model/package_graph.dart'; | ||
import 'package:dartdoc/src/model/model.dart'; | ||
import 'package:dartdoc/src/runtime_stats.dart'; | ||
import 'package:meta/meta.dart'; | ||
|
||
/// Creates a [Generator] with an [HtmlGeneratorBackend] backend. | ||
|
@@ -37,6 +37,52 @@ class HtmlGeneratorBackend extends GeneratorBackendBase { | |
HtmlGeneratorBackend( | ||
super.options, super.templates, super.writer, super.resourceProvider); | ||
|
||
@override | ||
void generateClass(PackageGraph packageGraph, Library library, Class clazz) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each of these which used to just render one HTML file must now render two, with the addition of their sidebar file, so they are overridden here, adding a lot of repeated boilerplate. |
||
super.generateClass(packageGraph, library, clazz); | ||
var data = ClassTemplateData(options, packageGraph, library, clazz); | ||
var sidebarContent = templates.renderSidebarForContainer(data); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't this undo the performance benefits of caching the sidebar? We're going to be running the template for the sidebar repeatedly unless I'm misreading something. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This shouldn't undo the caching benefits we had earlier (it should more-or-less mimic them): The problem we had before caching is that each page would independently generate it's left sidebar (sibling elements) and right sidebar (children), so that the left sidebar was being generated n+1 times for n siblings (by each of the n siblings, and once by the parent). The caching mechanism ensured each sidebar was generated once. This change does something similar: each TemplateData object is responsible for generating its own right sidebar, containing is children elements. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I had been misreading the code then. LGTM |
||
write(writer, clazz.sidebarPath, data, sidebarContent); | ||
runtimeStats.incrementAccumulator('writtenSidebarFileCount'); | ||
} | ||
|
||
@override | ||
void generateEnum(PackageGraph packageGraph, Library library, Enum eNum) { | ||
super.generateEnum(packageGraph, library, eNum); | ||
var data = EnumTemplateData(options, packageGraph, library, eNum); | ||
var sidebarContent = templates.renderSidebarForContainer(data); | ||
write(writer, eNum.sidebarPath, data, sidebarContent); | ||
runtimeStats.incrementAccumulator('writtenSidebarFileCount'); | ||
} | ||
|
||
@override | ||
void generateExtension( | ||
PackageGraph packageGraph, Library library, Extension extension) { | ||
super.generateExtension(packageGraph, library, extension); | ||
var data = ExtensionTemplateData(options, packageGraph, library, extension); | ||
var sidebarContent = templates.renderSidebarForContainer(data); | ||
write(writer, extension.sidebarPath, data, sidebarContent); | ||
runtimeStats.incrementAccumulator('writtenSidebarFileCount'); | ||
} | ||
|
||
@override | ||
void generateLibrary(PackageGraph packageGraph, Library library) { | ||
super.generateLibrary(packageGraph, library); | ||
var data = LibraryTemplateData(options, packageGraph, library); | ||
var sidebarContent = templates.renderSidebarForLibrary(data); | ||
write(writer, library.sidebarPath, data, sidebarContent); | ||
runtimeStats.incrementAccumulator('writtenSidebarFileCount'); | ||
} | ||
|
||
@override | ||
void generateMixin(PackageGraph packageGraph, Library library, Mixin mixin) { | ||
super.generateMixin(packageGraph, library, mixin); | ||
var data = MixinTemplateData(options, packageGraph, library, mixin); | ||
var sidebarContent = templates.renderSidebarForContainer(data); | ||
write(writer, mixin.sidebarPath, data, sidebarContent); | ||
runtimeStats.incrementAccumulator('writtenSidebarFileCount'); | ||
} | ||
|
||
@override | ||
void generatePackage(PackageGraph packageGraph, Package package) { | ||
super.generatePackage(packageGraph, package); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used to cache the sidebars generated, and no longer need to.