threads / patch / 17231

patchbash: offer to show (un)staged changes

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

## tl;dr

22 messages between Jan 18, 2009 and Feb 3, 2009. Diffs are folded; open one to read it.

replies: 21people: 7as markdown or json

Thomas Rast· Jan 18, 2009, 00:56 UTC · lore

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(-)
Show changes to contrib/completion/git-completion.bash +19 −2
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)
Junio C Hamano· Jan 18, 2009, 01:53 UTC · re: Thomas Rast · lore

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

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?

Thomas Rast· Jan 18, 2009, 02:06 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 7 quoted lines
> 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
Shawn O. Pearce· Jan 19, 2009, 17:29 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> 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.
Martin Langhoff· Jan 19, 2009, 18:00 UTC · re: Shawn O. Pearce · lore

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

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
Shawn O. Pearce· Jan 19, 2009, 18:11 UTC · re: Martin Langhoff · lore

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

Martin Langhoff <martin.langhoff@gmail.com> wrote:
Show 14 quoted lines
> 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.
Mike Hommey· Jan 19, 2009, 18:28 UTC · re: Shawn O. Pearce · lore

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

On Mon, Jan 19, 2009 at 10:11:58AM -0800, Shawn O. Pearce wrote:
Show 21 quoted lines
> 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
Martin Langhoff· Jan 19, 2009, 18:42 UTC · re: Shawn O. Pearce · lore

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

On Mon, Jan 19, 2009 at 1:11 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
Show 5 quoted lines
> *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
Boyd Stephen Smith Jr.· Jan 19, 2009, 19:06 UTC · re: Martin Langhoff · lore

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

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:
Show 7 quoted lines
>> ...  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/                      \_/     
Martin Langhoff· Jan 19, 2009, 19:12 UTC · re: Boyd Stephen Smith Jr. · lore

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

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
Boyd Stephen Smith Jr.· Jan 19, 2009, 19:01 UTC · re: Shawn O. Pearce · lore

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

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/                      \_/     
Thomas Rast· Jan 19, 2009, 21:38 UTC · re: Shawn O. Pearce · lore

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

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:
Show 6 quoted lines
> 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.

Show 10 quoted lines
> 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(-)
Show changes to contrib/completion/git-completion.bash +20 −2
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)
Thomas Rast· Feb 1, 2009, 22:13 UTC · re: Thomas Rast · lore

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

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(-)
Show changes to contrib/completion/git-completion.bash +20 −2
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)
Shawn O. Pearce· Feb 1, 2009, 22:29 UTC · re: Thomas Rast · lore

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

Thomas Rast <trast@student.ethz.ch> wrote:
Show 12 quoted lines
> 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.

 
Show 42 quoted lines
> 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.
Thomas Rast· Feb 3, 2009, 09:20 UTC · re: Shawn O. Pearce · lore

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

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(-)
Show changes to contrib/completion/git-completion.bash +24 −2
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)
Shawn O. Pearce· Feb 3, 2009, 18:11 UTC · re: Thomas Rast · lore

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

Thomas Rast <trast@student.ethz.ch> wrote:
Show 11 quoted lines
> 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>
Show 8 quoted lines
> 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.
Tuncer Ayaz· Feb 1, 2009, 22:48 UTC · re: Shawn O. Pearce · lore

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

On Mon, Jan 19, 2009 at 6:29 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
Show 15 quoted lines
> 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.

Show 18 quoted lines
> 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
>
Junio C Hamano· Feb 1, 2009, 23:43 UTC · re: Tuncer Ayaz · lore

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

Tuncer Ayaz <tuncer.ayaz@gmail.com> writes:
Show 24 quoted lines
> 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.

Tuncer Ayaz· Feb 2, 2009, 00:50 UTC · re: Junio C Hamano · lore

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

On Mon, Feb 2, 2009 at 12:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 29 quoted lines
> 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?
Tuncer Ayaz· Feb 2, 2009, 19:31 UTC · re: Tuncer Ayaz · lore

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

On Mon, Feb 2, 2009 at 1:50 AM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
Show 44 quoted lines
> 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.

Junio C Hamano· Jan 18, 2009, 02:13 UTC · re: Thomas Rast · lore

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

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?

Thomas Rast· Jan 18, 2009, 02:32 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 11 quoted lines
> 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

← back to recent threads