jumanjihouse / pre-commit-hooks

git pre-commit hooks that work with http://pre-commit.com/
MIT License
114 stars 52 forks source link

Bug: Filenames with spaces are treated as multiple arguments which generate file not found errors #71

Open jamesquilty opened 3 years ago

jamesquilty commented 3 years ago

I tried activating the markdownlint hook for my repo, which runs without complaint with mdl . but with the pre-commit hook mdl crashed on some calls with a cryptic multi-line traceback which appears to be saying "File not found: ."

The problem appears to be that, while pre-commit correctly quotes filenames containing spaces when passing them as arguments to the hook script, the run-mdl Ruby script assumes that there are no spaces in the arguments and joins them all together with no quoting or escaping with args = ARGV.join(' ') and then calls mdl #{args} 2>&1. Cryptic tracebacks ensue.

I don't know Ruby, but the alternatives for fixing this are fairly clear:

  1. Use the shellescape builtin with something like ARGV.shellescape.join(' ') (I'm only speculating that this will function as desired in Ruby), or
  2. Iterate over each element of ARGV, running mdl on each and managing $CHILD_STATUS.exitstatus within the loop.

This is a blocker for me, I can't activate this hook if it can't handle filenames which contain spaces.

Other places where this construct is used may be seen via a search for ARGV.join(' ') within this repo.

Related: puts "args: #{args}" is handled inconsistently within the scripts. I agree with the comment and conditional seen in run-fasterer, run-reek and run-rubocop:

# ... pre-commit looks better if there is no output on success.
...
puts args if ENV['DEBUG']

It would be... better... if argument printing in all these scripts were harmonized to use puts "args: #{args}" if ENV['DEBUG'] (or whatever works, if my speculation about Ruby syntax is incorrect).