-
Notifications
You must be signed in to change notification settings - Fork 688
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
Feature auto-stash #531
base: master
Are you sure you want to change the base?
Feature auto-stash #531
Conversation
git-ftp
Outdated
} | ||
|
||
unset_stash(){ | ||
[ $AUTO_STASH -eq 1 ] && [ $(git stash list -n 1 | grep -q "Stashed by Git-ftp") ] && git stash pop -q |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will work in most cases but will go wrong from time to time (leading to weird issue reports). Example:
- Working a feature A using auto stash.
- Something goes wrong (network failure) and Git-ftp exits with an error.
- I start debugging / reading docs and give up after a while.
- I start feature B.
- After pushing feature B, my stack from feature A is restored, possibly with conflicts.
A variation of this: I had multiple failures and several Git-ftp stash commits are piled up. They re-surface whenever I have a clean working directory.
Ideally, the stash command would tell us the commit it saved. But it doesn't. We can check the output for No local changes to save
and skip the pop in that case. Or we extend the stash message with a random token and grep for that. For example:
git stash push ... -m "Stashed by Git-ftp. Token: $token"
# ...
[ $(git stash list -n 1 | grep -q "Stashed by Git-ftp. Token: $token") ] && git stash pop
But that's probably overcomplicating it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your first two cases could be handled by just calling unset_stash
during cleanup
which is called before the program exits with an error.
I don’t think it’s that easy to check the output string. Some people have localised versions of git so the output could be in another language and wouldn’t match. But I like the idea with the token. We could just use the current system time (mills) for this. I’ll try this.
git-ftp
Outdated
set_stash() { | ||
local stash_config="$(get_config auto-stash)" | ||
[ -n "$stash_config" ] && AUTO_STASH="$(boolean $stash_config)" | ||
[ $AUTO_STASH -eq 1 ] && git stash push --include-untracked -q -m "Stashed by Git-ftp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The push
option is new. My Git version doesn't know it yet. Maybe we should use git stash save
for compatibility. Newer versions still recognise it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you should consider to update your git version? 😜😉
No, seriously. You're right, they still recognise it, but save
is deprecated since almost two years. So I think it’s better to use the future-proof command, isn’t it?
What we could do is check the git-version and say “Oh sorry, this option is not supported for your git version. Please update git or stash your changes manually.”
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a lot of value in long term support. Don't change a running system. That's why I'm working with Debian stable. No surprises, it just works. My current version will still be supported for another three years.
Uh, I just saw that a new Debian stable was released ten days ago. So yes, I will update my Git version along with everything else soon! :-)
The auto-stash feature calls `git stash` before checking for a dirty repo. It also stashes untracked/new files so the repo is really clean. After doing the other stuff the stash is applied (popped) again, so the state of the working tree is restored. If there are no files to stash, git stash succeds without adding a stash entry. Therefore we have to check if the last stash is really made from Git-ftp before poping. In addition to the option the config is suported, too.
Add a few tests for the new auto-stash feature. As well a test for dirty repositories.
b57e157
to
7a2dd5f
Compare
I intentionally did the version check in an extra commit to keep it as a separate step. But If you want to, i can combine this with the token commit, because some of the changes made there are overwritten with the version check. |
Since `git stash push` was introduced with v2.13 we have to check for older git versions. This check is added and to avoid parsing the output of `git --version` multiple times, the version check is refactored.
7a2dd5f
to
7ae8adf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good. I accidentally overlooked some of the commits. Thank you for keeping on top of it.
if [ $AUTO_STASH -eq 1 ]; then | ||
TMP_STASH_TOKEN="$(date +%s)" | ||
local stash_message="Stashed by Git-ftp. Token: $TMP_STASH_TOKEN" | ||
if [ "${GIT_VERSION[0]}" -le 2 ] && [ "${GIT_VERSION[1]}" -lt 13 ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, we are relying on Git not to publishing version 1.13. I guess that's a save bet. ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the version check in check_git_version
freaked me out. I thought it would allow 1.8 first, but it doesn’t. So I decided to go a similar way here to keep it simple.
Hello Any progress about this feature ? Could be nice to be to merge it :) Thanks |
The auto-stash feature calls
git stash
before checking for a dirty repo. It also stashes untracked/new files so the repo is really clean. After doing the other stuff the stash is applied (popped) again,so the state of the working tree is restored.
If there are no files to stash, git stash succeds without adding a stash entry.
Therefore we have to check if the last stash is really made from Git-ftp before poping.
In addition to the option the config is supported, too.
Auto-stash is supported for actions init, push and catchup.
resolves #236