[PATCH] bash: offer to show (un)staged changes

[PATCH] bash: offer to show (un)staged changes

[Thomas Rast]

Add a bit of code to __git_ps1 that lets it append '*' to the branch
name if there are any uncommitted changes, and '+' if there are any
staged changes.

Since this is a rather expensive operation and will force a lot of
data into the cache whenever you first enter a repository, you have to
enable it manually by setting GIT_PS1_EXPENSIVE to something nonempty.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

I came up with this after sending two incomplete patches on the same
night, and really like it.  Perhaps others might find it useful.

Of course it would be brilliant if there were a way to ask the kernel
if a certain directory is cached, but I couldn't find one, let alone
one accessible from the shell.


 contrib/completion/git-completion.bash |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f8b845a..36ea528 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -34,6 +34,10 @@
 #       are currently in a git repository.  The %s token will be
 #       the name of the current branch.
 #
+#	In addition, if you set GIT_PS1_EXPENSIVE to a nonempty value,
+#	unstaged (*) and staged (+) changes will be shown next to the
+#	branch name.
+#
 # To submit patches:
 #
 #    *) Read Documentation/SubmittingPatches
@@ -116,10 +120,23 @@ __git_ps1 ()
 			fi
 		fi
 
+		local w
+		local i
+
+		if test ! -z "$GIT_PS1_EXPENSIVE"; then
+			git update-index --refresh >/dev/null 2>&1 || w="*"
+			if git rev-parse --quiet --verify HEAD >/dev/null; then
+				git diff-index --cached --quiet \
+					--ignore-submodules HEAD -- || i="+"
+			else
+				i="#"
+			fi
+		fi
+
 		if [ -n "${1-}" ]; then
-			printf "$1" "${b##refs/heads/}$r"
+			printf "$1" "${b##refs/heads/}$w$i$r"
 		else
-			printf " (%s)" "${b##refs/heads/}$r"
+			printf " (%s)" "${b##refs/heads/}$w$i$r"
 		fi
 	fi
 }
-- 
tg: (7bbd8d6..) t/ps1-dirty-state (depends on: origin/master)

Re: [PATCH] bash: offer to show (un)staged changes

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> +		if test ! -z "$GIT_PS1_EXPENSIVE"; then
> +			git update-index --refresh >/dev/null 2>&1 || w="*"

This makes the feature unavailable for people who care about the stat
dirtiness and explicitly set diff.autorefreshindex to false, doesn't it?

Re: [PATCH] bash: offer to show (un)staged changes

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > +		if test ! -z "$GIT_PS1_EXPENSIVE"; then
> > +			git update-index --refresh >/dev/null 2>&1 || w="*"
> 
> This makes the feature unavailable for people who care about the stat
> dirtiness and explicitly set diff.autorefreshindex to false, doesn't it?

True, and I admit I didn't know there was an option to change that.
OTOH git-diff-files doesn't normally update the index even if the
option is set.  Should I ask 'git diff --exit-code --raw' or some such
instead?

(Why would people want to keep the stat info dirty even though there
may not have been any changes?)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch




[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] bash: offer to show (un)staged changes

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> I came up with this after sending two incomplete patches on the same
> night, and really like it.  Perhaps others might find it useful.

Any patch worth discussing (on this list at least) would need a nontrivial
commit log message that you need to really think while writing.  It is
natural to assume people would be making them with their editor, not with
"commit -m".  These two incomplete patches could have been avoided if you
paid attention to the status output that is in the commit log message
buffer.  Perhaps we should make it even louder in some way?

Re: [PATCH] bash: offer to show (un)staged changes

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 2037 bytes --]

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > I came up with this after sending two incomplete patches on the same
> > night, and really like it.  Perhaps others might find it useful.
> 
> Any patch worth discussing (on this list at least) would need a nontrivial
> commit log message that you need to really think while writing.  It is
> natural to assume people would be making them with their editor, not with
> "commit -m".  These two incomplete patches could have been avoided if you
> paid attention to the status output that is in the commit log message
> buffer.  Perhaps we should make it even louder in some way?

Actually I tend to write the commit (and message) sometime halfway
through, and then amend the commit with fixes, docs and such, possibly
tweaking the message if I need to.  That night I just forgot to amend
before format-patch, and there's no status message at that point which
could have reminded me.  So the *+ display is just what I needed; it
shows the status right before I get a chance to format-patch (or
whatever else command expects a commit).

[As a side note, this kind of workflow is what will probably prevent
me from working with any other SCM in the near future.  I simply
cannot imagine going back to a world without add -p, commit --amend
and rebase -i.]

That being said, I never look at that status message; so far I've been
too lazy to make my emacs add syntax highlighting there, and without
it, it's just a big chunk of text.  In fact, for most commits the 1-2
file names completely drown in the big chunk of surrounding,
invariant, instructions.  If it becomes even louder in the ASCII
dimension, that most likely means I'll just have to steer my eye away
from it even harder to see the commit message I'm typing.  Perhaps
some colours would help, I should really try that.

Of course the real solution would be to hack less and sleep more, but
who would want to do that?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] bash: offer to show (un)staged changes

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > +		if test ! -z "$GIT_PS1_EXPENSIVE"; then
> > +			git update-index --refresh >/dev/null 2>&1 || w="*"
> 
> This makes the feature unavailable for people who care about the stat
> dirtiness and explicitly set diff.autorefreshindex to false, doesn't it?

Yup, and I'm one of those people who sets autorefresindex to false
in my ~/.gitconfig, usually before I even have user.{name,email} set.

I do like the idea of what Thomas is trying to do here, but its
so bloody expensive to compute dirty state on every prompt in
some repositories that I'd shoot myself.  E.g. WebKit is huge,
computing the dirty state inside of the WebKit repository on each
prompt would absolutely kill CLI performance to a point of it not
being usuable.  But git.git is small enough its OK on pretty much
everything except Cygwin.

So as much as I'd like to use this without the update-index --refresh
bit, I'm not sure its viable in every project out there.  If we had
an inotify sort of daemon to keep the data current so the prompt
doesn't have to stat every source file on every display it would
be reasonable, but we don't have such a thing yet for Git.

-- 
Shawn.

Re: [PATCH] bash: offer to show (un)staged changes

[Martin Langhoff]

On Mon, Jan 19, 2009 at 12:29 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> ...  If we had
> an inotify sort of daemon to keep the data current so the prompt
> doesn't have to stat every source file on every display it would
> be reasonable, but we don't have such a thing yet for Git.

Note that inotify is not recursive and is a hog if you ask it to
monitor lots of dents.

I am not convinced that an inotify-enabled git is a good idea for
anything but small/mid-sized project. And such don't need it either.

In other words, the kernel's cache will outperform any userland
attempting to keep track of the fs via inotify.

cheers,



m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

Re: [PATCH] bash: offer to show (un)staged changes

[Shawn O. Pearce]

Martin Langhoff <martin.langhoff@gmail.com> wrote:
> On Mon, Jan 19, 2009 at 12:29 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> > ...  If we had
> > an inotify sort of daemon to keep the data current so the prompt
> > doesn't have to stat every source file on every display it would
> > be reasonable, but we don't have such a thing yet for Git.
> 
> Note that inotify is not recursive and is a hog if you ask it to
> monitor lots of dents.
> 
> I am not convinced that an inotify-enabled git is a good idea for
> anything but small/mid-sized project. And such don't need it either.
> 
> In other words, the kernel's cache will outperform any userland
> attempting to keep track of the fs via inotify.

*sigh*

I was hoping it would work well for the really huge repository case,
like WebKit, where the stats against the work tree just kill the
user space application.

But if its only good for smaller sets of files, then yea, just
doing the stats on demand is going to be better than anything an
inotify daemon might be able to offer.

-- 
Shawn.

Re: [PATCH] bash: offer to show (un)staged changes

[Mike Hommey]

On Mon, Jan 19, 2009 at 10:11:58AM -0800, Shawn O. Pearce wrote:
> Martin Langhoff <martin.langhoff@gmail.com> wrote:
> > On Mon, Jan 19, 2009 at 12:29 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> > > ...  If we had
> > > an inotify sort of daemon to keep the data current so the prompt
> > > doesn't have to stat every source file on every display it would
> > > be reasonable, but we don't have such a thing yet for Git.
> > 
> > Note that inotify is not recursive and is a hog if you ask it to
> > monitor lots of dents.
> > 
> > I am not convinced that an inotify-enabled git is a good idea for
> > anything but small/mid-sized project. And such don't need it either.
> > 
> > In other words, the kernel's cache will outperform any userland
> > attempting to keep track of the fs via inotify.
> 
> *sigh*
> 
> I was hoping it would work well for the really huge repository case,
> like WebKit, where the stats against the work tree just kill the
> user space application.

