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

How to set CLI option --check-external-hash to false? #735

Closed
remarkablemark opened this issue Jul 23, 2022 · 6 comments · Fixed by #738
Closed

How to set CLI option --check-external-hash to false? #735

remarkablemark opened this issue Jul 23, 2022 · 6 comments · Fixed by #738

Comments

@remarkablemark
Copy link

When I run the command:

htmlproofer --check-external-hash false _site

I get the error:

htmlproofer 4.2.0 | Error:  false does not exist
Traceback (most recent call last):
        9: from /Users/remarkablemark/.rbenv/versions/2.7.3/bin/htmlproofer:23:in `<main>'
        8: from /Users/remarkablemark/.rbenv/versions/2.7.3/bin/htmlproofer:23:in `load'
        7: from /Users/remarkablemark/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/html-proofer-4.2.0/bin/htmlproofer:11:in `<top (required)>'
        6: from /Users/remarkablemark/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/mercenary-0.3.6/lib/mercenary.rb:19:in `program'
        5: from /Users/remarkablemark/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/mercenary-0.3.6/lib/mercenary/program.rb:42:in `go'
        4: from /Users/remarkablemark/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/mercenary-0.3.6/lib/mercenary/command.rb:220:in `execute'
        3: from /Users/remarkablemark/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/mercenary-0.3.6/lib/mercenary/command.rb:220:in `each'
        2: from /Users/remarkablemark/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/mercenary-0.3.6/lib/mercenary/command.rb:220:in `block in execute'
        1: from /Users/remarkablemark/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/html-proofer-4.2.0/bin/htmlproofer:91:in `block (2 levels) in <top (required)>'
/Users/remarkablemark/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/html-proofer-4.2.0/lib/html_proofer.rb:25:in `check_file': false does not exist (ArgumentError)

This is from htmlproofer --help:

--check-external-hash  Checks whether external hashes exist (even if the webpage exists) (default: `true`).
@riccardoporreca
Copy link
Collaborator

@remarkablemark, I had the same issue at some point, but the help in fact also hints to the fact that options should follow the path

Usage:

  htmlproofer PATH [options]

The following would do

htmlproofer _site --check-external-hash false

@remarkablemark
Copy link
Author

remarkablemark commented Jul 23, 2022

Thanks for the response @riccardoporreca! I tried your suggestion, but the build is still failing for me. See remarkablemark/remarkablemark.github.io#42:

Run bundle exec htmlproofer _site --check-external-hash false --ignore-status-codes 0,301,400,401,403,429,999
Running 3 checks (Images, Scripts, Links) in ["_site"] on *.html files...


Ran on 382 files!


For the Links > External check, the following failures were found:

* At _site/blog/201[6](https://github.com/remarkablemark/remarkablemark.github.io/runs/7481977717?check_suite_focus=true#step:5:7)/06/15/firefox-binary-webdriverjs/index.html:13[8](https://github.com/remarkablemark/remarkablemark.github.io/runs/7481977717?check_suite_focus=true#step:5:9):

  External link https://seleniumhq.github.io/selenium/docs/api/javascript/module/selenium-webdriver/firefox_exports_Options.html#setBinary failed: https://seleniumhq.github.io/selenium/docs/api/javascript/module/selenium-webdriver/firefox_exports_Options.html exists, but the hash 'setBinary' does not (status code 200)

...


HTML-Proofer found 75 failures!
Error: Process completed with exit code 1.

@riccardoporreca
Copy link
Collaborator

@remarkablemark, indeed, the way such boolean command-line options are defined in

p.option 'check_external_hash', '--check-external-hash', 'Checks whether external hashes exist (even if the webpage exists) (default: `true`).'
p.option 'check_internal_hash', '--check-internal-hash', 'Checks whether internal hashes exist (even if the webpage exists) (default: `true`).'

is meant for not specifying true/false at the command line, but always results in true if specified. This makes sense for opt-in options defaulting to false, but does not if the default is true (opt-out). So at the moment it is not possible to disable checking internal/external hashes from the command-line.

@gjtorikian, the natural approach to such cases (there are in fact others beyond hash checking) is to use enable the --no prefix as follows

  p.option 'check_external_hash', '--[no-]check-external-hash', 'Checks whether external hashes exist (even if the webpage exists) (default: `true`).'
  p.option 'check_internal_hash', '--[no-]check-internal-hash', 'Checks whether internal hashes exist (even if the webpage exists) (default: `true`).'

Passing true or false explicitly seems possible as follows, although it is not very natural given that for the opt-in options no explicit true is required:

  p.option 'check_external_hash', '--check-external-hash <false>', TrueClass, 'Checks whether external hashes exist (even if the webpage exists) (default: `true`).'
  p.option 'check_internal_hash', '--check-internal-hash <false>', TrueClass, 'Checks whether internal hashes exist (even if the webpage exists) (default: `true`).'

Happy to help out with a PR (including tests) for wither of the two approaches.

@remarkablemark
Copy link
Author

Thanks for the update @riccardoporreca. Would it be easier to default --check-external-hash and --check-internal-hash to false since then you can pass it as a CLI arg if you want to opt-in? Or would that constitute as a breaking change?

@gjtorikian
Copy link
Owner

The CLI parser used here has often been a problem. The correct usage here is supposed to be --allow-hash-href=false. The next patch release of html-proofer should fix it. (I am releasing one after I have #313 solved)

@remarkablemark
Copy link
Author

Thank you @gjtorikian!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants