Showing stash state in bash prompt
PATCH 1/1 Showing stash state in bash prompt
Greetings
Daniel
Signature · 14 lines
Daniel Trstenjak Tel : +49 (0)7071-9457-264 science + computing ag FAX : +49 (0)7071-9457-511 Hagellocher Weg 73 mailto: [email protected] D-72070 Tübingen WWW : http://www.science-computing.de/ -- Vorstand/Board of Management: Dr. Bernd Finkbeiner, Dr. Roland Niemeier, Dr. Arno Steitz, Dr. Ingrid Zech Vorsitzender des Aufsichtsrats/ Chairman of the Supervisory Board: Michel Lepert Sitz/Registered Office: Tuebingen Registergericht/Registration Court: Stuttgart Registernummer/Commercial Register No.: HRB 382196
contrib/completion/git-completion.bash +7 -2
@@ -117,6 +117,7 @@ __git_ps1 () local w local i + local s local c if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
@@ -136,15 +137,19 @@ __git_ps1 () else i="#" fi + stash_list=`git stash list` + if [ -n "$stash_list" ]; then + s="$" + fi fi fi fi if [ -n "$b" ]; then if [ -n "${1-}" ]; then - printf "$1" "$c${b##refs/heads/}$w$i$r" + printf "$1" "$c${b##refs/heads/}$w$i$s$r" else - printf " (%s)" "$c${b##refs/heads/}$w$i$r" + printf " (%s)" "$c${b##refs/heads/}$w$i$s$r" fi fi fi
Heya,
On Wed, May 13, 2009 at 11:44, Daniel Trstenjak
<[email protected]> wrote:
Please have a look at Documentation/SubmittingPatches, available online at [0], main points that you need to address:
1. inline your patch rather than attaching it
2. commit message
3. signed-off-by line
Feel free to ask if you have any questions wrt how to submit your patch if the document is unclear.
[0] http://repo.or.cz/w/git.git?a=blob;f=Documentation/SubmittingPatches
Signature · 3 lines
Cheers, Sverre Rabbelier
Showing stash state in bash prompt.
--- contrib/completion/git-completion.bash | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
contrib/completion/git-completion.bash +7 -2
@@ -117,6 +117,7 @@ __git_ps1 () local w local i + local s local c if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
@@ -136,15 +137,19 @@ __git_ps1 () else i="#" fi + stash_list=`git stash list` + if [ -n "$stash_list" ]; then + s="$" + fi fi fi fi if [ -n "$b" ]; then if [ -n "${1-}" ]; then - printf "$1" "$c${b##refs/heads/}$w$i$r" + printf "$1" "$c${b##refs/heads/}$w$i$s$r" else - printf " (%s)" "$c${b##refs/heads/}$w$i$r" + printf " (%s)" "$c${b##refs/heads/}$w$i$s$r" fi fi fi
Signature · 12 lines
1.6.2 -- Vorstand/Board of Management: Dr. Bernd Finkbeiner, Dr. Roland Niemeier, Dr. Arno Steitz, Dr. Ingrid Zech Vorsitzender des Aufsichtsrats/ Chairman of the Supervisory Board: Michel Lepert Sitz/Registered Office: Tuebingen Registergericht/Registration Court: Stuttgart Registernummer/Commercial Register No.: HRB 382196
Quoted 1 line · Daniel Trstenjak
Showing stash state in bash prompt.
That's what you already said in the "Subject:" ;-)
Here in the proposed commit message, you describe what the code after applying your patch does in a bit more detail, defend why such a new behaviour is desirable, and defend why your implementation is superior than possible alternative solutions to achieve that goal.
Going back to the "Subject: ", we typically put the name of the affected area and colon to the message, and use imperative mood, as if you are giving an order to the code to "do this (instead of what you currently do)".
E.g.
Subject: [PATCH] completion: show presense of stashed changes
Users often forget that there are stashed changes that want to be unstashed. Add a '$' in the prompt string to remind them.
Note that I do not necessarily agree with the above justification; I am just illustrating the established style around here.
contrib/completion/git-completion.bash __git_ps1 () · line 123
Quoted 12 lines
contrib/completion/git-completion.bash +1
-0@@ -117,6 +117,7 @@ __git_ps1 () local w local i + local s local c if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
Ok.
contrib/completion/git-completion.bash __git_ps1 () · line 140
Quoted 5 lines
contrib/completion/git-completion.bash +1
-0@@ -136,15 +137,19 @@ __git_ps1 () else i="#" fi + stash_list=`git stash list`
Don't you want to localize this variable to avoid contaminating the namespace?
contrib/completion/git-completion.bash __git_ps1 () · line 142
Quoted 2 lines
contrib/completion/git-completion.bash +2
-0+ if [ -n "$stash_list" ]; then + s="$"
Is the presense the only thing that matters? Notice that I reworded your "stash state" to "presense of" to clarify that you are giving only one bit of information in the counter-proposed "Subject:" above.
I personally prefer your "only one bit" patch over a possible alternative to count the stash entries with "git stash list | wc -l" that would be way more costly, and if you thought about such an alternative and discarded it, it would save other people's time if you say so in your proposed commit message. So...
Subject: [PATCH] completion: show presense of stashed changes
Users often forget that there are stashed changes that want to be unstashed. Add a '$' in the prompt string to remind them when there is a stashed state.
We could count and show the number of stash entries instead, but that would be more costly at runtime than it is worth.
and with localization of stash_list variable I think the patch becomes better.
Thanks.
Quoted 3 lines · Junio C Hamano
Quoted 1 line · Daniel Trstenjak
Showing stash state in bash prompt.
[...]
Quoted 4 lines
Subject: [PATCH] completion: show presense of stashed changes
Users often forget that there are stashed changes that want to be unstashed. Add a '$' in the prompt string to remind them.
I'd hate to see this go in without any sort of configurability, since I would have to patch it out again for my own builds.
The way git-stash is currently documented teaches a stash/apply workflow, in which pop is not used at all. For example, in git-stash.txt itself, the description of 'apply' has all the meat while 'pop' just refers to 'apply'. In user-manual.txt, there is an example that teaches stash/apply.
This may historically be because 'git stash pop' was added later on:
bd56ff5 (git-stash: add new 'pop' subcommand, 2008-02-22) f2c66ed (Add git-stash script, 2007-06-30)
But IMHO it would not be a good idea to teach people stash/pop anyway:
'stash drop' is irreversible, because the stash is itself implemented through the reflog and thus not guarded by one.
And unfortunately, for people who use stash/apply instead of stash/pop (including me :-), the proposed indicator merely shows if they have ever used stash in the current repository.
Signature · 2 lines
Thomas Rast
trast@{inf,student}.ethz.chQuoted 3 lines · Thomas Rast
But IMHO it would not be a good idea to teach people stash/pop anyway:
'stash drop' is irreversible, because the stash is itself implemented through the reflog and thus not guarded by one.
I don't understand why you say this -- sure "drop" is dangerous, but that's exactly why you should use "pop" instead, because it makes sure the changes are _somewhere_. I found with the old (pre-"pop") stash, I'd often end up in a situation where I'd lose track of whether I had done a stash apply or not, and the risk of inadvertently doing a drop _without_ a corresponding apply was very real.
stash/pop is safer, more convenient, and I think it better matches how people actually work.
-Miles
Signature · 4 lines
What the fuck do white people have to be blue about!? Banana Republic ran out of Khakis? The Espresso Machine is jammed? Hootie and The Blowfish are breaking up??! Shit, white people oughtta understand, their job is to GIVE people the blues, not to get them! -- George Carlin
Quoted 11 lines · Miles Bader
Quoted 3 lines · Thomas Rast
But IMHO it would not be a good idea to teach people stash/pop anyway:
'stash drop' is irreversible, because the stash is itself implemented through the reflog and thus not guarded by one.I don't understand why you say this -- sure "drop" is dangerous, but that's exactly why you should use "pop" instead, because it makes sure the changes are _somewhere_. I found with the old (pre-"pop") stash, I'd often end up in a situation where I'd lose track of whether I had done a stash apply or not, and the risk of inadvertently doing a drop _without_ a corresponding apply was very real.
"pop" doesn't always succeed. If you have conflicts in applying, then you end up with conflict markers, and the stash remains. You then fix up and commit as you see fit, but your stash is still there. So this bash prompt will nag you, which I think is what Thomas was complaining about (but perhaps the nagging would then convince you to keep a cleaner stash area by dropping the resolved stash).
-Peff
Quoted 2 lines
Quoted 2 lines
I'd often end up in a situation where I'd lose track of whether I had done a stash apply or not, and the risk of inadvertently doing a drop
While we're on this - would anyone else like to see a "git unstash" as an alias to "git stash apply" ?
For me it seems more natural to be able to do :
git stash
something
git unstash
Quoted 10 lines · John Tapsell
Quoted 2 lines
Quoted 2 lines
I'd often end up in a situation where I'd lose track of whether I had done a stash apply or not, and the risk of inadvertently doing a drop
While we're on this - would anyone else like to see a "git unstash" as an alias to "git stash apply" ?
For me it seems more natural to be able to do :git stash
something
git unstash
Try this:
git config --global alias.unstash "stash apply"
~~ B
2009/5/15 Brian Gernhardt <[email protected]>:
Quoted 1 line
git config --global alias.unstash "stash apply"
Tho it should really be "stash pop"
-miles
Signature · 1 line
Do not taunt Happy Fun Ball.
RFC PATCH 1/1 Documentation: teach stash/pop workflow instead of stash/apply
In reply to Jeff KingRecent discussion on the list showed some comments in favour of a stash/pop workflow:
http://marc.info/?l=git&m=124234911423358&w=2
http://marc.info/?l=git&m=124235348327711&w=2
Change the stash documentation and examples to document pop in its own right (and apply in terms of pop), and use stash/pop in the examples.
---
I meant to write this for a while now, but never got around to it.
Quoted 8 lines · Jeff King
Quoted 6 lines · Miles Bader
I don't understand why you say this -- sure "drop" is dangerous, but that's exactly why you should use "pop" instead, because it makes sure the changes are _somewhere_. I found with the old (pre-"pop") stash, I'd often end up in a situation where I'd lose track of whether I had done a stash apply or not, and the risk of inadvertently doing a drop _without_ a corresponding apply was very real.
Quoted 6 lines
"pop" doesn't always succeed. If you have conflicts in applying, then you end up with conflict markers, and the stash remains. You then fix up and commit as you see fit, but your stash is still there. So this bash prompt will nag you, which I think is what Thomas was complaining about (but perhaps the nagging would then convince you to keep a cleaner stash area by dropping the resolved stash).
Actually I was mostly concerned about dropping the stashes at all. But I guess if you treat the stash as a short-term stack that holds a change or two while you're working on something else, stash/pop fits better.
I'd still prefer some configurability in the original patch, as I think nagging the user into discarding data is a bad thing, even though I now agree that if the 'pop' actually went through, it's not really discarded. (Also, ISTR a discussion about automatic gc'ing of the stash reflog where a few people said they expect to hit any reasonably short time limit in some of their repos and thus risk losing work; regardless of cleanup disclipline, they would also have the prompt on all the time).
By the way, why doesn't gmane find these mails? I tried things like
http://search.gmane.org/?query=stash&group=gmane.comp.version-control.git&[email protected]
but the entire thread seems to be missing from gmane.
Documentation/git-stash.txt | 30 ++++++++++++++++-------------- Documentation/user-manual.txt | 4 ++-- 2 files changed, 18 insertions(+), 16 deletions(-)
Documentation/git-stash.txt +16 -14
@@ -75,14 +75,22 @@ show [<stash>]:: it will accept any format known to 'git-diff' (e.g., `git stash show -p stash@\{1}` to view the second most recent stash in patch form). -apply [--index] [<stash>]:: +pop [<stash>]:: - Restore the changes recorded in the stash on top of the current - working tree state. When no `<stash>` is given, applies the latest - one. The working directory must match the index. + Remove a single stashed state from the stash list and apply it + on top of the current working tree state, i.e., do the inverse + operation of `git stash save`. The working directory must + match the index. + -This operation can fail with conflicts; you need to resolve them -by hand in the working tree. +Applying the state can fail with conflicts; in this case, it is not +removed from the stash list. You need to resolve the conflicts by hand +and call `git stash drop` manually afterwards. ++ +When no `<stash>` is given, `stash@\{0}` is assumed. See also `apply`. + +apply [--index] [<stash>]:: + + Like `pop`, but do not remove the state from the stash list. + If the `--index` option is used, then tries to reinstate not only the working tree's changes, but also the index's ones. However, this can fail, when you
@@ -112,12 +120,6 @@ drop [<stash>]:: Remove a single stashed state from the stash list. When no `<stash>` is given, it removes the latest one. i.e. `stash@\{0}` -pop [<stash>]:: - - Remove a single stashed state from the stash list and apply on top - of the current working tree state. When no `<stash>` is given, - `stash@\{0}` is assumed. See also `apply`. - create:: Create a stash (which is a regular commit object) and return its
@@ -163,7 +165,7 @@ $ git pull file foobar not up to date, cannot merge. $ git stash $ git pull -$ git stash apply +$ git stash pop ---------------------------------------------------------------- Interrupted workflow::
@@ -192,7 +194,7 @@ You can use 'git-stash' to simplify the above, like this: $ git stash $ edit emergency fix $ git commit -a -m "Fix in a hurry" -$ git stash apply +$ git stash pop # ... continue hacking ... ----------------------------------------------------------------
Documentation/user-manual.txt +2 -2
@@ -1520,10 +1520,10 @@ $ git commit -a -m "blorpl: typofix" ------------------------------------------------ After that, you can go back to what you were working on with -`git stash apply`: +`git stash pop`: ------------------------------------------------ -$ git stash apply +$ git stash pop ------------------------------------------------
Signature · 1 line
1.6.3.1.276.gb65cd
Re: [RFC PATCH] Documentation: teach stash/pop workflow instead of stash/apply
FWIW, I think the change makes sense.
Re: [RFC PATCH] Documentation: teach stash/pop workflow instead of stash/apply
In reply to Thomas RastQuoted 8 lines · Thomas Rast
Recent discussion on the list showed some comments in favour of a stash/pop workflow:
http://marc.info/?l=git&m=124234911423358&w=2
http://marc.info/?l=git&m=124235348327711&w=2Change the stash documentation and examples to document pop in its own right (and apply in terms of pop), and use stash/pop in the examples.
[snip]
Documentation/git-stash.txt show [<stash>]:: · line 87
Quoted 3 lines
Documentation/git-stash.txt +3
-0+Applying the state can fail with conflicts; in this case, it is not +removed from the stash list. You need to resolve the conflicts by hand +and call `git stash drop` manually afterwards.
For what it is worth, when I take classes on git in $DAYJOB I insist on using only save/pop (and drop when needed as described above). My students are largely freshers with little or no dev experience, and this just seems much more intuitive. Even the aspect of having to drop manually is logical, because something exceptional happened.
I certainly would appreciate this patch, to answer the "RFC" aspect in your subject line.