jish / pre-commit

A slightly improved pre-commit hook for git
https://jish.github.io/pre-commit/
Other
796 stars 96 forks source link

Checks fail to start due to judged a binary file #238

Closed kyoshidajp closed 8 years ago

kyoshidajp commented 8 years ago

Hi.

I want to check my Ruby file by rubocop. But check didn't work when I commit it.

That file body is the following.

# コメント
def sample
  # comment
  # comment
  # comment
end

However next file is worked well.

# コメント
def sample
  # comment
  # comment
end

I guess the reason why is because binary? method of PreCommit::Utils::StagedFiles module is "best guess".

So, I suggest that add not-binary list file and binary? method return false when argument file is found in that list file. The list file support specified pattern, like a .gitignore.

Or support check include binary file option, for example a environment variable.

mpapis commented 8 years ago

we could extend the binary check to exclude unicode letters too ... do you want to take a shoot at it?

kyoshidajp commented 8 years ago

I'm afraid if excluded unicode letters, Rubocop::Cop::Style::AsciiComments wouldn't work.

kyoshidajp commented 8 years ago

I was wrong. Exclude unicode letters only binary?, then it is good for me.

jish commented 8 years ago

Unfortunately, the logic we are using to determine if a file is binary or not, is basically just a guess =/

I am certainly open to expanding the binary check to include unicode letters as well. Or maybe we can find a better way to determine if a file is binary or not.

kyoshidajp commented 8 years ago

How about using .binary? method in mime-types.

This gem require target files have filename extension. And still not completely correct. For example, file name is sample.rb even though PNG file.

However this behavior enough for my case.

jish commented 8 years ago

Resolved in https://github.com/jish/pre-commit/pull/240