Skip to content

Commit

Permalink
tweaks from code review
Browse files Browse the repository at this point in the history
Closes #857

Squashed commit of the following:

commit e12a65d
Author: Seth Ladd <sethladd@google.com>
Date:   Mon Aug 24 13:39:41 2015 -0700

    tweaks from code review

commit 4f6bc8b
Author: Seth Ladd <sethladd@google.com>
Date:   Mon Aug 24 12:52:44 2015 -0700

    slightly darker blue for links and masthead

commit abdb0c5
Author: Seth Ladd <sethladd@google.com>
Date:   Mon Aug 24 12:45:59 2015 -0700

    don't hint that you can open the docs from a file: URI

commit 7d01861
Author: Seth Ladd <sethladd@google.com>
Date:   Mon Aug 24 12:42:50 2015 -0700

    print message if we fail to load the search index

commit 5b0f920
Author: Seth Ladd <sethladd@google.com>
Date:   Mon Aug 24 12:39:06 2015 -0700

    add files

commit 52ec401
Author: Seth Ladd <sethladd@google.com>
Date:   Mon Aug 24 12:38:50 2015 -0700

    display explicit getter and setter for properties
  • Loading branch information
sethladd committed Aug 24, 2015
1 parent 75989c2 commit 5877d0b
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 54 deletions.
19 changes: 12 additions & 7 deletions bin/dartdoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ main(List<String> arguments) async {

try {
DartDocResults results = await dartdoc.generateDocs();
print('\nSuccess! Open file://${results.outDir.absolute.path}/index.html');
print('\nSuccess! Docs generated into ${results.outDir.absolute.path}');
} catch (e, st) {
if (e is DartDocFailure) {
stderr.writeln('Generation failed: ${e}.');
Expand Down Expand Up @@ -152,31 +152,36 @@ ArgParser _createArgsParser() {
parser.addFlag('version',
help: 'Display the version for $name.', negatable: false);
parser.addOption('dart-sdk',
help: "Location of the Dart SDK. Use if SDK isn't automatically located.");
help:
"Location of the Dart SDK. Use if SDK isn't automatically located.");
parser.addFlag('sdk-docs',
help: 'Generate ONLY the docs for the Dart SDK.', negatable: false);
parser.addOption('sdk-readme',
help: 'Path to the SDK description file. Use if generating Dart SDK docs.');
help:
'Path to the SDK description file. Use if generating Dart SDK docs.');
parser.addOption('input',
help: 'Path to source directory', defaultsTo: Directory.current.path);
parser.addOption('output',
help: 'Path to output directory.', defaultsTo: defaultOutDir);
parser.addOption('header',
help: 'path to file containing HTML text, inserted into the header of every page.');
help:
'path to file containing HTML text, inserted into the header of every page.');
parser.addOption('footer',
help: 'path to file containing HTML text, inserted into the footer of every page.');
help:
'path to file containing HTML text, inserted into the footer of every page.');
parser.addOption('package-root', help: 'The path to the package root.');
parser.addOption('url-mapping',
help: '--url-mapping=libraryUri,/path/to/library.dart directs dartdoc to '
'use "library.dart" as the source for an import of "libraryUri"',
'use "library.dart" as the source for an import of "libraryUri"',
allowMultiple: true,
splitCommas: false);
parser.addOption('exclude',
help: 'Comma-separated list of library names to ignore.');
parser.addOption('include',
help: 'Comma-separated list of library names to generate docs for.');
parser.addOption('hosted-url',
help: 'URL where the docs will be hosted (used to generate the sitemap).');
help:
'URL where the docs will be hosted (used to generate the sitemap).');
return parser;
}

Expand Down
3 changes: 3 additions & 0 deletions lib/resources/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ function initSearch() {
searchIndex = JSON.parse(jsonReq.responseText);
initTypeahead();
});
jsonReq.addEventListener('error', function() {
$('#search-box').prop('placeholder', 'Error loading search index');
});
jsonReq.send();
}

Expand Down
4 changes: 2 additions & 2 deletions lib/resources/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ nav .container {
}

