Skip to content
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

Preserve newline following {@endtemplate}. #2289

Merged
merged 4 commits into from
Aug 11, 2020
Merged
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
11 changes: 6 additions & 5 deletions lib/src/model/comment_processable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import 'package:dartdoc/src/warnings.dart';
import 'package:path/path.dart' as path;

final _templatePattern = RegExp(
r'[ ]*{@template\s+(.+?)}([\s\S]+?){@endtemplate}[ ]*\n?',
r'[ ]*{@template\s+(.+?)}([\s\S]+?){@endtemplate}[ ]*(\n?)',
multiLine: true);
final _htmlPattern = RegExp(
r'[ ]*{@inject-html\s*}([\s\S]+?){@end-inject-html}[ ]*\n?',
Expand Down Expand Up @@ -366,9 +366,6 @@ mixin CommentProcessable on Documentable, Warnable, Locatable, SourceCodeMixin {
/// The width and height must be integers specifying the dimensions of the
/// video file in pixels.
String _injectAnimations(String rawDocs) {
// Make sure we have a set to keep track of used IDs for this href.
package.usedAnimationIdsByHref[href] ??= {};

String getUniqueId(String base) {
var animationIdCount = 1;
var id = '$base$animationIdCount';
Expand All @@ -382,6 +379,9 @@ mixin CommentProcessable on Documentable, Warnable, Locatable, SourceCodeMixin {
}

return rawDocs.replaceAllMapped(_basicAnimationPattern, (basicMatch) {
// Make sure we have a set to keep track of used IDs for this href.
package.usedAnimationIdsByHref[href] ??= {};

var parser = ArgParser();
parser.addOption('id');
var args = _parseArgs(basicMatch[1], parser, 'animation');
Expand Down Expand Up @@ -481,8 +481,9 @@ mixin CommentProcessable on Documentable, Warnable, Locatable, SourceCodeMixin {
return rawDocs.replaceAllMapped(_templatePattern, (match) {
var name = match[1].trim();
var content = match[2].trim();
var trailingNewline = match[3];
packageGraph.addMacro(name, content);
return '{@macro $name}';
return '{@macro $name}$trailingNewline';
});
}

Expand Down
1 change: 1 addition & 0 deletions lib/src/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export 'canonicalization.dart';
export 'categorization.dart';
export 'category.dart';
export 'class.dart';
export 'comment_processable.dart';
export 'constructor.dart';
export 'container.dart';
export 'container_member.dart';
Expand Down
1 change: 1 addition & 0 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ dev_dependencies:
grinder: ^0.8.2
io: ^0.3.0
http: ^0.12.0
mockito: ^4.1.1
pedantic: ^1.9.0
test: ^1.3.0

Expand Down
10 changes: 10 additions & 0 deletions test/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Tests

Most of dartdoc's tests are large end-to-end tests which read real files in
real packages, in the `testing/` directory. Unit tests exist in `test/unit/`.

Many of the end-to-end test cases should be rewritten as unit tests.

At some point, the distinction should flip, such that unit tests are generally
located in `test/`, and end-to-end tests are found in a specific directory, or
in files whose names signify the distinction.
2 changes: 1 addition & 1 deletion test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,7 @@ void main() {

test("renders a macro within the same comment where it's defined", () {
expect(withMacro.documentation,
equals('Macro method\n\nFoo macro content\nMore docs'));
equals('Macro method\n\nFoo macro content\n\nMore docs'));
});

test("renders a macro in another method, not the same where it's defined",
Expand Down
8 changes: 6 additions & 2 deletions test/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,12 @@ class SubprocessLauncher {

var exitCode = await process.exitCode;
if (exitCode != 0) {
throw ProcessException(executable, arguments,
'SubprocessLauncher got non-zero exitCode: $exitCode', exitCode);
throw ProcessException(
executable,
arguments,
'SubprocessLauncher got non-zero exitCode: $exitCode\n\n'
'stdout: ${process.stdout}\n\nstderr: ${process.stderr}',
exitCode);
}
return jsonObjects;
}
Expand Down
195 changes: 195 additions & 0 deletions test/unit/comment_processable_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:io' show Directory;
Copy link
Member

Choose a reason for hiding this comment

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

nit: copyright


import 'package:dartdoc/src/dartdoc_options.dart';
import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/package_meta.dart';
import 'package:dartdoc/src/warnings.dart';
import 'package:mockito/mockito.dart';
import 'package:test/test.dart';

void main() {
_Processor processor;
setUp(() {
processor = _Processor(_FakeDartdocOptionContext());
processor.href = '/project/a.dart';
});

test('removes triple slashes', () async {
var doc = await processor.processComment('''
/// Text.
/// More text.
''');
expect(doc, equals('''
Text.
More text.'''));
});

test('removes space after triple slashes', () async {
var doc = await processor.processComment('''
/// Text.
/// More text.
''');
// TODO(srawlins): Actually, the three spaces before 'More' is perhaps not
// the best fit. Should it only be two, to match the indent from the first
// line's "Text"?
expect(doc, equals('''
Text.
More text.'''));
});

test('leaves blank lines', () async {
var doc = await processor.processComment('''
/// Text.
///
/// More text.
''');
expect(doc, equals('''
Text.

More text.'''));
});

test('processes @template', () async {
var doc = await processor.processComment('''
/// Text.
///
/// {@template abc}
/// Template text.
/// {@endtemplate}
///
/// End text.
''');
expect(doc, equals('''
Text.

{@macro abc}

End text.'''));
verify(processor.packageGraph.addMacro('abc', 'Template text.')).called(1);
});

test('processes leading @template', () async {
var doc = await processor.processComment('''
/// {@template abc}
/// Template text.
/// {@endtemplate}
///
/// End text.
''');
expect(doc, equals('''
{@macro abc}

End text.'''));
verify(processor.packageGraph.addMacro('abc', 'Template text.')).called(1);
});

test('processes trailing @template', () async {
var doc = await processor.processComment('''
/// Text.
///
/// {@template abc}
/// Template text.
/// {@endtemplate}
''');
expect(doc, equals('''
Text.

{@macro abc}'''));
verify(processor.packageGraph.addMacro('abc', 'Template text.')).called(1);
});

test('processes @template w/o blank line following', () async {
var doc = await processor.processComment('''
/// Text.
///
/// {@template abc}
/// Template text.
/// {@endtemplate}
/// End text.
''');
expect(doc, equals('''
Text.

{@macro abc}
End text.'''));
verify(processor.packageGraph.addMacro('abc', 'Template text.')).called(1);
});

test('allows whitespace around @template name', () async {
var doc = await processor.processComment('''
/// {@template abc }
/// Template text.
/// {@endtemplate}
''');
expect(doc, equals('''
{@macro abc}'''));
verify(processor.packageGraph.addMacro('abc', 'Template text.')).called(1);
});

// TODO(srawlins): More unit tests: @example, @youtube, @animation,
// @inject-html, @tool.
}

/// In order to mix in [CommentProcessable], we must first implement
/// the super-class constraints.
abstract class __Processor extends Fake
implements Documentable, Warnable, Locatable, SourceCodeMixin {}

/// A simple comment processor for testing [CommentProcessable].
class _Processor extends __Processor with CommentProcessable {
@override
final _FakeDartdocOptionContext config;

@override
final _FakePackage package;

@override
final _MockPackageGraph packageGraph;

@override
String href;

_Processor(this.config)
: package = _FakePackage(),
packageGraph = _MockPackageGraph() {
throwOnMissingStub(packageGraph);
when(packageGraph.addMacro(any, any)).thenReturn(null);
}
}

class _FakeDirectory extends Fake implements Directory {
@override
final String path;

_FakeDirectory() : path = '/project';
}

class _FakePackage extends Fake implements Package {
@override
final PackageMeta packageMeta;

_FakePackage() : packageMeta = _FakePackageMeta();
}

class _FakePackageMeta extends Fake implements PackageMeta {
@override
final Directory dir;

_FakePackageMeta() : dir = _FakeDirectory();
}

class _FakeDartdocOptionContext extends Fake implements DartdocOptionContext {
@override
final bool allowTools;

@override
final bool injectHtml;

_FakeDartdocOptionContext({this.allowTools = false, this.injectHtml = false});
}

class _MockPackageGraph extends Mock implements PackageGraph {}