Add system hook

[taoufik07]

I hope you don't mind opening the PR without first opening an issue

[aaronmondal]

@koalaman @taoufik07 Would be nice to have something like this merged. I'm not sure whether the $VERSION in the description works in this case though. As long as I pass a valid git tag to rev, it just uses whatever version spellcheck system binary is currently available. It will print the potentially wrong, misleading version when running the hook.

[tfurf]

I'm also interested in this feature --- docker isn't always available for various reasons and being able to route around that requirement would be helpful.

[b8rky]

the "template" is updated but the .pre-commit-hooks.yaml still needs to be updated. e.g. master...b8rky:shellcheck-precommit:add-system-hook

in the mean time, I'm using my fork:

https://github.com/blueledger/toolbox/blob/90082f3cc7ad1ed32c1643008f8c6823d5259f14/.pre-commit-config.yaml#L15-L19

[aaronmondal]

FYI we found that using pre-commit hooks via Nix is more robust than the default .pre-commit-hooks.yaml. Easy to integrate into CI and reproducible across dev environments. We are using many different hooks and kept running into issues where some systems couldn't build some hooks properly. With Nix you can just use binaries directly as hooks and things "just work". Since shellcheck is already integrated into devenv you don't even need to set up the path to the shellcheck executable manually:

https://github.com/eomii/rules_ll/blob/75825f027b36fe411508e16c1b39368a715faa21/pre-commit-hooks.nix#L68-L73

https://github.com/cachix/devenv

[atsaloli]

@koalaman Do you have any concerns about this pull request? This would be extremely useful -- I have an older system and don't have Docker as I need to keep things lightweight.

[atsaloli]

ChatGPT wrote this precommit config (.pre-commit-config.yaml) for me that uses local shellcheck, no Docker required:

repos:
  -   repo: local
      hooks:
      -   id: shellcheck
          name: shellcheck
          entry: shellcheck
          language: system
          files: \.sh$

It works! Plus I can see colors (which disappear when I run shellcheck through Docker).

[Rivierg]

ChatGPT wrote this precommit config (.pre-commit-config.yaml) for me that uses local shellcheck, no Docker required:

Thanks @atsaloli , that was helpful! To cover more shell scripts that might not have the extension .sh I would recommend filtering with types using types: [shell] instead of files: \.sh$ as pre-commit will try to detect all supported shell scripts by looking at the shebang #! in each file.

Note that some shells like csh won't be checked:

ShellCheck only supports sh/bash/dash/ksh scripts.

@koalaman this MR has been pending for almost a year, it would be a helpful feature to merge if you could please look into it ? Thanks

[Freed-Wu]

pre-commit have supported haskell hooks. https://pre-commit.com/#haskell

I think a haskell hook will be better?

Related issue: koalaman/shellcheck#2909

[Freed-Wu]

I usually use the following system hook of shellcheck:

  - repo: https://github.com/jumanjihouse/pre-commit-hooks
    rev: 3.0.0
    hooks:
      - id: shellcheck
        exclude_types:
          - zsh

Related PR: koalaman/shellcheck#2925

[koalaman]

Hey, am I understanding it right that this just invokes whichever version of ShellCheck the user already has installed, if any?

[Freed-Wu]

Hey, am I understanding it right that this just invokes whichever version of ShellCheck the user already has installed, if any?

Yes, if user don't installed shellcheck, it will failed.

[koalaman]

I see. If it ignores the specified shellcheck version and just runs whatever the user has, then my understanding is that it would be better off as a repo: local system hook