header {
background-color: #2196F3;
background-color: rgb(0, 102, 152);
color: white;
box-shadow: 0 1px 4px 0 rgba(0, 0, 0, 0.37);
}
Expand Down Expand Up @@ -138,7 +138,7 @@ p.no-docs {
}

a, a:hover {
color: #1976D2;
color: rgb(0, 102, 152);
}

pre.prettyprint {
Expand Down
4 changes: 3 additions & 1 deletion lib/src/html_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ class Templates {
'source_code',
'sidebar_for_library',
'name_link',
'has_more_docs'
'has_more_docs',
'accessor_getter',
'accessor_setter'
];

for (String partial in partials) {
Expand Down
46 changes: 27 additions & 19 deletions lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ abstract class ModelElement implements Comparable, Nameable, Documentable {
String get _computeDocumentationComment =>
element.computeDocumentationComment();

/// Returns the docs, stripped of their
/// leading comments syntax.
///
/// This getter will walk up the inheritance hierarchy
/// to find docs, if the current class doesn't have docs
/// for this element.
String get documentation {
if (_rawDocs != null) return _rawDocs;

Expand Down Expand Up @@ -1384,8 +1390,8 @@ class Field extends ModelElement

bool get hasSetter => _field.setter != null;

PropertyAccessorElement get getter => _field.getter;
PropertyAccessorElement get setter => _field.setter;
PropertyAccessorElement get _getter => _field.getter;
PropertyAccessorElement get _setter => _field.setter;

String computeDocumentationComment() => _field.computeDocumentationComment();

Expand Down Expand Up @@ -1610,38 +1616,40 @@ class Accessor extends ModelElement implements EnclosedElement {
abstract class GetterSetterCombo {
bool get hasGetter;
bool get hasSetter;
Library get library;

PropertyAccessorElement get getter;
PropertyAccessorElement get setter;
PropertyAccessorElement get _getter;
PropertyAccessorElement get _setter;

String computeDocumentationComment();
Accessor get getter {
return _getter == null ? null : new ModelElement.from(_getter, library);
}

Accessor get setter {
return _setter == null ? null : new ModelElement.from(_setter, library);
}

// TODO: now that we have explicit getter and setters, we probably
// want a cleaner way to do this. Only the one-liner is using this
// now. The detail pages should be using getter and setter directly.
String get _computeDocumentationComment {
var buffer = new StringBuffer();

if (hasGetter) {
String docs = stripComments(getter.computeDocumentationComment());
String docs = stripComments(_getter.computeDocumentationComment());
if (docs != null) buffer.write(docs);
}

if (hasSetter && !setter.isSynthetic) {
String docs = stripComments(setter.computeDocumentationComment());
if (hasSetter && !_setter.isSynthetic) {
String docs = stripComments(_setter.computeDocumentationComment());
if (docs != null) {
if (buffer.isNotEmpty) buffer.write('\n\n');
buffer.write(docs);
}
}

if (buffer.isNotEmpty) return buffer.toString();

// TODO: check that we'd ever get here. Doesn't seem like we would.
// This is old.
return computeDocumentationComment();
return buffer.toString();
}

String get getterDocsAsHtml => '';

String get setterDocsAsHtml => '';
}

/// Top-level variables. But also picks up getters and setters?
Expand Down Expand Up @@ -1685,8 +1693,8 @@ class TopLevelVariable extends ModelElement
bool get hasGetter => _variable.getter != null;
bool get hasSetter => _variable.setter != null;

PropertyAccessorElement get getter => _variable.getter;
PropertyAccessorElement get setter => _variable.setter;
PropertyAccessorElement get _getter => _variable.getter;
PropertyAccessorElement get _setter => _variable.setter;

String computeDocumentationComment() {
return _variable.computeDocumentationComment();
Expand Down
9 changes: 9 additions & 0 deletions lib/templates/_accessor_getter.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<section id="getter">

<section class="multi-line-signature">
<span class="returntype">{{{ linkedReturnType }}}</span>
{{>name_summary}}
</section>

{{>documentation}}
</section>
9 changes: 9 additions & 0 deletions lib/templates/_accessor_setter.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<section id="setter">

<section class="multi-line-signature">
<span class="returntype">void</span>
{{>name_summary}}<span class="signature">(<wbr>{{{ linkedParamsNoMetadata }}})
</section>

{{>documentation}}
</section>
19 changes: 7 additions & 12 deletions lib/templates/property.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,13 @@ <h5><a href="{{href}}">{{name}}</a></h5>

<div class="col-xs-12 col-sm-9 col-md-8 main-content">

<section class="multi-line-signature">
{{#property}}
<span class="returntype">{{{ linkedReturnType }}}</span>
{{>name_summary}}

{{>readable_writable}}
{{/property}}
</section>

{{#property}}
{{>documentation}}
{{/property}}
{{#property.getter}}
{{>accessor_getter}}
{{/property.getter}}

{{#property.setter}}
{{>accessor_setter}}
{{/property.setter}}

</div> <!-- /.main-content -->

Expand Down
17 changes: 6 additions & 11 deletions lib/templates/top_level_property.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,13 @@ <h5><a href="{{href}}">{{name}}</a></h5>

<div class="col-xs-12 col-sm-9 col-md-8 main-content">

<section class="multi-line-signature">
{{#property}}
<span class="returntype">{{{ linkedReturnType }}}</span>
{{>name_summary}}
{{#property.getter}}
{{>accessor_getter}}
{{/property.getter}}

{{>readable_writable}}
{{/property}}
</section>

{{#property}}
{{>documentation}}
{{/property}}
{{#property.setter}}
{{>accessor_setter}}
{{/property.setter}}

</div> <!-- /.main-content -->

Expand Down
53 changes: 51 additions & 2 deletions test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,45 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
() {
expect(sFromApple.documentationAsHtml.contains('/**'), isFalse);
});

test('explicit getter/setter has a getter accessor', () {
expect(lengthX.getter, isNotNull);
expect(lengthX.getter.name, equals('lengthX'));
});

test('explicit getter/setter has a setter accessor', () {
expect(lengthX.setter, isNotNull);
expect(lengthX.setter.name, equals('lengthX='));
});

test('a stand-alone setter does not have a getter', () {
expect(onlySetter.getter, isNull);
});
});

group('Accessor', () {
Accessor onlyGetterGetter,
onlyGetterSetter,
onlySetterGetter,
onlySetterSetter;

setUp(() {
TopLevelVariable justGetter =
fakeLibrary.properties.firstWhere((p) => p.name == 'justGetter');
onlyGetterGetter = justGetter.getter;
onlyGetterSetter = justGetter.setter;
TopLevelVariable justSetter =
fakeLibrary.properties.firstWhere((p) => p.name == 'justSetter');
onlySetterSetter = justSetter.setter;
onlySetterGetter = justSetter.getter;
});

test('are available on top-level variables', () {
expect(onlyGetterGetter.name, equals('justGetter'));
expect(onlyGetterSetter, isNull);
expect(onlySetterGetter, isNull);
expect(onlySetterSetter.name, equals('justSetter='));
});
});

group('Top-level Variable', () {
Expand Down Expand Up @@ -879,12 +918,12 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
expect(v3.linkedReturnType, 'dynamic');
});

test('getter documentation', () {
test('just a getter has documentation', () {
expect(justGetter.documentation,
equals('Just a getter. No partner setter.'));
});

test('setter documentation', () {
test('just a setter has documentation', () {
expect(justSetter.documentation,
equals('Just a setter. No partner getter.'));
});
Expand All @@ -893,6 +932,16 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
expect(setAndGet.documentation, contains('The getter for setAndGet.'));
expect(setAndGet.documentation, contains('The setter for setAndGet.'));
});

test('has a getter accessor', () {
expect(setAndGet.getter, isNotNull);
expect(setAndGet.getter.name, equals('setAndGet'));
});

test('has a setter accessor', () {
expect(setAndGet.setter, isNotNull);
expect(setAndGet.setter.name, equals('setAndGet='));
});
});

group('Constant', () {
Expand Down

0 comments on commit 5877d0b

Please sign in to comment.