search

msiebuhr / slint

Lint-tool for web projects
BSD 3-Clause "New" or "Revised" License
7 stars 1 forks source link

--git missing some cases #24

Open Munter opened 10 years ago

Munter commented 10 years ago

When I don't stage changes before I commit them, slint --git doesn't pick up the changes that are about to be added to the commit when linting.

This lets me bypass any pre-commit hook set up with slint --git

Munter commented 10 years ago

I realize slint might not know the context, so this might not be entirely possible. Maybe a recommendation for a git pre-commit hook that ensures files are linted correctly before committing?

papandreou commented 10 years ago

Hmm, weird. What are the exact commands that you run in order to commit without staging?

Munter commented 10 years ago 
echo "\n\n" >> app.js; git commit -am "Break linting by adding to many trailing newlines"
echo "\n\n" >> app.js; git commit -m "Break linting by adding to many trailing newlines" app.js
echo "\n\n" >> app.js; git commit -m "Break linting by adding to many trailing newlines" .
Munter commented 10 years ago

I suspect that list of files might be available in the pre-commit hook scope. So maybe that's where it should be detected and passed on

papandreou commented 10 years ago

Found some bugs that made the slint binary never emit a non-zero exit code, fixed on master. However, I can still reproduce the problem you're describing. It's very surprising to me that those files aren't added to the index before the pre-commit hook is executed. I haven't seen anyone mention that quirk anywhere. Could be an issue with nodegit as well, though.

papandreou commented 10 years ago

Hmm, if I run git diff --cached in my pre-commit hook, it does output the changes, so clearly git commit -a ... doesn't bypass the staging area. That's a relief.

It means that the bug lies somewhere within GitFakeFs, FsPlusGit or nodegit. Sigh.