I was hoping something like that too, especially for cold cache cases,
after resuming from hibernation, for instance, in which case the kernel
cache is empty and inotify tracking will help know if something changed.

Mike

Re: [PATCH] bash: offer to show (un)staged changes

[Martin Langhoff]

On Mon, Jan 19, 2009 at 1:11 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> *sigh*
>
> I was hoping it would work well for the really huge repository case,
> like WebKit, where the stats against the work tree just kill the
> user space application.

Even hot-cache? My perception is that in hot-cache conditions the perf is good.

If it is cold-cache, what in the end you are hoping for is "pegging"
some stuff in the cache. Perhaps there's a way to tell the kernel to
skew the cache eviction scheme. Still, if the kernel's algorythms are
good, the kernel knows more about your fs usage patterns than you
do...

cheers,



m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

Re: [PATCH] bash: offer to show (un)staged changes

[Boyd Stephen Smith Jr.]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

On Monday 2009 January 19 11:29:39 Shawn O. Pearce wrote:
>E.g. WebKit is huge,
>computing the dirty state inside of the WebKit repository on each
>prompt would absolutely kill CLI performance to a point of it not
>being usuable.

Is the performance any better on other VCSes for this operation?  If not, 
maybe its just a feature that's infeasible for large trees.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] bash: offer to show (un)staged changes

[Boyd Stephen Smith Jr.]

[-- Attachment #1: Type: text/plain, Size: 847 bytes --]

On Monday 2009 January 19 12:00:10 Martin Langhoff wrote:
>On Mon, Jan 19, 2009 at 12:29 PM, Shawn O. Pearce <spearce@spearce.org> 
wrote:
>> ...  If we had
>> an inotify sort of daemon to keep the data current so the prompt
>> doesn't have to stat every source file on every display it would
>> be reasonable, but we don't have such a thing yet for Git.
>
>[T]he kernel's cache will outperform any userland
>attempting to keep track of the fs via inotify.

Really?  Why have inotify then?  I thought its only purpose is "to keep track 
of the fs".  If it is never a net win, why even use/provide it?
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] bash: offer to show (un)staged changes

[Martin Langhoff]

On Mon, Jan 19, 2009 at 2:06 PM, Boyd Stephen Smith Jr.
<bss@iguanasuicide.net> wrote:
> Really?  Why have inotify then?  I thought its only purpose is "to keep track
> of the fs".  If it is never a net win, why even use/provide it?

Lots of programs want to monitor _one file_ or _one directory_. Cron
daemons monitor /etc/cron.*/ , incron monitors /etc/incron.d/ ,
programs that watch mailspool directories can be made more efficient
this way too.

You might find your system today has perhaps a dozen inotify watches.
Judiciously used, it's great.

cheers



m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

[PATCH v2] bash: offer to show (un)staged changes

[Thomas Rast]

Add a bit of code to __git_ps1 that lets it append '*' to the branch
name if there are any unstaged changes, and '+' if there are any
staged changes.

Since this is a rather expensive operation and will force a lot of
data into the cache whenever you first enter a repository, you have to
enable it manually by setting bash.showDirtyState to a true value.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>

---

Shawn O. Pearce wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
> > This makes the feature unavailable for people who care about the stat
> > dirtiness and explicitly set diff.autorefreshindex to false, doesn't it?
> 
> Yup, and I'm one of those people who sets autorefresindex to false
> in my ~/.gitconfig, usually before I even have user.{name,email} set.

I tried to alleviate the problem with a combination of diff options
that hopefully does the right thing in all cases.

> I do like the idea of what Thomas is trying to do here, but its
> so bloody expensive to compute dirty state on every prompt in
> some repositories that I'd shoot myself.  E.g. WebKit is huge,
> computing the dirty state inside of the WebKit repository on each
> prompt would absolutely kill CLI performance to a point of it not
> being usuable.  But git.git is small enough its OK on pretty much
> everything except Cygwin.
> 
> So as much as I'd like to use this without the update-index --refresh
> bit, I'm not sure its viable in every project out there.

