evilmartians / lefthook

Fast and powerful Git hooks manager for any type of projects.
MIT License
4.75k stars 211 forks source link

Pass `--all-files` flag to the command being run during the hook #702

Closed kormanowsky closed 5 months ago

kormanowsky commented 5 months ago

:zap: Summary

In #120, --all-files flag was introduced. However, when some hook executes a command, the latter has no way to determine if the flag was passed or not because lefthook does not preserve it / does not do anything to tell the command about this flag.

Value

Some programs are designed to support only-changed mode, when the program is applied only to the files which have been changed. Calling such a program from lefthook's run invocation without any sign of --all-files being passed (that is, all the files being processed by the program, not only changed ones) leads to a) extra work in the program, b) conflict between the lefthook running with all files and the program running with only changed ones.

Passing --all-files or any equivalent of the flag would help some programs to save some time and not get into conflict with lefthook. That would eliminate the ambiguity and make results coincide with the expectations.

Behavior and configuration changes

Lefthook should pass --all-files or an equivalent to the program being called, if the mentioned flag is passed to lefthook itself.

mrexox commented 5 months ago

Hey @kormanowsky! I think I got your idea, but let me explain the purpose of the --all-files flag.

Lefthook is supposed to be used implicitly by git hooks. When called implicitly lefthook never receives --all-files flag. This flag is useful only for testing purpose.

For example, you write the following pre-commit hook:

pre-commit:
  commands:
    lint:
      run: yarn lint --fix {staged_files}
      stage_fixed: true

When you try to test it you will see that the hook is skipped (since no files are staged). But if you all it with the flag like lefthook run pre-commit --all-files the {staged_files} template will be replaced with all the files of you Git repo.

So, in case you know that your command can work agains a list of files it should work properly with the list of all project files too. So, I wonder in what case you need to tell an external command that --all-files flag was provided? Please describe your situation and I will try to help.

kormanowsky commented 5 months ago

Hi @mrexox!

Forgot to say that there may be a script (e. g. a linter) that checks only changed lines inside only changed files. It uses information from git about changes and tries to find those changed lines. But it is sometimes useful to do a check for all the lines in all files, regardless of changes.

This is where --all-files (and some sign of it being passed) becomes helpful. Without the flag / an equivalent, there will be the conflict I described above because custom program will check only changed lines in all files, but we expect it to check them all.

And it seems that there is no way to avoid this conflict rather than creating 2 different commands and putting {push_files} to the first and {all_files} to the second one. This leads to copy-pasting commands and thus is not convenient.

mrexox commented 5 months ago

It looks like you have two cases:

My vision is that your case is very special. Lefthook does the replacement of templates but can't pass something else to a command. Instead I'd suggest you to use a different local-only hook, like this:

lint-all:
  commands:
    run: yarn run lint --all-files

If you want to reduce duplication you could use YAML anchors:

pre-push:
  commands: &commands
    ...
    lint:
      run: yarn lint {staged_files}

lint-all:
  commands: 
    <<: *commands
    lint:
      run: yarn lint --all-files

In this example you only overwrite the commands you want to run differently. And since you run the hook manually you don't have to stick with pre-push. You can name the hook whatever you want.

Does this look helpful?

kormanowsky commented 5 months ago

I think this is what I need for my case. @mrexox, thanks for your help!