-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Description
Following on from the discussion in #215 around cygwin, the hooks system runs hooks via the shell on Windows. This results in a dialog box if the hook uses an extension that isn't registered (such as .sh as used in the tests, which needs cygwin). Also, using shell=True opens up the opportunity for all sorts of weirdness (a hook with an extension of .csv would fire up Excel, for instance).
In any case, shell scripts aren't going to be a portable solution for hooks - not all Windows users have cygwin, certainly.
I suggest:
- Don't use the
shell=Trueflag, even on Windows. - Recommend using Python scripts for portable templates
- Note that platform-specific batch files (shell scripts on Unix, bat files on Windows) can be used in templates not intended for cross-platform use.
- Rewrite the relevant tests to use Python scripts for hooks.
If this is seen as a reasonable solution, I can submit a PR implementing this.
This does lose the ability for people with Cygwin to use hooks written as shell scripts. I don't know how critical that is.
Longer term, something more flexible might be worth adding but I can't immediately think of any way of being sure that anything other than a Python script will be portable, so it'd always be somewhat of a "personal use only" feature...