I mostly work on small-ish repos, and while the initial loading is
_very_ noticeable, it's ok after that.  But your point about repos
being different is very good, so I changed the patch to use a
git-config variable instead of a shell environment variable.  That
way, you could even configure it to a different setting per-repo.

(Which might end up rather confusing, but at least it's possible.)


 contrib/completion/git-completion.bash |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f8b845a..7864ca7 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -34,6 +34,10 @@
 #       are currently in a git repository.  The %s token will be
 #       the name of the current branch.
 #
+#	In addition, if you set bash.showDirtyState to a true value,
+#	unstaged (*) and staged (+) changes will be shown next to the
+#	branch name.
+#
 # To submit patches:
 #
 #    *) Read Documentation/SubmittingPatches
@@ -116,10 +120,24 @@ __git_ps1 ()
 			fi
 		fi
 
+		local w
+		local i
+
+		if test "$(git config --bool bash.showDirtyState)" = "true"; then
+			git diff --no-ext-diff --ignore-submodules \
+				--quiet --exit-code || w="*"
+			if git rev-parse --quiet --verify HEAD >/dev/null; then
+				git diff-index --cached --quiet \
+					--ignore-submodules HEAD -- || i="+"
+			else
+				i="#"
+			fi
+		fi
+
 		if [ -n "${1-}" ]; then
-			printf "$1" "${b##refs/heads/}$r"
+			printf "$1" "${b##refs/heads/}$w$i$r"
 		else
-			printf " (%s)" "${b##refs/heads/}$r"
+			printf " (%s)" "${b##refs/heads/}$w$i$r"
 		fi
 	fi
 }
-- 
tg: (7bbd8d6..) t/ps1-dirty-state (depends on: origin/master)

[PATCH v2] bash: offer to show (un)staged changes

[Thomas Rast]

Add a bit of code to __git_ps1 that lets it append '*' to the branch
name if there are any unstaged changes, and '+' if there are any
staged changes.

Since this is a rather expensive operation and will force a lot of
data into the cache whenever you first enter a repository, you have to
enable it manually by setting bash.showDirtyState to a true value.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

This got no replies... was there anything wrong with v2?


 contrib/completion/git-completion.bash |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f8b845a..7864ca7 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -34,6 +34,10 @@
 #       are currently in a git repository.  The %s token will be
 #       the name of the current branch.
 #
+#	In addition, if you set bash.showDirtyState to a true value,
+#	unstaged (*) and staged (+) changes will be shown next to the
+#	branch name.
+#
 # To submit patches:
 #
 #    *) Read Documentation/SubmittingPatches
@@ -116,10 +120,24 @@ __git_ps1 ()
 			fi
 		fi
 
+		local w
+		local i
+
+		if test "$(git config --bool bash.showDirtyState)" = "true"; then
+			git diff --no-ext-diff --ignore-submodules \
+				--quiet --exit-code || w="*"
+			if git rev-parse --quiet --verify HEAD >/dev/null; then
+				git diff-index --cached --quiet \
+					--ignore-submodules HEAD -- || i="+"
+			else
+				i="#"
+			fi
+		fi
+
 		if [ -n "${1-}" ]; then
-			printf "$1" "${b##refs/heads/}$r"
+			printf "$1" "${b##refs/heads/}$w$i$r"
 		else
-			printf " (%s)" "${b##refs/heads/}$r"
+			printf " (%s)" "${b##refs/heads/}$w$i$r"
 		fi
 	fi
 }
-- 
tg: (7bbd8d6..) t/ps1-dirty-state (depends on: origin/master)

Re: [PATCH v2] bash: offer to show (un)staged changes

[Shawn O. Pearce]

Thomas Rast <trast@student.ethz.ch> wrote:
> Add a bit of code to __git_ps1 that lets it append '*' to the branch
> name if there are any unstaged changes, and '+' if there are any
> staged changes.
> 
> Since this is a rather expensive operation and will force a lot of
> data into the cache whenever you first enter a repository, you have to
> enable it manually by setting bash.showDirtyState to a true value.
> 
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
> 
> This got no replies... was there anything wrong with v2?

