Feature Request: Automatic git add

[AquiTCD]

lefthook is great library, thanks!

I would like to use like

1. run a linter and a formatter at pre-commit.
   (like--auto-correct by RuboCop, --fix by ESLint)
2. include the result of changing by auto-fix to staged code.
3. then, commit.

first, I tried like

pre-commit:
  commands:
    backend-linter:
      glob: "*.rb"
      run: bundle exec rubocop --auto-correct --force-exclusion {staged_files}  && git add {staged_files}

but it does not work like I expected.
should be alright if the staged range is whole file,
but in the case of staged as hunk(like git add -p ...), re-add whole file.

Does any one know a solution?

---

I used use lint-staged and husky like that.

{
  "lint-staged": {
    "*.rb": [
      "bundle exec rubocop --auto-correct --force-exclusion",
      "git add"
     ]
  }
}

in this case include the result of changing by auto-correct to a commit.

---

I fixed typo force-exclution = > force-exclusion

[Arkweid]

Thank you @AquiTCD for question. Have you tried this one?

  run: bundle exec rubocop --auto-correct --force-exclution {staged_files}  && git add

Just git add without {staged_files}

[AquiTCD]

Thank you for the answer.
I tried it, but it said

/Users/aqui/.ghq/github.com/AquiTCD/ruby-sandbox/vendor/bundle/ruby/2.6.0/gems/lefthook-0.6.3/libexec/lefthook-mac run pre-commit
Lefthook v0.6.3
RUNNING HOOKS GROUP: pre-commit

  EXECUTE > backend-linter
Inspecting 1 file
W

Offenses:

src/test.rb:8:11: W: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/fortissimo1997/ruby-style-guide/blob/japanese/README.ja.md#consistent-string-literals)
    two = 'bbb'
          ^^^^^

1 file inspected, 1 offense detected, 1 offense corrected
Nothing specified, nothing added.
Maybe you wanted to say 'git add .'?

SUMMARY: (done in 1.51 seconds)
✔️  backend-linter
/Users/aqui/.ghq/github.com/AquiTCD/ruby-sandbox/vendor/bundle/ruby/2.6.0/gems/lefthook-0.6.3/libexec/lefthook-mac install
SYNCING lefthook.yml
SERVED HOOKS: pre-commit, prepare-commit-msg
/Users/aqui/.ghq/github.com/AquiTCD/ruby-sandbox/vendor/bundle/ruby/2.6.0/gems/lefthook-0.6.3/libexec/lefthook-mac run prepare-commit-msg .git/COMMIT_EDITMSG message
[master 51662ee] test
 1 file changed, 7 insertions(+), 1 deletion(-)

---

just information, lint-staged struggled this kind of issue before.
lint-staged/lint-staged#62

IMO
I think the concept of lefthook is not exactly same as lint-saged,
that's why perhaps lefthook should not concern about fixed code by pre-commit.

[Arkweid]

Thank. The concept of a lefthook is different, and we don’t want to be lint-staged on golang.
But I'll try to figure it out.

[EvHaus]

For me this is the one feature preventing me from migrating from husky+lint-staged to lefthook. I would love to see official/formal support for git add added as part of the configuration. Maybe something like this:

pre-commit:
  commands:
    eslint:
      add: true
      glob: "*.js"
      run: eslint --fix {staged_files}

Where add:true would add any files modified as part of the command into the commit itself.

[sondr3]

Wanted to chime in with this too, I was really surprised when the job formatted the files but didn't add them to the commit, which was something I expected from a tool like this. Right now this makes it fairly useless when you only want to run something that formats/fixes small errors. I hope you'll consider something like this as it'll make this tool perfect. Thanks for lefthook regardless 😄

[Arkweid]

Oke, let's give it a try. It would be great if someone will take it. Here's how the task can be achived:

1. Add new config key to the list
   https://github.com/Arkweid/lefthook/blob/master/cmd/run.go#L42
