Skip to content

Rearrange SDK PackageMeta instantiation to be similar to normal packages #1639

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

Merged
merged 36 commits into from
Mar 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
f733321
Fix or disable miscellaneous lints in dartdoc
jcollins-g Mar 12, 2018
668043e
Merge branch 'master' into fix-lints
jcollins-g Mar 15, 2018
6fedd10
Clean up a few more lints.
jcollins-g Mar 15, 2018
a32fd98
Rename category to package in all remaining references
jcollins-g Mar 15, 2018
b59a4df
dartfmt
jcollins-g Mar 15, 2018
bc64d2c
Review comments
jcollins-g Mar 15, 2018
50475c1
Merge branch 'fix-lints' into category-redo
jcollins-g Mar 15, 2018
6afc18b
Intermediate state
jcollins-g Mar 15, 2018
3ee7464
basic dartdoc options now working
jcollins-g Mar 15, 2018
5500887
Fix test failure (accidentally caching config info)
jcollins-g Mar 15, 2018
1e0fff4
Merge branch 'category-redo' into dartdoc-options-yaml
jcollins-g Mar 15, 2018
65d9d38
dartfmt
jcollins-g Mar 15, 2018
0ddc926
Merge branch 'fix-lints' into category-redo
jcollins-g Mar 15, 2018
a113ef8
dartfmt
jcollins-g Mar 15, 2018
47e3977
Merge branch 'category-redo' into dartdoc-options-yaml
jcollins-g Mar 15, 2018
e9b63db
Merge branch 'master' into fix-lints
jcollins-g Mar 16, 2018
62d30ca
Merge branch 'fix-lints' into category-redo
jcollins-g Mar 16, 2018
014a13f
Merge branch 'category-redo' into dartdoc-options-yaml
jcollins-g Mar 16, 2018
4c57a00
one last rename
jcollins-g Mar 16, 2018
45ff8d9
Merge branch 'category-redo' into dartdoc-options-yaml
jcollins-g Mar 16, 2018
59a50c6
Merge branch 'master' into category-redo
jcollins-g Mar 16, 2018
3737530
Merge branch 'category-redo' into dartdoc-options-yaml
jcollins-g Mar 16, 2018
ffc1149
Merge branch 'master' into dartdoc-options-yaml
jcollins-g Mar 19, 2018
de913dd
Remove overly complicated config inheritance; we can add it back if w…
jcollins-g Mar 19, 2018
76013a6
dartfmt
jcollins-g Mar 19, 2018
687ea5a
Unify SDK PackageMeta construction and add cache
jcollins-g Mar 20, 2018
9d0d8c5
update test package docs
jcollins-g Mar 20, 2018
216f6d2
PackageMeta construction sufficiently robust for refactor
jcollins-g Mar 20, 2018
aaba65d
Fix infinite loop and error messages
jcollins-g Mar 20, 2018
5883782
Rename path/p import directive to pathLib everywhere.
jcollins-g Mar 20, 2018
2873935
dartfmt
jcollins-g Mar 20, 2018
5d20ee6
Merge branch 'dartdoc-options-yaml' into dartdoc-sdkmeta-shuffle
jcollins-g Mar 20, 2018
092e2fe
'Dart SDK' + '' => 'Dart Core' + 'SDK'
jcollins-g Mar 20, 2018
17e8d6b
Merge branch 'master' into dartdoc-options-yaml
jcollins-g Mar 20, 2018
99b1c65
Merge branch 'dartdoc-options-yaml' into dartdoc-sdkmeta-shuffle
jcollins-g Mar 20, 2018
b8ccf9c
SDK name is no longer special
jcollins-g Mar 20, 2018
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
30 changes: 16 additions & 14 deletions bin/dartdoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,15 @@ main(List<String> arguments) async {
final bool sdkDocs = args['sdk-docs'];
final bool showProgress = args['show-progress'];

final String readme = args['sdk-readme'];
if (readme != null && !(new File(readme).existsSync())) {
stderr.writeln(
" fatal error: unable to locate the SDK description file at $readme.");
exit(1);
Directory inputDir;
if (sdkDocs) {
inputDir = sdkDir;
} else if (args['input'] == null) {
inputDir = Directory.current;
} else {
inputDir = args['input'];
}

Directory inputDir = new Directory(args['input']);
if (!inputDir.existsSync()) {
stderr.writeln(
" fatal error: unable to locate the input directory at ${inputDir
Expand Down Expand Up @@ -188,12 +189,12 @@ main(List<String> arguments) async {
});
}

PackageMeta packageMeta = sdkDocs
? new PackageMeta.fromSdk(sdkDir,
sdkReadmePath: readme,
displayAsPackages:
args['use-categories'] || args['display-as-packages'])
: new PackageMeta.fromDir(inputDir);
PackageMeta packageMeta = new PackageMeta.fromDir(inputDir);

if (packageMeta == null) {
stderr.writeln(' fatal error: Unable to generate documentation: no pubspec.yaml found');
exit(1);
}

if (!packageMeta.isValid) {
final String firstError = packageMeta.getInvalidReasons().first;
Expand All @@ -211,6 +212,7 @@ main(List<String> arguments) async {
}
}


logInfo("Generating documentation for '${packageMeta}' into "
"${outputDir.absolute.path}${Platform.pathSeparator}");

Expand Down Expand Up @@ -310,9 +312,9 @@ ArgParser _createArgsParser() {
help: 'Display progress indications to console stdout', negatable: false);
parser.addOption('sdk-readme',
help:
'Path to the SDK description file; use if generating Dart SDK docs.');
'Path to the SDK description file. Deprecated (ignored)');
parser.addOption('input',
help: 'Path to source directory.', defaultsTo: Directory.current.path);
help: 'Path to source directory.');
parser.addOption('output',
help: 'Path to output directory.', defaultsTo: defaultOutDir);
parser.addMultiOption('header',
Expand Down
31 changes: 6 additions & 25 deletions lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1710,7 +1710,6 @@ class Library extends ModelElement {
List<TopLevelVariable> _variables;
Namespace _exportedNamespace;
String _name;
String _packageName;
factory Library(LibraryElement element, PackageGraph packageGraph) {
return packageGraph.findOrCreateLibraryFor(element);
}
Expand Down Expand Up @@ -2015,16 +2014,7 @@ class Library extends ModelElement {

/// The real package, as opposed to the package we are documenting it with,
/// [PackageGraph.name]
String get packageName {
if (_packageName == null) {
if (name.startsWith('dart:')) {
_packageName = 'Dart Core';
} else {
_packageName = packageMeta?.name ?? '';
}
}
return _packageName;
}
String get packageName => packageMeta?.name ?? '';

/// The real packageMeta, as opposed to the package we are documenting with.
PackageMeta _packageMeta;
Expand Down Expand Up @@ -2187,16 +2177,7 @@ class Library extends ModelElement {

static PackageMeta getPackageMeta(LibraryElement element) {
String sourcePath = element.source.fullName;
File file = new File(pathLib.canonicalize(sourcePath));
Directory dir = file.parent;
while (dir.parent.path != dir.path && dir.existsSync()) {
File pubspec = new File(pathLib.join(dir.path, 'pubspec.yaml'));
if (pubspec.existsSync()) {
return new PackageMeta.fromDir(dir);
}
dir = dir.parent;
}
return null;
return new PackageMeta.fromDir(new File(pathLib.canonicalize(sourcePath)).parent);
}

static String getLibraryName(LibraryElement element) {
Expand Down Expand Up @@ -4204,7 +4185,7 @@ class PackageGraph extends Canonicalization with Nameable, Warnable {
String get name => packageMeta.name;

String get kind =>
(packageMeta.displayAsPackages || packageGraph.isSdk) ? '' : 'package';
(packageGraph.isSdk) ? 'SDK' : 'package';

@override
String get oneLineDoc => '';
Expand Down Expand Up @@ -4543,14 +4524,14 @@ class Package implements Comparable<Package> {
/// Returns:
/// -1 if this package is listed in --package-order.
/// 0 if this package is the original package we are documenting.
/// 1 if this group represents the Dart SDK.
/// 2 if this group has a name that contains the name of the original
/// 1 if this package represents the Dart SDK.
/// 2 if this package has a name that contains the name of the original
/// package we are documenting.
/// 3 otherwise.
int get _group {
if (config.packageOrder.contains(name)) return -1;
if (name.toLowerCase() == packageGraph.name.toLowerCase()) return 0;
if (name == "Dart Core") return 1;
if (isSdk) return 1;
if (name.toLowerCase().contains(packageGraph.name.toLowerCase())) return 2;
return 3;
}
Expand Down
64 changes: 43 additions & 21 deletions lib/src/package_meta.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,49 @@ library dartdoc.package_meta;

import 'dart:io';

import 'package:dartdoc/src/sdk.dart';
import 'package:path/path.dart' as pathLib;
import 'package:yaml/yaml.dart';

import 'logging.dart';


Map<String, PackageMeta> _packageMetaCache = {};

abstract class PackageMeta {
final Directory dir;
final bool displayAsPackages;

PackageMeta(this.dir, {this.displayAsPackages: false});

factory PackageMeta.fromDir(Directory dir) => new _FilePackageMeta(dir);
factory PackageMeta.fromSdk(Directory sdkDir,
{String sdkReadmePath, bool displayAsPackages}) =>
new _SdkMeta(sdkDir,
sdkReadmePath: sdkReadmePath, displayAsPackages: displayAsPackages);
PackageMeta(this.dir);

/// This factory is guaranteed to return the same object for any given
/// [dir.absolute.path]. Multiple [dir.absolute.path]s will resolve to the
/// same object if they are part of the same package. Returns null
/// if the directory is not part of a known package.
factory PackageMeta.fromDir(Directory dir) {
Directory original = dir.absolute;
dir = original;
if (!_packageMetaCache.containsKey(dir.path)) {
PackageMeta packageMeta;
// There are pubspec.yaml files inside the SDK. Ignore them.
if (pathLib.isWithin(getSdkDir().absolute.path, dir.path) || getSdkDir().path == dir.path) {
packageMeta = new _SdkMeta(getSdkDir());
} else {
while (dir.existsSync()) {
File pubspec = new File(pathLib.join(dir.path, 'pubspec.yaml'));
if (pubspec.existsSync()) {
packageMeta = new _FilePackageMeta(dir);
break;
}
// Allow a package to be at root (possible in a Windows setting with
// drive letter mappings).
if (dir.path == dir.parent.absolute.path) break;
dir = dir.parent.absolute;
}
}
_packageMetaCache[dir.absolute.path] = packageMeta;
}
return _packageMetaCache[dir.absolute.path];
}

bool get isSdk;
bool get needsPubGet => false;
Expand All @@ -46,7 +73,7 @@ abstract class PackageMeta {
FileContents getChangelogContents();

/// Returns true if we are a valid package, valid enough to generate docs.
bool get isValid;
bool get isValid => getInvalidReasons().isEmpty;

/// Returns a list of reasons this package is invalid, or an
/// empty list if no reasons found.
Expand Down Expand Up @@ -151,8 +178,6 @@ class _FilePackageMeta extends PackageMeta {
return _changelog;
}

@override
bool get isValid => getInvalidReasons().isEmpty;

/// Returns a list of reasons this package is invalid, or an
/// empty list if no reasons found.
Expand Down Expand Up @@ -184,10 +209,12 @@ File _locate(Directory dir, List<String> fileNames) {
}

class _SdkMeta extends PackageMeta {
final String sdkReadmePath;
String sdkReadmePath;

_SdkMeta(Directory dir, {this.sdkReadmePath, bool displayAsPackages})
: super(dir, displayAsPackages: displayAsPackages);
_SdkMeta(Directory dir)
: super(dir) {
sdkReadmePath = pathLib.join(dir.path, 'lib', 'api_readme.md');
}

@override
bool get isSdk => true;
Expand All @@ -198,7 +225,7 @@ class _SdkMeta extends PackageMeta {
}

@override
String get name => 'Dart SDK';
String get name => 'Dart';
@override
String get version =>
new File(pathLib.join(dir.path, 'version')).readAsStringSync().trim();
Expand All @@ -211,15 +238,10 @@ class _SdkMeta extends PackageMeta {

@override
FileContents getReadmeContents() {
File f = sdkReadmePath != null
? new File(sdkReadmePath)
: new File(pathLib.join(dir.path, 'lib', 'api_readme.md'));
File f = new File(pathLib.join(dir.path, 'lib', 'api_readme.md'));
return f.existsSync() ? new FileContents(f) : null;
}

@override
bool get isValid => true;

@override
List<String> getInvalidReasons() => [];

Expand Down
3 changes: 1 addition & 2 deletions lib/templates/index.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{{>head}}

<div class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left">
<h5>{{self.name}} {{self.kind}}</h5>
<h5><span class="package-name">{{self.name}}</span> <span class="package-kind">{{self.kind}}</span></h5>

{{#displayAsPackages}}
<ol>
Expand All @@ -11,7 +11,6 @@ <h5>{{self.name}} {{self.kind}}</h5>
<li>{{{linkedName}}}</li>
{{/publicLibraries}}
{{/packageGraph.publicPackages}}

</ol>
{{/displayAsPackages}}

Expand Down
2 changes: 1 addition & 1 deletion lib/templates/library.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{{>head}}

<div class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left">
<h5>{{parent.name}} {{parent.kind}}</h5>
<h5><span class="package-name">{{parent.name}}</span> <span class="package-kind">{{parent.kind}}</span></h5>
{{#displayAsPackages}}
<ol>
{{#packageGraph.publicPackages}}
Expand Down
3 changes: 2 additions & 1 deletion test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ void main() {
});

test('sdk name', () {
expect(sdkAsPackageGraph.name, equals('Dart SDK'));
expect(sdkAsPackageGraph.name, equals('Dart'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW: equals('Dart') can be simplified to 'Dart'...

expect(sdkAsPackageGraph.kind, equals('SDK'));
});

test('sdk homepage', () {
Expand Down
16 changes: 5 additions & 11 deletions test/package_meta_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,19 @@ import 'dart:io';

import 'package:dartdoc/src/package_meta.dart';
import 'package:dartdoc/src/sdk.dart';
import 'package:path/path.dart' as pathLib;
import 'package:test/test.dart';

void main() {
group('PackageMeta for a directory without a pubspec', () {
PackageMeta p;

setUp(() {
var d = new Directory(pathLib.join(
Directory.current.path, 'testing/test_package_not_valid'));
if (!d.existsSync()) {
throw "$d cannot be found";
}
var d = Directory.systemTemp.createTempSync('test_package_not_valid');
p = new PackageMeta.fromDir(d);
});

test('is not valid', () {
expect(p.isValid, isFalse);
expect(p.getInvalidReasons(), isNotEmpty);
expect(p.getInvalidReasons(), contains('no pubspec.yaml found'));
expect(p, isNull);
});
});

Expand Down Expand Up @@ -76,15 +69,16 @@ void main() {
});

group('PackageMeta.fromSdk', () {
PackageMeta p = new PackageMeta.fromSdk(getSdkDir());
PackageMeta p = new PackageMeta.fromDir(getSdkDir());

test('has a name', () {
expect(p.name, 'Dart SDK');
expect(p.name, 'Dart');
});

test('is valid', () {
expect(p.isValid, isTrue);
expect(p.getInvalidReasons(), isEmpty);
expect(p.isSdk, isTrue);
});

test('has a version', () {
Expand Down
2 changes: 1 addition & 1 deletion test/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void delete(Directory dir) {

init() async {
sdkDir = getSdkDir();
sdkPackageMeta = new PackageMeta.fromSdk(sdkDir);
sdkPackageMeta = new PackageMeta.fromDir(sdkDir);
setConfig();

testPackageGraph = await bootBasicPackage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<main>

<div class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left">
<h5>test_package package</h5>
<h5><span class="package-name">test_package</span> <span class="package-kind">package</span></h5>

<ol>
<li class="section-title"><a href="index.html#libraries">Libraries</a></li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<main>

<div class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left">
<h5>test_package package</h5>
<h5><span class="package-name">test_package</span> <span class="package-kind">package</span></h5>

<ol>
<li class="section-title"><a href="index.html#libraries">Libraries</a></li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<main>

<div class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left">
<h5>test_package package</h5>
<h5><span class="package-name">test_package</span> <span class="package-kind">package</span></h5>

<ol>
<li class="section-title"><a href="index.html#libraries">Libraries</a></li>
Expand Down
2 changes: 1 addition & 1 deletion testing/test_package_docs/css/css-library.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<main>

<div class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left">
<h5>test_package package</h5>
<h5><span class="package-name">test_package</span> <span class="package-kind">package</span></h5>

<ol>
<li class="section-title"><a href="index.html#libraries">Libraries</a></li>
Expand Down
2 changes: 1 addition & 1 deletion testing/test_package_docs/ex/ex-library.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<main>

<div class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left">
<h5>test_package package</h5>
<h5><span class="package-name">test_package</span> <span class="package-kind">package</span></h5>

<ol>
<li class="section-title"><a href="index.html#libraries">Libraries</a></li>
Expand Down
2 changes: 1 addition & 1 deletion testing/test_package_docs/fake/fake-library.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<main>

<div class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left">
<h5>test_package package</h5>
<h5><span class="package-name">test_package</span> <span class="package-kind">package</span></h5>

<ol>
<li class="section-title"><a href="index.html#libraries">Libraries</a></li>
Expand Down
Loading