Is there a way to change (prettify) partially staged files in pre-commit?

[rpominov]

We use Lefthook to apply prettier during commit, our config looks like this:

pre-commit:
  parallel: true
  commands:
    js:
      glob: "*.js"
      run: yarn prettier --write {staged_files} && git add {staged_files}

This works fine, until we try to use partial staging (git add -p) to stage only some lines of a file. The git add {staged_files} from the hook then adds entire file, and entire file gets committed.

Is there a way to handle this use-case?

[Arkweid]

There aren't prebuilt solutions. You are free to just write your own solution, it probably will looks like this:

run: git stash -k && prettier && git stash pop

You could find more info about possible solutions here: https://github.com/okonet/lint-staged/issues/62

PS. If you'll find a good solutions, feel free to make the PR in README.md for other users :)

[Bertg]

I don't think there is full proof solution to this problem. The basic idea of git stash -k && prettier && git stash pop can be made to work (keeping in mind empty stashes etc...) but issues will arise.

A tool like prettier (or any tool that modifies the code in the commit) cam modify in such a way that the apply/pop step could fail. What should the Dev experience then be. Continue the commit? First figure out the issues? Before this developer workflow is defined, we can't start working on a actually fix for the problem.

[glasserc]

I think this is a duplicate of #60.

[filipemir]

Fwiw: lint-staged handles this case very gracefully by default. But it did take quite a bit of legwork to get it to that spot. Here's the PR introducing this feature: https://github.com/okonet/lint-staged/pull/75/ Would love to see this being introduced into lefthook, though I'm unsure I'll have the time to pursue it myself.

The straight-forward use of git stash @Arkweid mentions above doesn't quite cut it. When you pop the stash, it will reintroduce both the unstaged changes (which we want) and the staged changes before you run an auto-formatter on them (which we don't want). That will create a merge conflict between the auto-formatted changes and the not-yet-formatted stashed changes. As I understand it there's no easy way to tell git to ONLY stash unstaged changes (the -k flag stashes unstaged changes and staged changes, while leaving the staged changes in place).

[DaJoTo]

@filipemir you could make it perform an interim commit that skips any hooks. So anything staged would live in the commit, could then do git stash -u which would grab anything left over including unstaged files. At this point the head should be clean, then just reset the interim commit. You now have the stash without the staged changes, and only the staged changes to play with. I'm not sure this guarantees no conflict though, the stash will still expect lines from the staged stuff in a state it's familiar with and they are mutable. I suppose the only foolproof way would be multiple lints, so first do a lint, then do the commit, then the stash, then reset commit, then lint again, add the changes and pop the stash; all of this assumes it's acceptable to modify the unstaged parts too.

[filipemir]

@DaJoTo I don't think it's possible to have 100% guarantees of no conflict. What you suggest makes sense to me as well, but i do think it's tricky to handle all cases gracefully. That's the bulk of the lint-staged's code, and why i pointed it out as an alternative. In my experience their approach has worked well. I ended up adapting a system using it for my purposes

[scop]

Haven't used lefthook myself yet, but eyeballing it.

To me the approach taken by pre-commit has worked so well that I have never even thought about it. In a nutshell, it boils down to: if applying the stashed changes fails, undo all changes made to the tree (presumably by a formatter), and then apply the stashed changes (which should at this point "always" succeed, if it doesn't, then just fail and let the user fix it up).

The current implementation is at https://github.com/pre-commit/pre-commit/blob/d021bbfabda4e30152afc1c8c2ebf2a405b7c55e/pre_commit/staged_files_only.py (aside, uses external patchfile for the stash instead of the git stash, which to me seems the better choice as that's guaranteed to not mess up with the git stash).

Personally, I've also never missed having the changes made by formatters automatically applied or even added for me. My gut feeling is that having the choice, I'd definitely turn such an option off, and continue to review the changes and amend the commits manually as appropriate.

Anyway FWIW for me: automated stash management is a must have, having gotten used to it with other tools. Even simple implementations of it (see e.g. the pre-commit one) are so quirkfilled that replicating that boilerplate everywhere in definitions/scripts of my own is a no go.

[Bertg]

We added a script to detect if there are partially staged files, it prevents prettier from running, and failed the hook.

It's not an ideal situation, but at least it prevents left hook from doing unexpected changes.

lefthook.yml

pre-commit:
  parallel: true
  commands:
    prettier:
      glob: "*.{js,ts,jsx,tsx,yml,json,md}"
      run: scripts/detect-partial-staged-files && prettier -u --write {staged_files} && git add {staged_files}

scripts/detect-partial-staged-files

#!/bin/bash

partialy_staged_files=$(comm -12 <(git diff --name-only | sort) <(git diff --name-only --staged | sort))

if [ -n "$partialy_staged_files" ]; then
  echo
  echo "We detected partially staged files."
  echo "$partialy_staged_files"
  echo
  echo "Use git stash -k to stash away your unstanged chunks."
  exit 1
fi

I'm sure this can be optimised, by applying the glob and exclude filters in the script. I anyone wants to help out here, that would be great.

Maybe this can be added to the left hook project? Could look like this:

pre-commit:
  parallel: true
  commands:
    prettier:
      glob: "*.{js,ts,jsx,tsx,yml,json,md}"
      halt_on_partially_staged_files: true
      run: scripts/detect-partial-staged-files && npm_config_yes=true npx [email protected] -u --write {staged_files} && git add {staged_files}

[jcook-uptycs]

https://pre-commit.com/ does handle this. I think it does it by doing a stash before it start running any of the tools/command.

By having it run outside of the commands:runs: means it is handled for everything the same, also it avoids big issues that would happen with running stash on each command in combination with parallel: true

[mrexox]

This feature is on development now. I want to add the same mechanism as in pre-commit and enable it by default. Files will be stashed before all commands and scripts, so other commands won't need to deal with them. And this will only be applied to pre-commit hook.

WIP is here: https://github.com/evilmartians/lefthook/pull/402

[scop]

Nice. Just a minor correction/clarification/note related to @jcook-uptycs' comment and repeating mine above (see link in it): pre-commit handles this indeed, but the stashing it does is not git stash; it does its thing using a separate patchfile, which has the benefit that it won't mess with the user's actual git stash. I suggest adopting a similar approach here.

[mrexox]

Hey everyone! I've released this behavior in a new v1.3.0 version. Please, update and test it. I'll be glad to hear any feedback (and fix issues as soon as posible, if they appear).

Now lefthook should behave like pre-commit - it saves the diff, stashes the unstaged changes, applied the hooks, and recovers the changed from diff. If anything goes wrong - there is a stash which can be used a backup.

[mrexox]

Closing this issue. Please open a new one if you find a bug related to this!