Skip to content

Commit 75c4956

Browse files
srawlinssrawlins
committed
Introduce a new URL for library pages, at index.html
Fixes dart-lang#1346 This "moves" the URL of each library from, e.g. 'package-two_lib2/package-two_lib2-library.html' to 'package-two_lib2/index.html', such that we can have links with 'package-two_lib2'. The old URLs are preserved via redirecting HTML files. (HTTP redirects are more standard and recommended, but dartdoc does not contain an HTTP server; HTTP redirects could be implemented at the hosting server, which is different for different doc hosts (Google Storage, Firebase, ...). Instead we use simpler HTML redirects. In fact, a given backend host could use the HTML redirecting files as a database to inform HTTP redirect rules, that need to be implemented differently for each backend. So dartdoc now produces redirecting files, but a given backend could instead return an HTTP 301 Permanent Redirect response when requesting one of the redirecting files.) This adds a negligible number of files (1 per documented library), and a negligible number of bytes (tiny redirect files), but actually is probably a net negative in terms of bytes, as many links are now shortened, removing the HTML file name of library URLs.
1 parent 4c82b49 commit 75c4956

14 files changed

+245
-46
lines changed

‎lib/src/generator/generator_backend.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,14 @@ abstract class GeneratorBackend {
179179
runtimeStats.incrementAccumulator('writtenFunctionFileCount');
180180
}
181181

182-
/// Emits documentation content for the [library].
182+
/// Emits documentation content for the [library], and the content for the
183+
/// library's previous location (which just redirects to the new location).
183184
void generateLibrary(PackageGraph packageGraph, Library library) {
184185
var data = LibraryTemplateData(options, packageGraph, library);
185186
var content = templates.renderLibrary(data);
187+
var redirectContent = templates.renderLibraryRedirect(data);
186188
write(writer, library.filePath, data, content);
189+
write(writer, library.redirectingPath, data, redirectContent);
187190
runtimeStats.incrementAccumulator('writtenLibraryFileCount');
188191
}
189192

‎lib/src/generator/templates.aot_renderers_for_html.dart

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,29 @@ String renderLibrary(LibraryTemplateData context0) {
11371137
return buffer.toString();
11381138
}
11391139

1140+
String renderLibraryRedirect(LibraryTemplateData context0) {
1141+
final buffer = StringBuffer();
1142+
buffer.write('''<!DOCTYPE html>
1143+
<html lang="en">
1144+
<head>
1145+
<meta http-equiv="refresh" content="0; url=''');
1146+
if (context0.useBaseHref) {
1147+
var context1 = context0.htmlBase;
1148+
buffer.write(context0.htmlBase);
1149+
buffer.writeEscaped(context0.self.href);
1150+
}
1151+
buffer.write('''" />
1152+
</head>
1153+
<body>
1154+
<p><a href="''');
1155+
buffer.writeEscaped(context0.self.href);
1156+
buffer.write('''">Redirect</a></p>
1157+
</body>
1158+
</html>''');
1159+
1160+
return buffer.toString();
1161+
}
1162+
11401163
String renderMethod(MethodTemplateData context0) {
11411164
final buffer = StringBuffer();
11421165
buffer.write(_renderMethod_partial_head_0(context0));

‎lib/src/generator/templates.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
@Renderer(#renderFunction, Context<FunctionTemplateData>(), 'function')
2323
@Renderer(#renderIndex, Context<PackageTemplateData>(), 'index')
2424
@Renderer(#renderLibrary, Context<LibraryTemplateData>(), 'library')
25+
@Renderer(
26+
#renderLibraryRedirect, Context<LibraryTemplateData>(), 'library_redirect')
2527
@Renderer(#renderMethod, Context<MethodTemplateData>(), 'method')
2628
@Renderer(#renderMixin, Context<MixinTemplateData>(), 'mixin')
2729
@Renderer(#renderProperty, Context<PropertyTemplateData>(), 'property')
@@ -99,6 +101,7 @@ abstract class Templates {
99101
String renderFunction(FunctionTemplateData context);
100102
String renderIndex(PackageTemplateData context);
101103
String renderLibrary(LibraryTemplateData context);
104+
String renderLibraryRedirect(LibraryTemplateData context);
102105
String renderMethod(MethodTemplateData context);
103106
String renderMixin(MixinTemplateData context);
104107
String renderProperty(PropertyTemplateData context);
@@ -174,6 +177,10 @@ class HtmlAotTemplates implements Templates {
174177
String renderLibrary(LibraryTemplateData context) =>
175178
aot_renderers_for_html.renderLibrary(context);
176179

180+
@override
181+
String renderLibraryRedirect(LibraryTemplateData context) =>
182+
aot_renderers_for_html.renderLibraryRedirect(context);
183+
177184
@override
178185
String renderMethod(MethodTemplateData context) =>
179186
aot_renderers_for_html.renderMethod(context);
@@ -251,6 +258,10 @@ class RuntimeTemplates implements Templates {
251258
String renderLibrary(LibraryTemplateData context) =>
252259
runtime_renderers.renderLibrary(context, _libraryTemplate);
253260

261+
@override
262+
String renderLibraryRedirect(LibraryTemplateData context) =>
263+
runtime_renderers.renderLibraryRedirect(context, _libraryTemplate);
264+
254265
@override
255266
String renderMethod(MethodTemplateData context) =>
256267
runtime_renderers.renderMethod(context, _methodTemplate);

‎lib/src/generator/templates.runtime_renderers.dart

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8443,6 +8443,28 @@ class _Renderer_Library extends RendererBase<Library> {
84438443
parent: r));
84448444
},
84458445
),
8446+
'redirectingPath': Property(
8447+
getValue: (CT_ c) => c.redirectingPath,
8448+
renderVariable:
8449+
(CT_ c, Property<CT_> self, List<String> remainingNames) {
8450+
if (remainingNames.isEmpty) {
8451+
return self.getValue(c).toString();
8452+
}
8453+
var name = remainingNames.first;
8454+
var nextProperty =
8455+
_Renderer_String.propertyMap().getValue(name);
8456+
return nextProperty.renderVariable(
8457+
self.getValue(c) as String,
8458+
nextProperty,
8459+
[...remainingNames.skip(1)]);
8460+
},
8461+
isNullValue: (CT_ c) => false,
8462+
renderValue: (CT_ c, RendererBase<CT_> r,
8463+
List<MustachioNode> ast, StringSink sink) {
8464+
_render_String(c.redirectingPath, ast, r.template, sink,
8465+
parent: r);
8466+
},
8467+
),
84468468
'referenceChildren': Property(
84478469
getValue: (CT_ c) => c.referenceChildren,
84488470
renderVariable: (CT_ c, Property<CT_> self,
@@ -8808,6 +8830,12 @@ class _Renderer_LibraryTemplateData extends RendererBase<LibraryTemplateData> {
88088830
}
88098831
}
88108832

8833+
String renderLibraryRedirect(LibraryTemplateData context, Template template) {
8834+
var buffer = StringBuffer();
8835+
_render_LibraryTemplateData(context, template.ast, template, buffer);
8836+
return buffer.toString();
8837+
}
8838+
88118839
class _Renderer_Locatable extends RendererBase<Locatable> {
88128840
static final Map<Type, Object> _propertyMapCache = {};
88138841
static Map<String, Property<CT_>> propertyMap<CT_ extends Locatable>() =>
@@ -12482,13 +12510,13 @@ class _Renderer_PackageTemplateData extends RendererBase<PackageTemplateData> {
1248212510
}
1248312511
}
1248412512

12485-
String renderIndex(PackageTemplateData context, Template template) {
12513+
String renderError(PackageTemplateData context, Template template) {
1248612514
var buffer = StringBuffer();
1248712515
_render_PackageTemplateData(context, template.ast, template, buffer);
1248812516
return buffer.toString();
1248912517
}
1249012518

12491-
String renderError(PackageTemplateData context, Template template) {
12519+
String renderIndex(PackageTemplateData context, Template template) {
1249212520
var buffer = StringBuffer();
1249312521
_render_PackageTemplateData(context, template.ast, template, buffer);
1249412522
return buffer.toString();

‎lib/src/model/library.dart

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ class Library extends ModelElement
199199
String get filePath => '$dirName/$fileName';
200200

201201
@override
202-
String get fileName => '$dirName-library.html';
202+
String get fileName => 'index.html';
203203

204204
String get sidebarPath => '$dirName/$dirName-library-sidebar.html';
205205

@@ -216,9 +216,20 @@ class Library extends ModelElement
216216
if (!identical(canonicalModelElement, this)) {
217217
return canonicalModelElement?.href;
218218
}
219-
return '${package.baseHref}$filePath';
219+
// The file name for a library is 'index.html', so we just link to the
220+
// directory name. This keeps the URL looking short, _without_ the
221+
// 'index.html' in the URL.
222+
return '${package.baseHref}$dirName';
220223
}
221224

225+
/// The previous value of [filePath].
226+
///
227+
/// This path is used to write a file that ontains an HTML redirect (not an
228+
/// HTTP redirect) to a library's current [filePath].
229+
String get redirectingPath => '$dirName/$dirName-library.html';
230+
231+
/// Whether a libary is anonymous, either because it has no library directive
232+
/// or it has a library directive without a name.
222233
bool get isAnonymous => element.name.isEmpty;
223234

224235
@override

‎lib/src/model/package.dart

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,13 @@ class Package extends LibraryContainer
136136
bool get isPublic =>
137137
_isLocalPublicByDefault || libraries.any((l) => l.isPublic);
138138

139-
/// Return true if this is the default package, this is part of an embedder
140-
/// SDK, or if [DartdocOptionContext.autoIncludeDependencies] is true -- but
141-
/// only if the package was not excluded on the command line.
139+
/// Whether this package is local.
140+
///
141+
/// A package can be local in three ways:
142+
/// * this is the default package,
143+
/// * this is part of an embedder SDK, or
144+
/// * [DartdocOptionContext.autoIncludeDependencies] is true, and this
145+
/// package is not excluded with `exclude-packages`.
142146
bool get isLocal {
143147
// Do not document as local if we excluded this package by name.
144148
if (_isExcluded) return false;

‎lib/src/validator.dart

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,7 @@ class Validator {
6161
return;
6262
}
6363
_visited.add(fullPath);
64-
final links = pageLinks.links;
65-
final baseHref = pageLinks.baseHref;
64+
var (links, baseHref) = pageLinks;
6665

6766
// Prevent extremely large stacks by storing the paths we are using
6867
// here instead -- occasionally, very large jobs have overflowed
@@ -75,10 +74,8 @@ class Validator {
7574
for (final href in links) {
7675
final uri = Uri.tryParse(href);
7776
if (uri == null || !uri.hasAuthority && !uri.hasFragment) {
78-
var linkPath = '$pathDirectory/$href';
79-
80-
linkPath = path.normalize(linkPath);
81-
final newFullPath = path.join(_origin, linkPath);
77+
var linkPath = path.normalize(path.url.join(pathDirectory, href));
78+
var newFullPath = path.join(_origin, linkPath);
8279
if (!_visited.contains(newFullPath)) {
8380
toVisit.add((linkPath, newFullPath));
8481
_visited.add(newFullPath);
@@ -156,13 +153,17 @@ class Validator {
156153
found.add(indexPath);
157154
for (var entry in jsonData.cast<Map<String, dynamic>>()) {
158155
if (entry.containsKey('href')) {
159-
final entryPath = path
160-
.joinAll([_origin, ...path.posix.split(entry['href'] as String)]);
161-
if (!_visited.contains(entryPath)) {
162-
_warn(PackageWarning.brokenLink, entryPath, _origin,
156+
var href =
157+
path.joinAll([_origin, ...path.url.split(entry['href'] as String)]);
158+
if (path.extension(href).isEmpty) {
159+
// An aliased link to an `index.html` file.
160+
href = path.url.join(href, 'index.html');
161+
}
162+
if (!_visited.contains(href)) {
163+
_warn(PackageWarning.brokenLink, href, _origin,
163164
referredFrom: fullPath);
164165
}
165-
found.add(entryPath);
166+
found.add(href);
166167
}
167168
}
168169
final missingFromSearch = _visited.difference(found);
@@ -177,7 +178,7 @@ class Validator {
177178
///
178179
/// This is extracted to save memory during the check; be careful not to hang
179180
/// on to anything referencing the full file and doc tree.
180-
_PageLinks? _getLinksAndBaseHref(String fullPath) {
181+
(Set<String> links, String? baseHref)? _getLinksAndBaseHref(String fullPath) {
181182
final file = _config.resourceProvider.getFile(fullPath);
182183
if (!file.exists) {
183184
return null;
@@ -196,15 +197,20 @@ class Validator {
196197
final stringLinks = links
197198
.map((link) => link.attributes['href'])
198199
.nonNulls
199-
.where((href) =>
200-
href.isNotEmpty &&
201-
!href.startsWith('https:') &&
202-
!href.startsWith('http:') &&
203-
!href.startsWith('mailto:') &&
204-
!href.startsWith('ftp:'))
200+
.where((uri) =>
201+
uri.isNotEmpty &&
202+
!uri.startsWith('https:') &&
203+
!uri.startsWith('http:') &&
204+
!uri.startsWith('mailto:') &&
205+
!uri.startsWith('ftp:'))
206+
.map((uri) =>
207+
// An aliased link to an `index.html` file.
208+
path.extension(uri).isEmpty
209+
? path.url.join(uri, 'index.html')
210+
: uri)
205211
.toSet();
206212

207-
return _PageLinks(stringLinks, baseHref);
213+
return (stringLinks, baseHref);
208214
}
209215

210216
/// Warns on erroneous file paths.
@@ -246,11 +252,3 @@ class Validator {
246252
message: message, referredFrom: referredFromElements);
247253
}
248254
}
249-
250-
/// Stores all links found in a page, and the base href.
251-
class _PageLinks {
252-
final Set<String> links;
253-
final String? baseHref;
254-
255-
const _PageLinks(this.links, this.baseHref);
256-
}

‎lib/templates/library_redirect.html

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta http-equiv="refresh" content="0; url={{ #useBaseHref }}{{ #htmlBase }}{{{ htmlBase }}}{{ /htmlBase }}{{ self.href }}{{ /useBaseHref }}" />
5+
</head>
6+
<body>
7+
<p><a href="{{ self.href }}">Redirect</a></p>
8+
</body>
9+
</html>

‎test/dartdoc_test_base.dart

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ abstract class DartdocTestBase {
4343

4444
String get sdkConstraint => '>=3.6.0 <4.0.0';
4545

46+
/// A mapping of pub dependency names to the paths.
47+
///
48+
/// These are written out to a pubspec file using `path` keys.
49+
Map<String, String> get pubDependencyPaths => const {};
50+
4651
List<String> get experiments => ['enhanced-parts', 'wildcard-variables'];
4752

4853
bool get skipUnreachableSdkLibraries => true;
@@ -61,7 +66,10 @@ abstract class DartdocTestBase {
6166
}
6267

6368
Future<void> _setUpPackage() async {
64-
var pubspec = d.buildPubspecText(sdkConstraint: sdkConstraint);
69+
var pubspec = d.buildPubspecText(
70+
sdkConstraint: sdkConstraint,
71+
dependencies: pubDependencyPaths,
72+
);
6573
String? analysisOptions;
6674
if (experiments.isNotEmpty) {
6775
analysisOptions = '''

‎test/end2end/model_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1221,7 +1221,7 @@ void main() async {
12211221
expect(
12221222
aFunctionUsingRenamedLib.documentationAsHtml,
12231223
contains(
1224-
'Link to library: <a href="${htmlBasePlaceholder}mylibpub/mylibpub-library.html">renamedLib</a>'),
1224+
'Link to library: <a href="${htmlBasePlaceholder}mylibpub">renamedLib</a>'),
12251225
);
12261226
expect(
12271227
aFunctionUsingRenamedLib.documentationAsHtml,

0 commit comments

Comments
 (0)