Also glob commands without templates & files

[sanmai-NL]

Closes # (issue)

⚡ Summary

Commands that neither had templates in run nor files didn't apply file globs, even if the--all-files or all-including-untracked-files CLI param was set. Applying globs is useful to limit commands that run on a fixed path (e.g., src/) and don't use templates, but should only run if relevant paths have changed.

☑️ Checklist

Please merge #594 first.

• Check locally
• Add tests
• Add documentation: should not be needed, since this behavior is in line with the current docs. In that sense, this change fixes a defect.

[mrexox]

Could you please provide an example usage with the hook? It feels like instead of replacing {staged_files} with all files we need to replace {files} or all other templates. But I'm not sure until I understand the use case.

[sanmai-NL]

post-build:
  commands:
    clippy:
      files: git diff --name-only --staged # remove this line
      glob: '*.rs'
      run: cargo clippy --all-targets --future-incompat-report -- -D warnings 2> /dev/null

Without the files key, and with e.g. lefthook run post-build, clippy is being run even if no .rs file is staged. No file list is being built up nor is globbing applied.

$   git ls-files --cached --exclude-standard | grep -E '.rs$' | wc -l
       3
$   git diff --name-only --cached | grep -E '.rs$' | wc -l
       0
$ lefthook run -vvvv post-build
  lefthook run -vvvv post-build
│ [lefthook] cmd: [git rev-parse --show-toplevel]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: /Users/dummy/dummy
                                                                                      
│ [lefthook] cmd: [git rev-parse --git-path hooks]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: .git/hooks
                          
│ [lefthook] cmd: [git rev-parse --git-path info]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: .git/info
                         
│ [lefthook] cmd: [git rev-parse --git-dir]
│ [lefthook] dir: 
│ [lefthook] err: <nil>
│ [lefthook] out: .git
                    
╭──────────────────────────────────────╮
│ 🥊 lefthook v1.5.5  hook: post-build │
╰──────────────────────────────────────╯
┃  clippy ❯ 


                                      
  ────────────────────────────────────
summary: (done in 0.58 seconds)       
🥊  clippy

I have done more tests but it's a bit of hassle to replicate and set up without risking my work.

[mrexox]

@sanmai-NL, this PR adds too specific changes to the lefthook processing. I am pretty sure that 99% of lefthook users don't need handling for the all files including untracked. But if I merge it I will have to support this feature which I feel can be implementing via a small hack to configuration.

Implicit filtering was added only for pre-commit and pre-push hooks and it caused issues to some users, so I'm afraid of adding implicit handling to other hooks, even custom. I want to keep the balance between convenience and obvious behavior of lefthook.

I can suggest you a workaround many users (including me) were using for the purpose you described:

# lefthook.yml

pre-build:
  commands:
    clippy:
      files: git diff --name-only --staged # remove this line
      glob: '*.rs'
      run: cargo clippy --all-targets --future-incompat-report -- -D warnings 2> /dev/null || echo {files} >/dev/null

This echo {files} >/dev/null command adds {files} template to run command, so all filters will be applied, including globs. If the result is empty the hook will be skipped. If the result is not empty, this extra echo won't affect anything.

Thank you for providing this change. I learned a new use case of lefthook. I hope I could suggest a useful workaround. I'm sorry that I can't accept this change to the codebase.

[sanmai-NL]

This workaround is not cross-OS compatible.

The main selling point of Lefthook is not Git hooks. I can create those with a custom script. But having a Git hook collection that works cross-OS without having write and understand complex source code if team members want to learn what's happening.

[mrexox]

@sanmai-NL , if you keep files option lefthook will apply the required filters. I think it is pretty OS-compatible compromise. Why do you need to remove the files option?