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

[Question] Why not use git stash save --keep-index #602

Closed
drmercer opened this issue Apr 17, 2019 · 4 comments · Fixed by #724
Closed

[Question] Why not use git stash save --keep-index #602

drmercer opened this issue Apr 17, 2019 · 4 comments · Fixed by #724

Comments

@drmercer
Copy link

drmercer commented Apr 17, 2019

Thanks for the great tool! I've noticed something that might be a way to improve it. Whenever I have partially-staged files, the "Stashing changes..." part takes a frustratingly long time, as does the "Restoring local changes..." part afterwards. It also makes git status have to take several seconds to refresh the index first when I use it after lint-staged runs.

I looked into the source and found the lines that do the stashing:
https://github.com/okonet/lint-staged/blob/63b085f7b8a6ae953c0070fcb4122499fdb73542/src/gitWorkflow.js#L67-L82

Is there a reason these can't be replaced by a call to
git stash save --keep-index 'Automatic stash by lint-staged', and then git stash pop to unstash?

(I'd be happy to open a PR if that's a viable change.)

@drmercer
Copy link
Author

I found #583 which talks about this more. Still unsure why the current solution works better than git stash --include-untracked --keep-index (which could introduce conflicts apparently). How does the current solution avoid introducing conflicts? 🤔 I'll do some experimenting if I have time.

@okonet
Copy link
Collaborator

okonet commented Jun 10, 2019

Hey @drmercer sorry for a late reply.

Of course git stash -k was the first thing I tried a few years ago but the problem with internal stash is that if there are conflicts then it's going to fail the stash pop operation with a very cryptic message. In order to avoid this I'm using a low level approach proposed by someone in the discussion to the issue.

The way it works now is described in the PR. The important bit is:

Compare new commit index and working copy and apply as many formatting changes to working copy as possible. If there are conflicting hunks, drop formatter's changes. User modifications that were stashed should always take precedence over formatters.

And this is where it's implemented: https://github.com/okonet/lint-staged/pull/75/files#diff-cf453db1a0c4f207753345e32ef28aa3R121

Feel free to experiment since there is a robust test coverage (I've invested a lot of time into TDD for this one). I'd be open for a better or more "cleaner" solution to the problem but I could not find one.

@okonet
Copy link
Collaborator

okonet commented Oct 31, 2019

🎉 This issue has been resolved in version 9.5.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@okonet
Copy link
Collaborator

okonet commented Jan 19, 2020

🎉 This issue has been resolved in version 10.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants