Extract fileName / fileType from ModelElement, LibraryContainer into FileStructure #3413
Conversation
| 'Unrecognized LibraryContainer subtype: ${libraryContainer.runtimeType}'); | ||
| } | ||
|
|
||
| return FileStructureImpl( |
There was a problem hiding this comment.
It seems to me a lot of ceremony, to declare kindAddition, and pathSafeName, maybe re-assign to them, and then just return a new FileStructureImpl. Up to you but it could just be:
switch (libraryContainer) {
case Category():
return FileStructureImpl(
libraryContainer.config.format, libraryContainer.name, 'topic');
case Package():
return FileStructureImpl(
libraryContainer.config.format, 'index', null);
default:
throw ...
}There was a problem hiding this comment.
Hmmm. I think the fact I rewrote this several times is leaking through. Yes, that's much better. Fixing here and in _fromModelElement.
| String get htmlId => throw UnimplementedError(); | ||
| final String fileType; | ||
| String pathSafeName; | ||
| final String? kindAddition; |
There was a problem hiding this comment.
Would you mind documenting what this field is?
There was a problem hiding this comment.
Not at all. It's kind of hacky so definitely deserves some description.
test/dartdoc_test_base.dart
Outdated
| @@ -70,23 +72,37 @@ analyzer: | |||
| packagePath, name, Uri.file('$packagePath/')); | |||
| } | |||
|
|
|||
| Future<PackageGraph> _bootPackageFromFiles(FileGenerator files) async { | |||
There was a problem hiding this comment.
I'm a little confused by the name. files doesn't seem like something you'd invoke. fileGenerator?
There was a problem hiding this comment.
Yes. I'm a little confused by why I have a function as a parameter here at all. Deleted that, so now the name makes sense.
test/dartdoc_test_base.dart
Outdated
| {String libraryPreamble = ''}) async { | ||
| await d.dir('lib', [ | ||
| d.file('lib.dart', ''' | ||
| {String libraryPreamble = '', FileGenerator? extraFiles}) async { |
There was a problem hiding this comment.
The new extraFiles parameter looks unused?
There was a problem hiding this comment.
good catch, missed that in a refactor.
This is the first extraction to the new structure and I think it worked out well; the way we construct filenames is now much more comprehensible and all in one place rather than spread throughout the class structure. I've deprecated the old interface so we can avoid major version bumps during this intermediate phase.
Future PRs will extract href, htmlId, and the other as-yet-unimplemented members of FileStructure as appropriate and tie them in to the generator.