Refactor html generator into frontend and backend #2115
Conversation
lib/src/generator_frontend.dart
Outdated
| import 'package:dartdoc/src/warnings.dart'; | ||
| import 'package:path/path.dart' as path; | ||
|
|
||
| typedef FileWriter = File Function(String filePath, Object content, |
There was a problem hiding this comment.
Would it be better if this were an abstract class?
There was a problem hiding this comment.
See comment on usage of FileWriter elsewhere.
lib/src/generator_frontend.dart
Outdated
|
|
||
| GeneratorFrontEnd(this._generatorBackend, this._writer); | ||
|
|
||
| // Implement FileWriter so we can wrap the one given to us. |
There was a problem hiding this comment.
This part feels a little weird, but we need to accomplish two things:
- Fix path separators
- Apply the
outputDirectorygiven to us, required by signature ofgenerate()
I think delegating to a writer is still appropriate because it can be mocked in tests. Perhaps with the writer being a separate component, we could remove outputDirectory from the signature of Generator.generate() and instead have DartdocFileWriter do this work instead.
There was a problem hiding this comment.
There is an awful lot of joining/splitting of outputDirectory going on (and that was in the original). Anything we can do to reduce the number of times we are recombining paths seems like a good idea; I've just never went through and cleaned this up.
There was a problem hiding this comment.
Latest push no longer has this "wrapping" structure, and all this path manipulation happens in one place, DartdocFileWriter.
| @@ -0,0 +1,377 @@ | |||
| // Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file | |||
There was a problem hiding this comment.
This file was html_generator_instance.dart . Most of the structure is maintained, but there's enough difference to convince Git that it's a new file.
lib/src/generator_frontend.dart
Outdated
| import 'package:dartdoc/src/warnings.dart'; | ||
| import 'package:path/path.dart' as path; | ||
|
|
||
| typedef FileWriter = File Function(String filePath, Object content, |
There was a problem hiding this comment.
See comment on usage of FileWriter elsewhere.
lib/src/generator_frontend.dart
Outdated
|
|
||
| GeneratorFrontEnd(this._generatorBackend, this._writer); | ||
|
|
||
| // Implement FileWriter so we can wrap the one given to us. |
There was a problem hiding this comment.
There is an awful lot of joining/splitting of outputDirectory going on (and that was in the original). Anything we can do to reduce the number of times we are recombining paths seems like a good idea; I've just never went through and cleaned this up.
lib/src/generator_frontend.dart
Outdated
| logInfo('Generating docs for category ${category.name} from ' | ||
| '${category.package.fullyQualifiedName}...'); | ||
| indexAccumulator.add(category); | ||
| _generatorBackend.generateCategory(write, packageGraph, category); |
There was a problem hiding this comment.
Yes, this approach is an improvement. Eventually it seems like we should be able to refactor and use a visitor pattern but I think this is definitely good enough for now.
lib/dartdoc.dart
Outdated
| List<Generator> generators = await initHtmlGenerators(config); | ||
| return Dartdoc._(config, generators); | ||
| return Dartdoc._( | ||
| config, await initHtmlGenerator(config, DartdocFileWriter().write)); |
There was a problem hiding this comment.
This suggests to me that DartdocFileWriter could be a class implementing call() -- this way you could just pass an instance of DartdocFileWriter and dispense with the typedef.
Make FileWriter an abstract class and give it the responsibility for recording written filenames. This removes the FileWriter wrapping in GeneratorFrontEnd.
This splits the generator logic into a frontend and backend. The frontend walks the package graph, delegates to the backend to render documentation for an element, and forwards rendered content to a
FileWriterto be written.There is currently one backend, for html content.