Dropped on the floor by me.  Sorry.

But I'm a bit worried about the config --bool test in the prompt.
Its a new fork+exec we weren't doing before.  I wonder if we should
use a shell variable to consider whether or not this should even
be executed and try to shortcut out if not.  E.g.:

  if test -n "$GIT_PS1_SHOWDIRTYSTATE"; then
    ... your new code block ...
  fi

and ask that users at some point set GIT_PS1_SHOWDIRTYSTATE=1 in
their shell startup scripts, and also set bash.showDirtyState true
in any of the repositories they care about it in.


 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index f8b845a..7864ca7 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -34,6 +34,10 @@
>  #       are currently in a git repository.  The %s token will be
>  #       the name of the current branch.
>  #
> +#	In addition, if you set bash.showDirtyState to a true value,
> +#	unstaged (*) and staged (+) changes will be shown next to the
> +#	branch name.
> +#
>  # To submit patches:
>  #
>  #    *) Read Documentation/SubmittingPatches
> @@ -116,10 +120,24 @@ __git_ps1 ()
>  			fi
>  		fi
>  
> +		local w
> +		local i
> +
> +		if test "$(git config --bool bash.showDirtyState)" = "true"; then
> +			git diff --no-ext-diff --ignore-submodules \
> +				--quiet --exit-code || w="*"
> +			if git rev-parse --quiet --verify HEAD >/dev/null; then
> +				git diff-index --cached --quiet \
> +					--ignore-submodules HEAD -- || i="+"
> +			else
> +				i="#"
> +			fi
> +		fi
> +
>  		if [ -n "${1-}" ]; then
> -			printf "$1" "${b##refs/heads/}$r"
> +			printf "$1" "${b##refs/heads/}$w$i$r"
>  		else
> -			printf " (%s)" "${b##refs/heads/}$r"
> +			printf " (%s)" "${b##refs/heads/}$w$i$r"
>  		fi
>  	fi
>  }

-- 
Shawn.

Re: [PATCH] bash: offer to show (un)staged changes

[Tuncer Ayaz]

On Mon, Jan 19, 2009 at 6:29 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
>> Thomas Rast <trast@student.ethz.ch> writes:
>>
>> > +           if test ! -z "$GIT_PS1_EXPENSIVE"; then
>> > +                   git update-index --refresh >/dev/null 2>&1 || w="*"
>>
>> This makes the feature unavailable for people who care about the stat
>> dirtiness and explicitly set diff.autorefreshindex to false, doesn't it?
>
> Yup, and I'm one of those people who sets autorefresindex to false
> in my ~/.gitconfig, usually before I even have user.{name,email} set.
>
> I do like the idea of what Thomas is trying to do here, but its
> so bloody expensive to compute dirty state on every prompt in
> some repositories that I'd shoot myself.  E.g. WebKit is huge,

I've been thinking about this and wondered
whether implementing "status --mini" or
"status --short" which prints "+?*" in wt-status.c
could be made fast enough.

Should we try to implement and profile this
or do we know it will be slow beforehand?

I am actually using this feature on the bash
prompt via calls to git commands and parsing
for added=+, unknown=? and changed=* as
others have done in combination with __git_ps1.

> computing the dirty state inside of the WebKit repository on each
> prompt would absolutely kill CLI performance to a point of it not
> being usuable.  But git.git is small enough its OK on pretty much
> everything except Cygwin.
>
> So as much as I'd like to use this without the update-index --refresh
> bit, I'm not sure its viable in every project out there.  If we had
> an inotify sort of daemon to keep the data current so the prompt
> doesn't have to stat every source file on every display it would
> be reasonable, but we don't have such a thing yet for Git.
>
> --
> Shawn.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] bash: offer to show (un)staged changes

[Junio C Hamano]

Tuncer Ayaz <tuncer.ayaz@gmail.com> writes:

