Skip to content
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

fix: eslint pre-commit hook now supports partially staged files #16137

Closed
wants to merge 1 commit into from

Conversation

peter-sanderson
Copy link
Contributor

@peter-sanderson peter-sanderson commented Jan 2, 2025

This PR tweaks the pre-commit hook to enable working with both staged and unstaged files.

In case you have staged changes on the file that you want to commit and then some other unstaged changes on the very same file, this fix will preserve the unstaged changes without committing them.

❗ 👉 The Git version at least 2.35 is required for this to work. For older Ubuntu the git can be updated from this repository: https://launchpad.net/%7Egit-core/+archive/ubuntu/ppa


The unsolved issue is, that if you have staged and unstaged changes next to each other, it will produce conflict

Staged:
image

Unstaged:

image

Conflict:

image

@peter-sanderson peter-sanderson added the no-project This label is used to specify that PR doesn't need to be added to a project label Jan 2, 2025
@peter-sanderson peter-sanderson marked this pull request as draft January 2, 2025 12:35
@peter-sanderson peter-sanderson force-pushed the pre-commit-hook-only-partial-files branch 2 times, most recently from 21ed04a to 7236dd9 Compare January 2, 2025 14:39
@peter-sanderson peter-sanderson marked this pull request as ready for review January 2, 2025 14:39
@@ -24,14 +26,49 @@ else
echo "$STAGED_FILES"
echo ""

# This will stashed all changes that are not added (the red files), but keeps added (the green files)
# echo -e "Stashing the ${GREEN}staged${NC} files."
git stash push --staged -q # Stash staged changes (green files)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--staged supported by git version 2.35+

@peter-sanderson peter-sanderson force-pushed the pre-commit-hook-only-partial-files branch from 7236dd9 to 35abf43 Compare January 6, 2025 16:16
@peter-sanderson peter-sanderson force-pushed the pre-commit-hook-only-partial-files branch from 35abf43 to d13fd28 Compare January 6, 2025 16:18
@Lemonexe
Copy link
Contributor

Lemonexe commented Jan 6, 2025

FWIW I am against this. Those are potentially destructive actions that can cause our work to be lost, and the code is not trivial. IMHO we would need "unit tests", if we were to trust it. And I don't think it's worth the engineering effort to write unit tests in bash to test git operations on file system 🤷

On the other hand, the current pre-commit script runs only eslint fix, which I believe to be trustworthy.

I am aware of the problem this PR solves. I guess, that if you need to commit partially staged files, you have to remember to do it with -n 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-project This label is used to specify that PR doesn't need to be added to a project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants