search

tarmolov / git-hooks-js

A tool to manage and run project git hooks
167 stars 30 forks source link

Hooks should not be ignored by .gitignore #35

Closed rjmunro closed 7 years ago

rjmunro commented 7 years ago

(moved from discussion in https://github.com/tarmolov/git-hooks-js/issues/30)

Currently this project has code to check if a hook file is included in .gitignore and not run it if that is the case. This was inspired by a problem where a vim .[filename].swp file was executed while developing a git hook: https://github.com/tarmolov/git-hooks-js/issues/12

This looses the ability to have local git hooks that you might want to deliberately ignore because you don't want to commit them, but you do want to run them on all your commits, and you do want to use the git-hooks-js folder structure because you have other hooks that you do want committed.

The vim swap file is not executable, and starts with a . character. Either of these reasons should be enough to not run the file. Perhaps a warning could be displayed on such files, something like "not running hook named .foobar.swp as it starts with a ." and / or "not running hook named .foobar.swp as it is not executable".

tarmolov commented 7 years ago

It seems that you want to run user hooks which put in $HOME/.githooks directory. Such feature is supported by the original git-hooks but I've dropped this feature in git-hooks-js because nobody used it.

Do you really have a such case? What kind of hooks do you need to run but don't want to commit?

codebling commented 7 years ago

I agree with warning. Currently, there is a check against executable files, although this does not apply to Windows. There is no check for files starting with a . (known as hidden files on *ix environments)

This being said, the original use case (#12) doesn't convince me that there's a need for this feature, as one can set the vim save location to a different directory (e.g. :set directory=/tmp) or edit in a different directory and cp the file to the hooks folder when done editing. Furthermore, forces an even more difficult workflow (having to git reset to unstage your hook before every commit to the repo vs editing in different directory and cp to githooks) upon people who want .gitignored hooks to execute (the point @rjmunro makes above)

codebling commented 7 years ago

@tarmolov he does not want to run hooks from $HOME/.githooks, he wants to run hooks that are ignored (.gitignore)

tarmolov commented 7 years ago

In my point of view, git-hooks is an extension for git. It builds in .git/hooks and embraces all git rules as well.

It means that git-hooks tries to use native git mechanics such as .gitignore file to be closer to the git philosophy.

If you have a specific case in your work which doesn't solve by git-hook, let me know and we'll figure out how to solve it in the best way. But I don't want to solve all hypothetic cases, only real ones.

rjmunro commented 7 years ago

No, not $HOME/.githooks - I'm talking about the feature that checks if a hook is matched by .gitignore rules and doesn't run it if it is. I think this is a misfeature. The original problem (#12) would be covered by ignoring files starting .. Adding this feature has the collateral damage of meaning that you can't have a project specific hook that you don't want to commit.

I'm requesting a feature that is currently broken & unnecessary be removed, not that a new feature be added.

I used to work on a project where I had to write dietitian a lot, but I kept writing dietician. We had pre-commit hooks for checking code style etc. that were shared with all devs (managed by some scripts that we wrote), and I added my own one to grep for dietician and prevent me committing. It would be embarrassing to commit that to the project - it's purely an artefact of my bad spelling. Under the current scheme I can't gitignore my own script, I have to commit it, or remember never to use git stash -u, git add -A, git add . etc. and carefully add other changed files one at a time.

I don't need this feature now, but I can imagine being confused by it if I stumble across it by mistake.

tarmolov commented 7 years ago

I got your point. Thank you. I'll think how to solve it properly.

codebling commented 7 years ago

Any update ?

tarmolov commented 7 years ago

Nope but I'm going to fix it in the nearest future. Is it urgent for you?

codebling commented 7 years ago

No, just wondering, as our discussion in other PR is affected by the code that controls this behaviour.