> On Mon, Jan 19, 2009 at 6:29 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
>> Junio C Hamano <gitster@pobox.com> wrote:
>>> Thomas Rast <trast@student.ethz.ch> writes:
>>>
>>> > +           if test ! -z "$GIT_PS1_EXPENSIVE"; then
>>> > +                   git update-index --refresh >/dev/null 2>&1 || w="*"
>>>
>>> This makes the feature unavailable for people who care about the stat
>>> dirtiness and explicitly set diff.autorefreshindex to false, doesn't it?
>>
>> Yup, and I'm one of those people who sets autorefresindex to false
>> in my ~/.gitconfig, usually before I even have user.{name,email} set.
>>
>> I do like the idea of what Thomas is trying to do here, but its
>> so bloody expensive to compute dirty state on every prompt in
>> some repositories that I'd shoot myself.  E.g. WebKit is huge,
>
> I've been thinking about this and wondered
> whether implementing "status --mini" or
> "status --short" which prints "+?*" in wt-status.c
> could be made fast enough.
>
> Should we try to implement and profile this
> or do we know it will be slow beforehand?

I think I've seen a patch to do something like that, soon after Shawn
announced his repo tool.

Re: [PATCH] bash: offer to show (un)staged changes

[Tuncer Ayaz]

On Mon, Feb 2, 2009 at 12:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Tuncer Ayaz <tuncer.ayaz@gmail.com> writes:
>
>> On Mon, Jan 19, 2009 at 6:29 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
>>> Junio C Hamano <gitster@pobox.com> wrote:
>>>> Thomas Rast <trast@student.ethz.ch> writes:
>>>>
>>>> > +           if test ! -z "$GIT_PS1_EXPENSIVE"; then
>>>> > +                   git update-index --refresh >/dev/null 2>&1 || w="*"
>>>>
>>>> This makes the feature unavailable for people who care about the stat
>>>> dirtiness and explicitly set diff.autorefreshindex to false, doesn't it?
>>>
>>> Yup, and I'm one of those people who sets autorefresindex to false
>>> in my ~/.gitconfig, usually before I even have user.{name,email} set.
>>>
>>> I do like the idea of what Thomas is trying to do here, but its
>>> so bloody expensive to compute dirty state on every prompt in
>>> some repositories that I'd shoot myself.  E.g. WebKit is huge,
>>
>> I've been thinking about this and wondered
>> whether implementing "status --mini" or
>> "status --short" which prints "+?*" in wt-status.c
>> could be made fast enough.
>>
>> Should we try to implement and profile this
>> or do we know it will be slow beforehand?
>
> I think I've seen a patch to do something like that, soon after Shawn
> announced his repo tool.

The best I could find is your patch from October 25th 2008
which implements:
       $ ./git-shortstatus
       M     Makefile
       R100  COPYING -> RENAMING
           M builtin-commit.c
           M builtin-revert.c
           M builtin.h
           M git.c
           M wt-status.c
           M wt-status.h

Is this what you meant?

Re: [PATCH] bash: offer to show (un)staged changes

[Tuncer Ayaz]

On Mon, Feb 2, 2009 at 1:50 AM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
> On Mon, Feb 2, 2009 at 12:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Tuncer Ayaz <tuncer.ayaz@gmail.com> writes:
>>
>>> On Mon, Jan 19, 2009 at 6:29 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
>>>> Junio C Hamano <gitster@pobox.com> wrote:
>>>>> Thomas Rast <trast@student.ethz.ch> writes:
>>>>>
>>>>> > +           if test ! -z "$GIT_PS1_EXPENSIVE"; then
>>>>> > +                   git update-index --refresh >/dev/null 2>&1 || w="*"
>>>>>
>>>>> This makes the feature unavailable for people who care about the stat
>>>>> dirtiness and explicitly set diff.autorefreshindex to false, doesn't it?
>>>>
>>>> Yup, and I'm one of those people who sets autorefresindex to false
>>>> in my ~/.gitconfig, usually before I even have user.{name,email} set.
>>>>
>>>> I do like the idea of what Thomas is trying to do here, but its
>>>> so bloody expensive to compute dirty state on every prompt in
>>>> some repositories that I'd shoot myself.  E.g. WebKit is huge,
>>>
>>> I've been thinking about this and wondered
>>> whether implementing "status --mini" or
>>> "status --short" which prints "+?*" in wt-status.c
>>> could be made fast enough.
>>>
>>> Should we try to implement and profile this
>>> or do we know it will be slow beforehand?
>>
>> I think I've seen a patch to do something like that, soon after Shawn
>> announced his repo tool.
>
> The best I could find is your patch from October 25th 2008
> which implements:
>       $ ./git-shortstatus
>       M     Makefile
>       R100  COPYING -> RENAMING
>           M builtin-commit.c
>           M builtin-revert.c
>           M builtin.h
>           M git.c
>           M wt-status.c
>           M wt-status.h
>
> Is this what you meant?