2. Grab it from config file like this:
   https://github.com/Arkweid/lefthook/blob/master/cmd/run.go#L469-L472
3. Add new behavior at the end of executing command:
   https://github.com/Arkweid/lefthook/blob/master/cmd/run.go#L248-L249
   Just something simple like that: ExecGitCommand("git add files")
   https://github.com/Arkweid/lefthook/blob/master/context/context.go#L24-L25
4. Double new code for _windows.go postfix files.

[lionskape]

I can try to suggest a quick solution, for your issue:

1. as a first command you can stash unstaged changes for all files, which have in "staged_files" (via git stash push -k -- [...] syntax), or for the whole project (via git stash -k -u)
2. after the execution you can do one of two things:
   2.1) automatically pop your code from stash (on post-commit, not forget to check if any stashed was created, for example by adding a commit hash to the stash name). But you can get a lot of fails and problems.
   2.2) keep the stash, and print a message to user, that you stashed something and it is not resolved now. Not forget to create a command text for the user (useful for users who wants to copy-paste).

[lucatrv]

AFAIK stashing is used by many git-hooks tools, see for instance lint-staged and pre-commit.

For a step-by-step description see the autokooks instructions for linting plugins and formatting plugins.

Other useful documentaiton:
https://danilodellaquila.com/en/blog/use-git-stash-in-your-pre-commit-hook
https://www.darrenlester.com/blog/git-pre-commit-stash
https://medium.com/@MadsSejersen/git-precommit-hooks-done-right-886cfdf4ffb0

Stashing involves the risk of manual intervention in case the hook fails badly, see for instance the following two pre-commit issues, that have been fixed:
pre-commit/pre-commit#176
pre-commit/pre-commit#397

Maybe precise-commits does something more / different, but I do not have precise information...

[lucatrv]

To make it clear, I wrote the information above just trying to help on this. As for myself, I run hooks only to check staged files, as I prefer to fix them manually in case the checks do not pass. So personally I would not even be against the decision to disallow automatic fixing, if it could be dangerous.

[barinbritva]

Is there any updates so far?

[O4epegb]

Maybe there should be info about this behaviour in this guide https://github.com/evilmartians/lefthook/wiki/Migration-from-husky-with-lint-staged ?

Because I think husky+lint-staged users expect that changed files will be re-added automatically.

[trajano]

I just did the reverse because of this and one other thing I found missing on lefthook.

1. The move to husky V8 from lefthook was quite tricky since they put everything in the .husky/ folder as a script (which means I have to ensure proper line endings on Windows) rather than relying on having things configured in package.json.
2. Since I am on monorepo with different types of components and styles having a lint-staged on a per project basis was something I was looking for. lefthook.yml seems to only go on the root and does not have a per package version. The translation to left hook wasn't too bad, but as noted you can only have staged files (which is what I was looking for).

[pvds]

Let's start with stating:

I much prefer Lefthook over any other alternative currently available.

Like many others I switched from Husky + Lint-staged, in my case the reasons were:

• primarily because developers had performance issues (3+ minutes waiting in some cases)
• reduce dependencies
• great dx using lefthook.yml

Even though Lefthook might not have intended to solve the "automatic git add" use case described in this issue,
it seems to be the main reason people are forced away from using it.

Currently my only viable options are:

1. Not change any files in the pre-commit hook using eg. npx stylelint {staged_files}
2. Return an exit code, forcing user to commit twice >
   eg.: npx stylelint {staged_files} || (npx stylelint --fix {staged_files} && exit 1)
3. Revert back to husky+lint-staged
4. Find another alternative (lot of subpar ones out there that eg need ruby or python to be installed or are very slow)

All of these options result in a suboptimal developer experience.

---

I think what the majority of people using Lefthook to do pre-commit checks want is:

Fix what can be fixed automatically without disrupting a developers workflow

---

Proper solution

As @mrexox mentioned here on PR #109 just using git add {staged_files} is dangerous since committing partially staged files would add the whole file.

The fix would have to be similar to how lint-staged fixed this issue in this PR: lint-staged/lint-staged#75.

IMHO the acceptance criteria would be:

• add a addChangedDuringCommand property (being over specific here on purpose, could be eg. addChanges/autoAdd)
• this property is only applicable to the pre-commit hook
• the logic would have to deal with partially staged files

[mrexox]

Hey @pvds! With 1.3.0 version I've added handling for partially staged files. Now for pre-commit hook unstaged changes are being hidden, and commands run on staged changes only. Then after the hook processing the unstaged changes are applied again.

This is not super stable although I haven't noticed any issues related to that. My next step is to decide:

• Enable auto git add by default as many people might want
• Add a configuration option like auto_add (or any other meaningful name) for this

I'm still thinking but I guess that I'm going to implement this feature in 1-2 months (maybe earlier, if there is a request for it).

I'll be glad to know what you think about these options. To sum up, my questions:

• Should it be an option auto_add or a default behavior (for pre-commit hook only, of course)?
• If it is an option, should it be true or false by default? false will be more backward-compliant.
• What naming do you like?

Namings:

• auto_add: true
• stage_changed: true
• restage: true

I think the last is the most intriguing question :)

[pvds]

@mrexox let's start with the most intriguing question!
I'm also struggling but I do know I would like the name to be a bit more descriptive.

From your suggestions I think stage_changed is describing best what happens.
There is the slight possibility someone might be confused and think "but I don't want to stage all my changes" .

My mind started wondering towards including the word hook, for example:

• add_hook_changes
• include_hook_changes
• stage_hook_changes

Then again this would make sense in case it's a property of the pre-commit hook, but may be less intuitive as a property of commands within the pre-commit hook. I guess you intended to implement it as the latter?

The changes are made by the run command so stage_run_changes could also be an option.

---

For now my answers to your questions would be:

1. It should be an option; leaving room for cases like using an exit code
2. The default value should be true, people (including myself) expect this to be default behaviour.
   Strictly making it a breaking change
3. Best names for the option as a property of commands within the pre-commit hook: stage_changed or
   stage_run_changes

NB. Setting the default value to true would require the feature to be super stable.
Could you explain why handling partially staged file is "not super stable"?

Would love to hear your considerations.
I will get some more opinions on the matter!

[trajano]

• Enable auto git add by default as many people might want

I don't recommend changing the default otherwise you may get into the same situation as svg/svgo#1128 and lock this conversation because it may become heated.

[mrexox]

Thank you for participating in this discussion. I guess maybe stage_fixed might also be a good name because most linters have a --fix or --autocorrect option and fixed makes more sense than changed. @pvds, what do you think?

An example:

pre-commit:
  commands:
    lint:
      run: npm run lint --fix
      stage_fixed: true

I'd like to leave a room for a default behavior but @trajano you are right, it makes sense to leave defaults untouched and then reach out to lefthook users about whether they want to change the default behavior. Also a major version bumping will make sense in this case.

[pvds]

I don't recommend changing the default otherwise you may get into the same situation as svg/svgo#1128 and lock this conversation because it may become heated.

I agree that we should be careful when changing defaults and take into account the potential impact on the community. However, I also believe it's important to hear different perspectives and ensure that everyone's opinions are valued. While the situation you've referenced is worth considering, it's important to remember that each situation is unique and should be evaluated on its own merits.

---

@mrexox @trajano I agree that we should not make this default behaviour without providing a configuration option to change it. Without more community feedback I agree the default value of this option should not be true.

Considering the name @mrexox I think your stage_fixed suggestion is great!

I dismissed using 'fixed' while brainstorming because I was thinking that changes made during the pre-commit hook might not always be due to an option called fixed but using stage_fixed actually describes the intent very well!

[mrexox]

Just released 1.3.5 version with this feature included. Please, test it and feel free to open a new issue if you face any problem 🙏

New option docs: stage_fixed