From 677c16d5255d6b4ab0c6b63b896395f00bacf10a Mon Sep 17 00:00:00 2001 From: Garen Torikian Date: Thu, 22 Sep 2016 15:54:43 -0700 Subject: [PATCH] Report on the content of the error, when able --- README.md | 3 +- lib/html-proofer/check.rb | 8 +++--- lib/html-proofer/check/favicon.rb | 2 +- lib/html-proofer/check/images.rb | 13 +++++---- lib/html-proofer/check/links.rb | 47 ++++++++++++++++--------------- lib/html-proofer/check/scripts.rb | 7 +++-- lib/html-proofer/issue.rb | 11 ++++++-- spec/html-proofer/proofer_spec.rb | 1 + 8 files changed, 50 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index ebe69d67..ab66640e 100644 --- a/README.md +++ b/README.md @@ -299,7 +299,7 @@ HTML-Proofer can be as noisy or as quiet as you'd like. If you set the `:log_lev Want to write your own test? Sure, that's possible! -Just create a class that inherits from `HTMLProofer::Check`. This subclass must define one method called `run`. This is called on your content, and is responsible for performing the validation on whatever elements you like. When you catch a broken issue, call `add_issue(message, line: line)` to explain the error. +Just create a class that inherits from `HTMLProofer::Check`. This subclass must define one method called `run`. This is called on your content, and is responsible for performing the validation on whatever elements you like. When you catch a broken issue, call `add_issue(message, line: line, content: content)` to explain the error. `line` refers to the line numbers, and `content` is the node content of the broken element. If you're working with the element's attributes (as most checks do), you'll also want to call `create_element(node)` as part of your suite. This constructs an object that contains all the attributes of the HTML element you're iterating on. @@ -373,4 +373,3 @@ Project | Repository [Raspberry Pi's documentation](http://www.raspberrypi.org/documentation/) | [raspberrypi/documentation](https://github.com/raspberrypi/documentation) [Squeak's website](http://squeak.org) | [squeak-smalltalk/squeak.org](https://github.com/squeak-smalltalk/squeak.org) [Atom Flight Manual](http://flight-manual.atom.io) | [atom/flight-manual.atom.io](https://github.com/atom/flight-manual.atom.io) - diff --git a/lib/html-proofer/check.rb b/lib/html-proofer/check.rb index cbd9ccf4..78c2c87a 100644 --- a/lib/html-proofer/check.rb +++ b/lib/html-proofer/check.rb @@ -18,14 +18,14 @@ def create_element(node) end def run - fail NotImplementedError, 'HTMLProofer::Check subclasses must implement #run' + raise NotImplementedError, 'HTMLProofer::Check subclasses must implement #run' end - def add_issue(desc, line: nil, status: -1) - @issues << Issue.new(@path, desc, line: line, status: status) + def add_issue(desc, line: nil, status: -1, content: nil) + @issues << Issue.new(@path, desc, line: line, status: status, content: content) end - def add_to_external_urls(url, _) + def add_to_external_urls(url) return if @external_urls[url] add_path_for_url(url) end diff --git a/lib/html-proofer/check/favicon.rb b/lib/html-proofer/check/favicon.rb index 23ae0c53..efa21896 100644 --- a/lib/html-proofer/check/favicon.rb +++ b/lib/html-proofer/check/favicon.rb @@ -10,6 +10,6 @@ def run return if found - add_issue 'no favicon specified' + add_issue('no favicon specified') end end diff --git a/lib/html-proofer/check/images.rb b/lib/html-proofer/check/images.rb index e29d2974..32ecd74d 100644 --- a/lib/html-proofer/check/images.rb +++ b/lib/html-proofer/check/images.rb @@ -17,32 +17,33 @@ def run @html.css('img').each do |node| @img = create_element(node) line = node.line + content = node.content next if @img.ignore? # screenshot filenames should return because of terrible names if terrible_filename? - add_issue("image has a terrible filename (#{@img.url})", line: line) + add_issue("image has a terrible filename (#{@img.url})", line: line, content: content) next end # does the image exist? if missing_src? - add_issue('image has no src or srcset attribute', line: line) + add_issue('image has no src or srcset attribute', line: line, content: content) else if @img.remote? - add_to_external_urls(@img.url, line) + add_to_external_urls(@img.url) elsif !@img.exists? - add_issue("internal image #{@img.url} does not exist", line: line) + add_issue("internal image #{@img.url} does not exist", line: line, content: content) end end if !@img.ignore_alt? && (@img.alt.nil? || (empty_alt_tag? && !@img.ignore_empty_alt?)) - add_issue("image #{@img.url} does not have an alt attribute", line: line) + add_issue("image #{@img.url} does not have an alt attribute", line: line, content: content) end if @img.check_img_http? && @img.scheme == 'http' - add_issue("image #{@img.url} uses the http scheme", line: line) + add_issue("image #{@img.url} uses the http scheme", line: line, content: content) end end diff --git a/lib/html-proofer/check/links.rb b/lib/html-proofer/check/links.rb index 40954070..8edb18f6 100644 --- a/lib/html-proofer/check/links.rb +++ b/lib/html-proofer/check/links.rb @@ -12,7 +12,8 @@ def placeholder? def run @html.css('a, link').each do |node| @link = create_element(node) - line = @node.line + line = node.line + content = node.to_s next if @link.ignore? @@ -21,17 +22,17 @@ def run # is it even a valid URL? unless @link.valid? - add_issue("#{@link.href} is an invalid URL", line: line) + add_issue("#{@link.href} is an invalid URL", line: line, content: content) next end - check_schemes(@link, line) + check_schemes(@link, line, content) # is there even an href? if missing_href? # HTML5 allows dropping the href: http://git.io/vBX0z next if @html.internal_subset.name == 'html' && @html.internal_subset.external_id.nil? - add_issue('anchor has no href attribute', line: line) + add_issue('anchor has no href attribute', line: line, content: content) next end @@ -39,66 +40,66 @@ def run next if @link.non_http_remote? # does the file even exist? if @link.remote? - add_to_external_urls(@link.href, line) + add_to_external_urls(@link.href) next elsif !@link.internal? && !@link.exists? - add_issue("internally linking to #{@link.href}, which does not exist", line: line) + add_issue("internally linking to #{@link.href}, which does not exist", line: line, content: content) end # does the local directory have a trailing slash? if @link.unslashed_directory? @link.absolute_path - add_issue("internally linking to a directory #{@link.absolute_path} without trailing slash", line: line) + add_issue("internally linking to a directory #{@link.absolute_path} without trailing slash", line: line, content: content) next end # verify the target hash - handle_hash(@link, line) if @link.hash + handle_hash(@link, line, content) if @link.hash end external_urls end - def check_schemes(link, line) + def check_schemes(link, line, content) case link.scheme when 'mailto' - handle_mailto(link, line) + handle_mailto(link, line, content) when 'tel' - handle_tel(link, line) + handle_tel(link, line, content) when 'http' return unless @options[:enforce_https] - add_issue("#{link.href} is not an HTTPS link", line: line) + add_issue("#{link.href} is not an HTTPS link", line: line, content: content) end end - def handle_mailto(link, line) + def handle_mailto(link, line, content) if link.path.empty? - add_issue("#{link.href} contains no email address", line: line) + add_issue("#{link.href} contains no email address", line: line, content: content) elsif !link.path.include?('@') - add_issue("#{link.href} contains an invalid email address", line: line) + add_issue("#{link.href} contains an invalid email address", line: line, content: content) end end - def handle_tel(link, line) - add_issue("#{link.href} contains no phone number", line: line) if link.path.empty? + def handle_tel(link, line, content) + add_issue("#{link.href} contains no phone number", line: line, content: content) if link.path.empty? end - def handle_hash(link, line) + def handle_hash(link, line, content) if link.internal? unless hash_check @html, link.hash - add_issue("linking to internal hash ##{link.hash} that does not exist", line: line) + add_issue("linking to internal hash ##{link.hash} that does not exist", line: line, content: content) end elsif link.external? - external_link_check(link, line) + external_link_check(link, line, content) end end - def external_link_check(link, line) + def external_link_check(link, line, content) if !link.exists? - add_issue("trying to find hash of #{link.href}, but #{link.absolute_path} does not exist", line: line) + add_issue("trying to find hash of #{link.href}, but #{link.absolute_path} does not exist", line: line, content: content) else target_html = create_nokogiri link.absolute_path unless hash_check target_html, link.hash - add_issue("linking to #{link.href}, but #{link.hash} does not exist", line: line) + add_issue("linking to #{link.href}, but #{link.hash} does not exist", line: line, content: content) end end end diff --git a/lib/html-proofer/check/scripts.rb b/lib/html-proofer/check/scripts.rb index 77068549..d0a96c15 100644 --- a/lib/html-proofer/check/scripts.rb +++ b/lib/html-proofer/check/scripts.rb @@ -9,17 +9,18 @@ def run @html.css('script').each do |node| @script = create_element(node) line = node.line + content = node.content next if @script.ignore? next unless node.text.strip.empty? # does the script exist? if missing_src? - add_issue('script is empty and has no src attribute', line: line) + add_issue('script is empty and has no src attribute', line: line, content: content) elsif @script.remote? - add_to_external_urls(@script.src, line) + add_to_external_urls(@script.src) elsif !@script.exists? - add_issue("internal script #{@script.src} does not exist", line: line) + add_issue("internal script #{@script.src} does not exist", line: line, content: content) end end diff --git a/lib/html-proofer/issue.rb b/lib/html-proofer/issue.rb index 20843378..7997abc0 100644 --- a/lib/html-proofer/issue.rb +++ b/lib/html-proofer/issue.rb @@ -1,12 +1,13 @@ module HTMLProofer class Issue - attr_reader :path, :desc, :status, :line + attr_reader :path, :desc, :status, :line, :content - def initialize(path, desc, line: nil, status: -1) + def initialize(path, desc, line: nil, status: -1, content: nil) @line = line.nil? ? '' : " (line #{line})" @path = path @desc = desc @status = status + @content = content end def to_s @@ -52,7 +53,11 @@ def report(sorted_issues, first_report, second_report) if first_report == :status @logger.log :error, " * #{issue}" else - @logger.log :error, " * #{issue.send(second_report)}#{issue.line}" + msg = " * #{issue.send(second_report)}#{issue.line}" + if !issue.content.nil? && !issue.content.empty? + msg = "#{msg}\n #{issue.content}" + end + @logger.log(:error, msg) end end end diff --git a/spec/html-proofer/proofer_spec.rb b/spec/html-proofer/proofer_spec.rb index c4929bfe..f41e4477 100644 --- a/spec/html-proofer/proofer_spec.rb +++ b/spec/html-proofer/proofer_spec.rb @@ -42,6 +42,7 @@ * image gpl.png does not have an alt attribute (line 7) * internal image gpl.png does not exist (line 7) * tel: contains no phone number (line 5) + Tel me - spec/html-proofer/fixtures/sorting/path/single_issue.html * image has a terrible filename (./Screen Shot 2012-08-09 at 7.51.18 AM.png) (line 1) '''.strip)