I am confident that this is the patch Junio referred to
and as time permits I will give it a try.

[PATCH v3] bash: offer to show (un)staged changes

[Thomas Rast]

Add a bit of code to __git_ps1 that lets it append '*' to the branch
name if there are any unstaged changes, and '+' if there are any
staged changes.

Since this is a rather expensive operation and will force a lot of
data into the cache whenever you first enter a repository, you have to
enable it manually by setting GIT_PS1_SHOWDIRTYSTATE to a nonempty
value.  The configuration variable bash.showDirtyState can then be
used to disable it again for some repositories.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Shawn O. Pearce wrote:
> But I'm a bit worried about the config --bool test in the prompt.
> Its a new fork+exec we weren't doing before.  I wonder if we should
> use a shell variable to consider whether or not this should even
> be executed and try to shortcut out if not.

Ok, why not.  I changed the default of bash.showDirtyState to true
since the user already opts in via GIT_PS1_SHOWDIRTYSTATE.


 contrib/completion/git-completion.bash |   26 ++++++++++++++++++++++++--
 1 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f8b845a..13cae8d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -34,6 +34,12 @@
 #       are currently in a git repository.  The %s token will be
 #       the name of the current branch.
 #
+#	In addition, if you set GIT_PS1_SHOWDIRTYSTATE to a nonempty
+#	value, unstaged (*) and staged (+) changes will be shown next
+#	to the branch name.  You can configure this per-repository
+#	with the bash.showDirtyState variable, which defaults to true
+#	once GIT_PS1_SHOWDIRTYSTATE is enabled.
+#
 # To submit patches:
 #
 #    *) Read Documentation/SubmittingPatches
@@ -116,10 +122,26 @@ __git_ps1 ()
 			fi
 		fi
 
+		local w
+		local i
+
+		if test -n "$GIT_PS1_SHOWDIRTYSTATE"; then
+			if test "$(git config --bool bash.showDirtyState)" != "false"; then
+				git diff --no-ext-diff --ignore-submodules \
+					--quiet --exit-code || w="*"
+				if git rev-parse --quiet --verify HEAD >/dev/null; then
+					git diff-index --cached --quiet \
+						--ignore-submodules HEAD -- || i="+"
+				else
+					i="#"
+				fi
+			fi
+		fi
+
 		if [ -n "${1-}" ]; then
-			printf "$1" "${b##refs/heads/}$r"
+			printf "$1" "${b##refs/heads/}$w$i$r"
 		else
-			printf " (%s)" "${b##refs/heads/}$r"
+			printf " (%s)" "${b##refs/heads/}$w$i$r"
 		fi
 	fi
 }
-- 
tg: (7bbd8d6..) t/ps1-dirty-state (depends on: origin/master)

Re: [PATCH v3] bash: offer to show (un)staged changes

[Shawn O. Pearce]

Thomas Rast <trast@student.ethz.ch> wrote:
> Add a bit of code to __git_ps1 that lets it append '*' to the branch
> name if there are any unstaged changes, and '+' if there are any
> staged changes.
> 
> Since this is a rather expensive operation and will force a lot of
> data into the cache whenever you first enter a repository, you have to
> enable it manually by setting GIT_PS1_SHOWDIRTYSTATE to a nonempty
> value.  The configuration variable bash.showDirtyState can then be
> used to disable it again for some repositories.
> 
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>

Acked-by: Shawn O. Pearce <spearce@spearce.org>

> Shawn O. Pearce wrote:
> > But I'm a bit worried about the config --bool test in the prompt.
> > Its a new fork+exec we weren't doing before.  I wonder if we should
> > use a shell variable to consider whether or not this should even
> > be executed and try to shortcut out if not.
> 
> Ok, why not.  I changed the default of bash.showDirtyState to true
> since the user already opts in via GIT_PS1_SHOWDIRTYSTATE.

Yea, that seems right.

-- 
Shawn.