jish / pre-commit

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

The binary check handles unicode source files. #240

Closed jish closed 8 years ago

jish commented 8 years ago

The binary? query makes a best guess as to whether a file is a binary file or not. The previous check was reading some bytes form the beginning of the file. It would then check to see what percentage of those bytes were common ASCII characters. If the file was made up of 70% common ASCII characters, we would "guess" that the file was not a binary file.

One of the failings of this test is that it does not take unicode characters into account. To fix that, we attempt to convert all unicode characters to the ASCII character ? and then perform the same calculation to see if 70% of the file contains common characters.

cc: @kyoshidajp @mpapis cf: https://github.com/jish/pre-commit/issues/238

jish commented 8 years ago

Ugh, dependencies have their own locked down dependencies...

https://travis-ci.org/jish/pre-commit/jobs/142644412

Gem::InstallError: ruby_dep requires Ruby version >= 2.0.0, ~> 2.0.

https://travis-ci.org/jish/pre-commit/jobs/142644415

Gem::InstallError: listen requires Ruby version >= 2.2.3, ~> 2.2.
mpapis commented 8 years ago

for the dependencies you can track the ruby version requirement change in the gems and then add as conditions in Gemfile - then bundler / rubygems should be able to handle it:

if RUBY_VERSION <= "2.0.0"
  gem "listen", "..."
end

I remember having few of those on some gems

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.01%) to 96.256% when pulling 547dc2460d52099ce9b1c9fd476208beb88b1278 on binary_check_handles_unicode into 860ce02ce2a1c3523e14297682a06f1402292c18 on master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.01%) to 96.256% when pulling 86f6806da92eac4728a2825ebfc5c92279e725d5 on binary_check_handles_unicode into 860ce02ce2a1c3523e14297682a06f1402292c18 on master.

mpapis commented 8 years ago

LGTM - please move jruby and merge :)

kyoshidajp commented 8 years ago

@jish Thanks. I did some spot checks by this branch, then that files seems to be fine.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.01%) to 96.256% when pulling 2771e891e25d6603d9bd60a477651a4dd087503a on binary_check_handles_unicode into 860ce02ce2a1c3523e14297682a06f1402292c18 on master.