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

Search index checker #1442

Merged
merged 2 commits into from
Jun 1, 2017
Merged

Search index checker #1442

merged 2 commits into from
Jun 1, 2017

Conversation

jcollins-g
Copy link
Contributor

Add a search index checker for #1434 and make some enhancements to warnings for broken links.

The original issue #1434 would now result in more informative broken link errors:

 warning: dartdoc generated a broken link to: angular_components/ActivateItemOnKeyPressMixin/isRtl.html
    to element angular_components.MaterialDropdownSelectComponent.isRtl: (file:///usr/local/google/home/jcollins/dart/angular2_components/lib/src/model/a11y/keyboard_handler_mixin.dart:40:8)
    linked to from angular_components.MaterialDropdownSelectComponent.visible: (file:///usr/local/google/home/jcollins/dart/angular2_components/lib/src/components/material_select/material_dropdown_select.dart)
 warning: dartdoc generated a broken link to: angular_components/ActivateItemOnKeyPressMixin/handleEndKey.html
    to element angular_components.MaterialDropdownSelectComponent.handleEndKey: (file:///usr/local/google/home/jcollins/dart/angular2_components/lib/src/model/a11y/keyboard_handler_mixin.dart:120:8)
    linked to from angular_components.MaterialDropdownSelectComponent.onFocus: (file:///usr/local/google/home/jcollins/dart/angular2_components/lib/src/components/material_select/material_dropdown_select.dart:207:8)

It would appear that since 0.12.0 the search index being filled with incorrect hyperlinks is no longer happening, but here is output from simulated failures in the index.json link checker:

 warning: dartdoc generated a broken link to: angular_components/KeyUpBoundaryDirective/hashCode.html (from index.json), to element angular_components.KeyUpBoundaryDirective.hashCode: (file:///usr/local/google/home/jcollins/dart/all_sdks/1.24.0-dev.5.0/lib/core/object.dart)
 warning: dartdoc generated a broken link to: angular_components/MaterialInkTooltipComponent/tooltipRef.html (from index.json), to element angular_components.MaterialInkTooltipComponent.tooltipRef: (file:///usr/local/google/home/jcollins/dart/angular2_components/lib/src/components/material_tooltip/src/ink_tooltip.dart)
 warning: dartdoc generated a file not in the search index: angular_components/StringSelectionOptions-class.html (from index.json), from angular_components.StringSelectionOptions: (file:///usr/local/google/home/jcollins/dart/angular2_components/lib/src/model/selection/string_selection_options.dart:36:7)
 warning: dartdoc generated a file not in the search index: angular_components/TrackLayoutChangesMixin/noSuchMethod.html (from index.json), from angular_components.TrackLayoutChangesMixin.noSuchMethod: (file:///usr/local/google/home/jcollins/dart/all_sdks/1.24.0-dev.5.0/lib/core/object.dart:107:20)

@jcollins-g jcollins-g requested a review from devoncarew May 25, 2017 20:56
@googlebot googlebot added the cla: yes Google CLA check succeeded. label May 25, 2017
@kevmoo
Copy link
Member

kevmoo commented May 25, 2017

Drive by: how is this tested?

@jcollins-g
Copy link
Contributor Author

This is a test.

@jcollins-g
Copy link
Contributor Author

More specifically: this is an enhancement to dartdoc's self-checking, and so it didn't make much sense to me to spend a great deal of time testing outside of manual checks that the check works.

Nevertheless, dartdoc in general needs improved testing and I have open issues on that.

@@ -3092,6 +3097,8 @@ class Package implements Nameable, Documentable {
break;
case PackageWarning.brokenLink:
warningMessage = 'dartdoc generated a broken link to: ${message}';
warnablePrefix = 'to element';
referredFromPrefix = 'linked to from';
Copy link
Member

Choose a reason for hiding this comment

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

linked to from => referenced from?

@devoncarew
Copy link
Member

devoncarew commented May 26, 2017

lgtm

Not as part of this PR, but you may want to make the location output a bit more terse. In ModelElement String get elementLocation, you could prefer a path to a uri if it was a file: uri, and you could use a relative path if the source was contained in the cwd. That would convert

file:///usr/local/google/home/jcollins/dart/angular2_components/lib/src/model/a11y/keyboard_handler_mixin.dart:40:8

to

lib/src/model/a11y/keyboard_handler_mixin.dart:40:8

or perhaps

angular2_components/lib/src/model/a11y/keyboard_handler_mixin.dart:40:8

@kevmoo
Copy link
Member

kevmoo commented May 26, 2017

This is a test.

Right, but it's a user-facing feature – which should have it's own tests. Just curious..

@jcollins-g
Copy link
Contributor Author

@kevmoo I'd be happy to discuss my testing philosophy with you offline, feel free to drop something on my calendar.

@jcollins-g
Copy link
Contributor Author

re: shortening URLs, that should be a lot more possible since they're all generated from the same place now, just haven't gotten to it yet.

@jcollins-g jcollins-g merged commit 6baa5e6 into master Jun 1, 2017
@kevmoo kevmoo deleted the search-index-checker branch June 21, 2017 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants