Skip to content

More portable solution for hooks #227

@pfmoore

Description

@pfmoore

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:

  1. Don't use the shell=True flag, even on Windows.
  2. Recommend using Python scripts for portable templates
  3. 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.
  4. 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...

Metadata

Metadata

Assignees

No one assigned

    Labels

    discussionenhancementThis issue/PR relates to a feature request.windowsThis issue/PR related to windows systems only

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions