Skip to content

Remove the include-externals option #4027

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions lib/src/dartdoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,6 @@ class Dartdoc {
runtimeStats.startPerfTask('buildPackageGraph');
var packageGraph = await packageBuilder.buildPackageGraph();
runtimeStats.endPerfTask();
if (packageBuilder.includeExternalsWasSpecified) {
packageGraph.defaultPackage.warn(
PackageWarning.deprecated,
message:
"The '--include-externals' option is deprecated, and will soon be "
'removed.',
);
}
var libs = packageGraph.libraryCount;
logInfo("Initialized dartdoc with $libs librar${libs == 1 ? 'y' : 'ies'}");

Expand Down
11 changes: 1 addition & 10 deletions lib/src/dartdoc_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1179,9 +1179,6 @@ class DartdocOptionContext extends DartdocOptionContextBase
late final Set<String> include =
Set.of(optionSet['include'].valueAt(context));

List<String> get includeExternal =>
optionSet['includeExternal'].valueAt(context);

bool get includeSource => optionSet['includeSource'].valueAt(context);

bool get injectHtml => optionSet['injectHtml'].valueAt(context);
Expand Down Expand Up @@ -1399,7 +1396,7 @@ List<DartdocOption> createDartdocOptions(
DartdocOptionArgOnly<bool>(
'autoIncludeDependencies', false, resourceProvider,
help: 'Include all the used libraries into the docs, even the ones not '
'in the current package or "include-external"',
'in the current package',
negatable: true),
DartdocOptionArgFile<List<String>>(
'categoryOrder', const [], resourceProvider,
Expand Down Expand Up @@ -1431,12 +1428,6 @@ List<DartdocOption> createDartdocOptions(
mustExist: true),
DartdocOptionArgFile<List<String>>('include', [], resourceProvider,
help: 'Names of libraries to document.', splitCommas: true),
DartdocOptionArgFile<List<String>>('includeExternal', [], resourceProvider,
optionIs: OptionKind.file,
help: 'Additional (external) dart files to include; use '
'"<directory name>/<file name>", as in "lib/material.dart".',
mustExist: true,
splitCommas: true),
DartdocOptionArgOnly<bool>('includeSource', true, resourceProvider,
help: 'Show source code blocks.', negatable: true),
DartdocOptionArgOnly<bool>('injectHtml', false, resourceProvider,
Expand Down
1 change: 0 additions & 1 deletion lib/src/generator/templates.runtime_renderers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25738,7 +25738,6 @@ const _invisibleGetters = {
'flutterRoot',
'hashCode',
'include',
'includeExternal',
'includeSource',
'injectHtml',
'inputDir',
Expand Down
37 changes: 3 additions & 34 deletions lib/src/model/package_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@ import 'package:path/path.dart' as p show Context;
abstract class PackageBuilder {
// Builds package graph to be used by documentation generator.
Future<PackageGraph> buildPackageGraph();

/// The `include-external` option is deprecated, so we track whether it was
/// used, to report it.
bool get includeExternalsWasSpecified;
}

/// A package builder that understands pub package format.
Expand Down Expand Up @@ -283,11 +279,6 @@ class PubPackageBuilder implements PackageBuilder {
processedLibraries.add(resolvedLibrary.element);
}
files.addAll(newFiles);
var externals = _includedExternalsFrom(newFiles);
if (externals.isNotEmpty) {
includeExternalsWasSpecified = true;
}
files.addAll(externals);

var packages = _packageMetasForFiles(files.difference(_knownParts));
filesInCurrentPass = {...files.difference(_knownParts)};
Expand Down Expand Up @@ -384,25 +375,11 @@ class PubPackageBuilder implements PackageBuilder {
return dirs;
}

/// Calculates 'includeExternal' based on a list of files.
///
/// Assumes each file might be part of a [DartdocOptionContext], and loads
/// those objects to find any [DartdocOptionContext.includeExternal]
/// configurations therein.
List<String> _includedExternalsFrom(Iterable<String> files) => [
for (var file in files)
...DartdocOptionContext.fromContext(
_config,
_config.resourceProvider.getFile(file),
_config.resourceProvider,
).includeExternal,
];

/// Returns the set of files that may contain elements that need to be
/// documented.
///
/// This takes into account the 'auto-include-dependencies' option, the
/// 'exclude' option, and the 'include-external' option.
/// This takes into account the 'auto-include-dependencies' option, and the
/// 'exclude' option.
Future<Set<String>> _getFilesToDocument() async {
if (_config.topLevelPackageMeta.isSdk) {
return _sdkFilesToDocument
Expand All @@ -412,12 +389,7 @@ class PubPackageBuilder implements PackageBuilder {
var packagesToDocument = await _findPackagesToDocument(
_config.inputDir,
);
var files = _findFilesToDocumentInPackage(packagesToDocument).toList();
var externals = _includedExternalsFrom(files);
if (externals.isNotEmpty) {
includeExternalsWasSpecified = true;
files = [...files, ...externals];
}
var files = _findFilesToDocumentInPackage(packagesToDocument);
return {
...files.map(
(s) => _pathContext.absolute(_resourceProvider.getFile(s).path)),
Expand Down Expand Up @@ -446,9 +418,6 @@ class PubPackageBuilder implements PackageBuilder {
};
}

@override
bool includeExternalsWasSpecified = false;

Iterable<String> get _embedderSdkFiles => [
for (var dartUri in _embedderSdkUris)
_pathContext.absolute(_resourceProvider
Expand Down
23 changes: 10 additions & 13 deletions lib/src/model/package_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -170,15 +170,14 @@ class PackageGraph with CommentReferable, Nameable {
e.canonicalModelElement == null ||
e is Library ||
e.enclosingElement!.isCanonical) {
for (var d in e.documentationFrom
.where((d) => d.hasDocumentationComment)) {
for (var d
in e.documentationFrom.where((d) => d.hasDocumentationComment)) {
if (d.needsPrecache && !precachedElements.contains(d)) {
precachedElements.add(d as ModelElement);
futures.add(d.precacheLocalDocs());
// [TopLevelVariable]s get their documentation from getters and
// setters, so should be precached if either has a template.
if (e is TopLevelVariable &&
!precachedElements.contains(e)) {
if (e is TopLevelVariable && !precachedElements.contains(e)) {
precachedElements.add(e);
futures.add(e.precacheLocalDocs());
}
Expand Down Expand Up @@ -647,7 +646,8 @@ class PackageGraph with CommentReferable, Nameable {
checkAndAddContainer(modelElement, container);
}
} else if (container is Mixin) {
for (var modelElement in container.superclassConstraints.modelElements) {
for (var modelElement
in container.superclassConstraints.modelElements) {
checkAndAddContainer(modelElement, container);
}
}
Expand Down Expand Up @@ -752,8 +752,7 @@ class PackageGraph with CommentReferable, Nameable {
// TODO(keertip): Find a better way to exclude members of extensions
// when libraries are specified using the "--include" flag.
if (library != null && library.isDocumented) {
return getModelFor(e, library,
enclosingContainer: preferredClass);
return getModelFor(e, library, enclosingContainer: preferredClass);
}
}
// TODO(jcollins-g): The data structures should be changed to eliminate
Expand All @@ -778,8 +777,7 @@ class PackageGraph with CommentReferable, Nameable {
var setterElement = setter2 == null
? null
: getModelFor(setter2, library) as Accessor;
canonicalModelElement = getModelForPropertyInducingElement(
e, library,
canonicalModelElement = getModelForPropertyInducingElement(e, library,
getter: getterElement, setter: setterElement);
} else {
canonicalModelElement = getModelFor(e, library);
Expand All @@ -791,8 +789,7 @@ class PackageGraph with CommentReferable, Nameable {
}
}
// Prefer fields and top-level variables.
if (e is PropertyAccessorElement2 &&
canonicalModelElement is Accessor) {
if (e is PropertyAccessorElement2 && canonicalModelElement is Accessor) {
canonicalModelElement = canonicalModelElement.enclosingCombo;
}
return canonicalModelElement;
Expand All @@ -804,8 +801,8 @@ class PackageGraph with CommentReferable, Nameable {
var elem = modelElement.element;
var candidates = <ModelElement>{};
if (lib != null) {
var constructedWithKey = allConstructedModelElements[
ConstructedModelElementsKey(elem, null)];
var constructedWithKey =
allConstructedModelElements[ConstructedModelElementsKey(elem, null)];
if (constructedWithKey != null) {
candidates.add(constructedWithKey);
}
Expand Down
29 changes: 0 additions & 29 deletions test/packages_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -330,35 +330,6 @@ dartdoc:
packageGraph.packages.singleWhere((p) => p.name == 'one');
expect(packageOne.documentedWhere, equals(DocumentLocation.missing));
});

test(
'includes external remote elements when includeExternal is specified',
() async {
packageOneRoot
.getChildAssumingFile('dartdoc_options.yaml')
.writeAsStringSync('''
dartdoc:
includeExternal:
- bin/script.dart
linkTo:
url: 'https://mypub.topdomain/%n%/%v%'
''');
var packageGraph = await utils.bootBasicPackage(
packageTwoRoot.path, packageMetaProvider, packageConfigProvider,
additionalArguments: ['--link-to-remote']);

expect(packageGraph.packages, hasLength(3));
var packageOne =
packageGraph.packages.singleWhere((p) => p.name == 'one');
expect(packageOne.documentedWhere, equals(DocumentLocation.remote));
// TODO(srawlins): Why is there more than one?
var libraryScript = packageOne.allLibraries.named('script');
var classScript = libraryScript.classesAndExceptions.named('Script');
expect(
classScript.href,
equals(
'https://mypub.topdomain/one/0.0.1/script/Script-class.html'));
});
});

group('SDK package', () {
Expand Down
2 changes: 1 addition & 1 deletion test/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Future<DartdocOptionContext> contextFromArgv(
'dartdoc', [createDartdocOptions], packageMetaProvider);
optionSet.parseArguments(argv);
return DartdocOptionContext.fromDefaultContextLocation(
optionSet, pubPackageMetaProvider.resourceProvider);
optionSet, packageMetaProvider.resourceProvider);
}

/// Convenience factory to build a [DartdocGeneratorOptionContext] and
Expand Down