-
-
Notifications
You must be signed in to change notification settings - Fork 23
fix: improve shebang detection #74
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
Conversation
in the following ways: 1. Handle whitespace in between the "!#" and the executor/interpreter and between the executor/interpreter and other args, 2. Handle the case where the first line of a script contains just "#!", and 3. Handle multiple arguments (e.g., correctly detect that `/usr/bin/env -S bash` is a shell shebang).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thank you for tackling that. I'm running ./test.sh
locally and will merge the PR if it passes.
@@ -38,8 +38,6 @@ def shellscript?(filename) | |||
(shellscript_extension?(filename) && shellscript_syntax?(filename)) | |||
end | |||
|
|||
private |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: #shellscript_syntax?
could have been left as private since we don't call/test it directly but that doesn't really matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops; happy to fix this.
end | ||
|
||
it "returns true for shell interpreters" do | ||
aggregate_failures "valid shebang handling" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL about aggregate_failures
, nice :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see an application for aggregate_failures
in the runner spec as well; PR incoming :)
Just published Bashcov 3.0.3 with this fix, thank you! |
I'm geting bashcov-3.0.3/lib/bashcov/detective.rb:61:in `scan': invalid byte sequence in UTF-8 (ArgumentError)
return false if scanner.scan(/#!\s*/).nil? because it is trying to read a |
@gszep -- oh dear :(. Nevermind; running the Bashcov test suite triggers this, too. 👀 |
in the following ways:
/usr/bin/env -S bash
is a shell shebang).Inspired by #73 , expanded to account for other shebang shenanigans. Also, tests.