Skip to content

Commit

Permalink
warn about links with empty href destination
Browse files Browse the repository at this point in the history
I often make blank links as a note to myself to remember to fill in
a URL. If I forget, though, there will be a link that just brings the
user to the top of the page, which isn't what I want. So now, we warn on
these empty links. If you want to bring the user to the top of the page,
use an explicit `#` character.

Fixes #492
  • Loading branch information
preaction committed May 9, 2016
1 parent ce9b108 commit 8fa1abd
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 5 deletions.
19 changes: 16 additions & 3 deletions lib/Statocles/Plugin/LinkCheck.pm
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ sub check_pages {

my %page_paths = ();
my %links = ();
my %empty = (); # Pages with empty links
for my $page ( @{ $event->pages } ) {
$page_paths{ $page->path } = 1;
if ( $page->DOES( 'Statocles::Page::Document' ) ) {
Expand All @@ -47,8 +48,13 @@ sub check_pages {
for my $attr ( qw( src href ) ) {
for my $el ( $dom->find( "[$attr]" )->each ) {
my $url = $el->attr( $attr );

if ( !$url ) {
push @{ $empty{ $page->path } }, $el;
}

$url =~ s{#.*$}{};
next unless $url;
next unless $url; # Skip checking fragment-internal links for now
next if $url =~ m{^(?:[a-z][a-z0-9+.-]*):}i;
next if $url =~ m{^//};
if ( $url !~ m{^/} ) {
Expand All @@ -68,18 +74,25 @@ sub check_pages {
}
}

my @missing; # Array of arrayrefs of [ link => page ] pairs
my @missing; # Array of arrayrefs of [ link_url, page_path, link_element ]
for my $link_url ( keys %links ) {
$link_url .= 'index.html' if $link_url =~ m{/$};
next if $page_paths{ $link_url } || $page_paths{ "$link_url/index.html" };
next if grep { $link_url =~ /^$_/ } @{ $self->ignore };
push @missing, [ $link_url, $_ ] for keys %{ $links{ $link_url } };
}

for my $page_url ( keys %empty ) {
push @missing, map { [ '', $page_url, $_ ] } @{ $empty{ $page_url } };
}

if ( @missing ) {
# Sort by page url and then missing link url
for my $m ( sort { $a->[1] cmp $b->[1] || $a->[0] cmp $b->[0] } @missing ) {
$event->emitter->log->warn( "URL broken on $m->[1]: '$m->[0]' not found" );
my $msg = $m->[0] ? sprintf( q{'%s' not found}, $m->[0] )
: sprintf( q{Link with text "%s" has no destination}, $m->[2]->text )
;
$event->emitter->log->warn( "URL broken on $m->[1]: $msg" );
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions t/app/blog/pages.t
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ my @page_tests = (
"Full URLs without schema are not broken" => '//example.com',
"Fixed relative URL" => '/blog/2014/06/02/more_tags/docs.html',
"Test a mailto: link" => 'mailto:[email protected]',
'No link destination' => '/blog/2014/06/02/more_tags/',
);
my @links = $dom->find( 'article .content a' )->each;
is scalar @links, scalar keys %links, 'all links found';
Expand Down Expand Up @@ -813,6 +814,7 @@ my @page_tests = (
"Full URLs without schema are not broken" => '//example.com',
"Fixed relative URL" => 'docs.html',
"Test a mailto: link" => 'mailto:[email protected]',
'No link destination' => '',
);
my @links = $dom->find( 'section a' )->each;
is scalar @links, scalar keys %links, 'all links found';
Expand Down
7 changes: 6 additions & 1 deletion t/plugin/link_check.t
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ subtest 'check links' => sub {
[ ignore(), warn => re(qr{\QURL broken on /blog/2014/06/02/more_tags/docs.html: '/missing/favicon.png' not found} ) ],
[ ignore(), warn => re(qr{\QURL broken on /blog/2014/06/02/more_tags/docs.html: '/missing/script.js' not found} ) ],
[ ignore(), warn => re(qr{\QURL broken on /blog/2014/06/02/more_tags/docs.html: '/missing/stylesheet.css' not found} ) ],
[ ignore(), warn => re(qr{\QURL broken on /blog/2014/06/02/more_tags/index.html: Link with text "No link destination" has no destination}) ],
[ ignore(), warn => re(qr{\QURL broken on /blog/2014/06/02/more_tags/index.html: '/blog/2014/06/02/does_not_exist' not found}) ],
[ ignore(), warn => re(qr{\QURL broken on /blog/2014/06/02/more_tags/index.html: '/blog/2014/06/02/more_tags/current_does_not_exist' not found}) ],
[ ignore(), warn => re(qr{\QURL broken on /blog/2014/06/02/more_tags/index.html: '/blog/2014/06/02/more_tags/does_not_exist' not found}) ],
Expand Down Expand Up @@ -89,6 +90,7 @@ subtest 'ignore patterns' => sub {

cmp_deeply $site->log->history,
bag(
[ ignore(), warn => re(qr{\QURL broken on /blog/2014/06/02/more_tags/index.html: Link with text "No link destination" has no destination}) ],
[ ignore(), warn => re(qr{\QURL broken on /blog/2014/06/02/more_tags/index.html: '/blog/2014/06/02/does_not_exist' not found}) ],
[ ignore(), warn => re(qr{\QURL broken on /blog/2014/06/02/more_tags/index.html: '/blog/2014/06/02/more_tags/current_does_not_exist' not found}) ],
[ ignore(), 'warn', re(qr{\QURL broken on $page: '/blog/2014/06/02/more_tags/does_not_exist' not found}) ],
Expand Down Expand Up @@ -116,7 +118,10 @@ subtest 'ignore patterns' => sub {

my $page = '/blog/2014/06/02/more_tags/index.html';

cmp_deeply $site->log->history, [],
cmp_deeply $site->log->history,
[
[ ignore(), warn => re(qr{\QURL broken on /blog/2014/06/02/more_tags/index.html: Link with text "No link destination" has no destination}) ],
],
'all broken links ignored'
or diag explain $site->log->history;
};
Expand Down
3 changes: 3 additions & 0 deletions t/share/app/blog/2014/06/02/more_tags/index.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ links:
[Fixed relative URL](docs.html)

[Test a mailto: link](mailto:[email protected])

[No link destination]()

2 changes: 1 addition & 1 deletion t/share/theme/site/layout.html.ep
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
</nav>
<%= $content %>
<footer>
<a href="<%= $site->data->{profile_url} // '' %>">Profile</a>
<a href="<%= $site->data->{profile_url} // '#' %>">Profile</a>
<div id="app-info"><%= $self->app->data->{info} // '' %></div>
</footer>
</body>
Expand Down

0 comments on commit 8fa1abd

Please sign in to comment.