-
Notifications
You must be signed in to change notification settings - Fork 222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Is there a way to change (prettify) partially staged files in pre-commit? #140
Comments
There aren't prebuilt solutions.
You could find more info about possible solutions here: PS. If you'll find a good solutions, feel free to make the PR in README.md for other users :) |
I don't think there is full proof solution to this problem. The basic idea of 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. |
I think this is a duplicate of #60. |
Fwiw: The straight-forward use of |
@filipemir you could make it perform an interim commit that skips any hooks. So anything staged would live in the commit, could then do |
@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 |
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. |
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.ymlpre-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. 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} |
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 |
This feature is on development now. I want to add the same mechanism as in WIP is here: #402 |
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 |
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 |
Closing this issue. Please open a new one if you find a bug related to this! |
We use Lefthook to apply prettier during commit, our config looks like this:
This works fine, until we try to use partial staging (
git add -p
) to stage only some lines of a file. Thegit add {staged_files}
from the hook then adds entire file, and entire file gets committed.Is there a way to handle this use-case?
The text was updated successfully, but these errors were encountered: