threads / patch / 34187

v2, 13 partsbash prompt speedup

Subject: [PATCH v2 00/13] bash prompt speedup

## tl;dr

22 messages between Jun 18, 2013 and Jun 18, 2013. Diffs are folded; open one to read it.

replies: 21people: 5as markdown or json

SZEDER Gábor· Jun 18, 2013, 02:16 UTC · lore
From: SZEDER Gábor <szeder@ira.uka.de>
Hi,

displaying the git-specific bash prompt on Windows/MinGW takes quite long, long enough to be noticeable. This is mainly caused by the numerous fork()s and exec()s to create subshells and run git or other commands, which are rather expensive on Windows.

This patch series eliminates many command substitutions and commands in __git_ps1() from top to bottom by replacing them with bash builtins or consolidating them. A few timing results are shown in the log message of patch 10.

SZEDER Gábor (13):
  bash prompt: fix redirection coding style in tests
  bash prompt: fix here document indentation in interactive rebase test
  completion, bash prompt: move __gitdir() tests to completion test
    suite
  bash prompt: add a test for symbolic link symbolic refs
  bash prompt: return early from __git_ps1() when not in a git
    repository
  bash prompt: run 'git rev-parse --git-dir' directly instead of
    __gitdir()
  bash prompt: use bash builtins to find out rebase state
  bash prompt: use bash builtins to find out current branch
  bash prompt: use bash builtins to get detached HEAD abbrev. object
    name
  bash prompt: combine 'git rev-parse' executions
  bash prompt: use bash builtins to check stash state
  bash prompt: avoid command substitution when checking for untracked
    files
  bash prompt: avoid command substitution when finalizing gitstring
 contrib/completion/git-completion.bash |   2 -
 contrib/completion/git-prompt.sh       | 223 ++++++++++++-----------
 t/t9902-completion.sh                  | 134 ++++++++++++++
 t/t9903-bash-prompt.sh                 | 319 +++++++++++----------------------
 4 files changed, 345 insertions(+), 333 deletions(-)
-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:16 UTC · re: SZEDER Gábor · lore

[PATCH v2 01/13] bash prompt: fix redirection coding style in tests

From: SZEDER Gábor <szeder@ira.uka.de>

Use '>file' instead of '> file', in accordance with the coding guidelines.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 t/t9903-bash-prompt.sh | 232 ++++++++++++++++++++++++-------------------------
 1 file changed, 116 insertions(+), 116 deletions(-)
Show changes to t/t9903-bash-prompt.sh +116 −116
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 15521cc4..7c7f8b97 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -14,98 +14,98 @@ actual="$TRASH_DIRECTORY/actual"
 test_expect_success 'setup for prompt tests' '
 	mkdir -p subdir/subsubdir &&
 	git init otherrepo &&
-	echo 1 > file &&
+	echo 1 >file &&
 	git add file &&
 	test_tick &&
 	git commit -m initial &&
 	git tag -a -m msg1 t1 &&
 	git checkout -b b1 &&
-	echo 2 > file &&
+	echo 2 >file &&
 	git commit -m "second b1" file &&
-	echo 3 > file &&
+	echo 3 >file &&
 	git commit -m "third b1" file &&
 	git tag -a -m msg2 t2 &&
 	git checkout -b b2 master &&
-	echo 0 > file &&
+	echo 0 >file &&
 	git commit -m "second b2" file &&
-	echo 00 > file &&
+	echo 00 >file &&
 	git commit -m "another b2" file &&
-	echo 000 > file &&
+	echo 000 >file &&
 	git commit -m "yet another b2" file &&
 	git checkout master
 '
 
 test_expect_success 'gitdir - from command line (through $__git_dir)' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
 	(
 		__git_dir="$TRASH_DIRECTORY/otherrepo/.git" &&
-		__gitdir > "$actual"
+		__gitdir >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'gitdir - repo as argument' '
-	echo "otherrepo/.git" > expected &&
-	__gitdir "otherrepo" > "$actual" &&
+	echo "otherrepo/.git" >expected &&
+	__gitdir "otherrepo" >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'gitdir - remote as argument' '
-	echo "remote" > expected &&
-	__gitdir "remote" > "$actual" &&
+	echo "remote" >expected &&
+	__gitdir "remote" >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'gitdir - .git directory in cwd' '
-	echo ".git" > expected &&
-	__gitdir > "$actual" &&
+	echo ".git" >expected &&
+	__gitdir >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'gitdir - .git directory in parent' '
-	echo "$(pwd -P)/.git" > expected &&
+	echo "$(pwd -P)/.git" >expected &&
 	(
 		cd subdir/subsubdir &&
-		__gitdir > "$actual"
+		__gitdir >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'gitdir - cwd is a .git directory' '
-	echo "." > expected &&
+	echo "." >expected &&
 	(
 		cd .git &&
-		__gitdir > "$actual"
+		__gitdir >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'gitdir - parent is a .git directory' '
-	echo "$(pwd -P)/.git" > expected &&
+	echo "$(pwd -P)/.git" >expected &&
 	(
 		cd .git/refs/heads &&
-		__gitdir > "$actual"
+		__gitdir >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'gitdir - $GIT_DIR set while .git directory in cwd' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
 	(
 		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
 		export GIT_DIR &&
-		__gitdir > "$actual"
+		__gitdir >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'gitdir - $GIT_DIR set while .git directory in parent' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" > expected &&
+	echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
 	(
 		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
 		export GIT_DIR &&
 		cd subdir &&
-		__gitdir > "$actual"
+		__gitdir >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
@@ -119,36 +119,36 @@ test_expect_success 'gitdir - non-existing $GIT_DIR' '
 '
 
 test_expect_success 'gitdir - gitfile in cwd' '
-	echo "$(pwd -P)/otherrepo/.git" > expected &&
-	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" > subdir/.git &&
+	echo "$(pwd -P)/otherrepo/.git" >expected &&
+	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" >subdir/.git &&
 	test_when_finished "rm -f subdir/.git" &&
 	(
 		cd subdir &&
-		__gitdir > "$actual"
+		__gitdir >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'gitdir - gitfile in parent' '
-	echo "$(pwd -P)/otherrepo/.git" > expected &&
-	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" > subdir/.git &&
+	echo "$(pwd -P)/otherrepo/.git" >expected &&
+	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" >subdir/.git &&
 	test_when_finished "rm -f subdir/.git" &&
 	(
 		cd subdir/subsubdir &&
-		__gitdir > "$actual"
+		__gitdir >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success SYMLINKS 'gitdir - resulting path avoids symlinks' '
-	echo "$(pwd -P)/otherrepo/.git" > expected &&
+	echo "$(pwd -P)/otherrepo/.git" >expected &&
 	mkdir otherrepo/dir &&
 	test_when_finished "rm -rf otherrepo/dir" &&
 	ln -s otherrepo/dir link &&
 	test_when_finished "rm -f link" &&
 	(
 		cd link &&
-		__gitdir > "$actual"
+		__gitdir >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
@@ -163,96 +163,96 @@ test_expect_success 'gitdir - not a git repository' '
 '
 
 test_expect_success 'prompt - branch name' '
-	printf " (master)" > expected &&
-	__git_ps1 > "$actual" &&
+	printf " (master)" >expected &&
+	__git_ps1 >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - detached head' '
-	printf " ((%s...))" $(git log -1 --format="%h" b1^) > expected &&
+	printf " ((%s...))" $(git log -1 --format="%h" b1^) >expected &&
 	git checkout b1^ &&
 	test_when_finished "git checkout master" &&
-	__git_ps1 > "$actual" &&
+	__git_ps1 >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - describe detached head - contains' '
-	printf " ((t2~1))" > expected &&
+	printf " ((t2~1))" >expected &&
 	git checkout b1^ &&
 	test_when_finished "git checkout master" &&
 	(
 		GIT_PS1_DESCRIBE_STYLE=contains &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - describe detached head - branch' '
-	printf " ((b1~1))" > expected &&
+	printf " ((b1~1))" >expected &&
 	git checkout b1^ &&
 	test_when_finished "git checkout master" &&
 	(
 		GIT_PS1_DESCRIBE_STYLE=branch &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - describe detached head - describe' '
-	printf " ((t1-1-g%s))" $(git log -1 --format="%h" b1^) > expected &&
+	printf " ((t1-1-g%s))" $(git log -1 --format="%h" b1^) >expected &&
 	git checkout b1^ &&
 	test_when_finished "git checkout master" &&
 	(
 		GIT_PS1_DESCRIBE_STYLE=describe &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - describe detached head - default' '
-	printf " ((t2))" > expected &&
+	printf " ((t2))" >expected &&
 	git checkout --detach b1 &&
 	test_when_finished "git checkout master" &&
-	__git_ps1 > "$actual" &&
+	__git_ps1 >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - inside .git directory' '
-	printf " (GIT_DIR!)" > expected &&
+	printf " (GIT_DIR!)" >expected &&
 	(
 		cd .git &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - deep inside .git directory' '
-	printf " (GIT_DIR!)" > expected &&
+	printf " (GIT_DIR!)" >expected &&
 	(
 		cd .git/refs/heads &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - inside bare repository' '
-	printf " (BARE:master)" > expected &&
+	printf " (BARE:master)" >expected &&
 	git init --bare bare.git &&
 	test_when_finished "rm -rf bare.git" &&
 	(
 		cd bare.git &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - interactive rebase' '
-	printf " (b1|REBASE-i 2/3)" > expected
+	printf " (b1|REBASE-i 2/3)" >expected
 	echo "#!$SHELL_PATH" >fake_editor.sh &&
 	cat >>fake_editor.sh <<\EOF &&
-echo "exec echo" > "$1"
-echo "edit $(git log -1 --format="%h")" >> "$1"
-echo "exec echo" >> "$1"
+echo "exec echo" >"$1"
+echo "edit $(git log -1 --format="%h")" >>"$1"
+echo "exec echo" >>"$1"
 EOF
 	test_when_finished "rm -f fake_editor.sh" &&
 	chmod a+x fake_editor.sh &&
@@ -261,277 +261,277 @@ EOF
 	test_when_finished "git checkout master" &&
 	git rebase -i HEAD^ &&
 	test_when_finished "git rebase --abort"
-	__git_ps1 > "$actual" &&
+	__git_ps1 >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - rebase merge' '
-	printf " (b2|REBASE-m 1/3)" > expected &&
+	printf " (b2|REBASE-m 1/3)" >expected &&
 	git checkout b2 &&
 	test_when_finished "git checkout master" &&
 	test_must_fail git rebase --merge b1 b2 &&
 	test_when_finished "git rebase --abort" &&
-	__git_ps1 > "$actual" &&
+	__git_ps1 >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - rebase' '
-	printf " (b2|REBASE 1/3)" > expected &&
+	printf " (b2|REBASE 1/3)" >expected &&
 	git checkout b2 &&
 	test_when_finished "git checkout master" &&
 	test_must_fail git rebase b1 b2 &&
 	test_when_finished "git rebase --abort" &&
-	__git_ps1 > "$actual" &&
+	__git_ps1 >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - merge' '
-	printf " (b1|MERGING)" > expected &&
+	printf " (b1|MERGING)" >expected &&
 	git checkout b1 &&
 	test_when_finished "git checkout master" &&
 	test_must_fail git merge b2 &&
 	test_when_finished "git reset --hard" &&
-	__git_ps1 > "$actual" &&
+	__git_ps1 >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - cherry-pick' '
-	printf " (master|CHERRY-PICKING)" > expected &&
+	printf " (master|CHERRY-PICKING)" >expected &&
 	test_must_fail git cherry-pick b1 &&
 	test_when_finished "git reset --hard" &&
-	__git_ps1 > "$actual" &&
+	__git_ps1 >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - bisect' '
-	printf " (master|BISECTING)" > expected &&
+	printf " (master|BISECTING)" >expected &&
 	git bisect start &&
 	test_when_finished "git bisect reset" &&
-	__git_ps1 > "$actual" &&
+	__git_ps1 >"$actual" &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - dirty status indicator - clean' '
-	printf " (master)" > expected &&
+	printf " (master)" >expected &&
 	(
 		GIT_PS1_SHOWDIRTYSTATE=y &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - dirty status indicator - dirty worktree' '
-	printf " (master *)" > expected &&
-	echo "dirty" > file &&
+	printf " (master *)" >expected &&
+	echo "dirty" >file &&
 	test_when_finished "git reset --hard" &&
 	(
 		GIT_PS1_SHOWDIRTYSTATE=y &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - dirty status indicator - dirty index' '
-	printf " (master +)" > expected &&
-	echo "dirty" > file &&
+	printf " (master +)" >expected &&
+	echo "dirty" >file &&
 	test_when_finished "git reset --hard" &&
 	git add -u &&
 	(
 		GIT_PS1_SHOWDIRTYSTATE=y &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - dirty status indicator - dirty index and worktree' '
-	printf " (master *+)" > expected &&
-	echo "dirty index" > file &&
+	printf " (master *+)" >expected &&
+	echo "dirty index" >file &&
 	test_when_finished "git reset --hard" &&
 	git add -u &&
-	echo "dirty worktree" > file &&
+	echo "dirty worktree" >file &&
 	(
 		GIT_PS1_SHOWDIRTYSTATE=y &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - dirty status indicator - before root commit' '
-	printf " (master #)" > expected &&
+	printf " (master #)" >expected &&
 	(
 		GIT_PS1_SHOWDIRTYSTATE=y &&
 		cd otherrepo &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - dirty status indicator - shell variable unset with config disabled' '
-	printf " (master)" > expected &&
-	echo "dirty" > file &&
+	printf " (master)" >expected &&
+	echo "dirty" >file &&
 	test_when_finished "git reset --hard" &&
 	test_config bash.showDirtyState false &&
 	(
 		sane_unset GIT_PS1_SHOWDIRTYSTATE &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - dirty status indicator - shell variable unset with config enabled' '
-	printf " (master)" > expected &&
-	echo "dirty" > file &&
+	printf " (master)" >expected &&
+	echo "dirty" >file &&
 	test_when_finished "git reset --hard" &&
 	test_config bash.showDirtyState true &&
 	(
 		sane_unset GIT_PS1_SHOWDIRTYSTATE &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - dirty status indicator - shell variable set with config disabled' '
-	printf " (master)" > expected &&
-	echo "dirty" > file &&
+	printf " (master)" >expected &&
+	echo "dirty" >file &&
 	test_when_finished "git reset --hard" &&
 	test_config bash.showDirtyState false &&
 	(
 		GIT_PS1_SHOWDIRTYSTATE=y &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - dirty status indicator - shell variable set with config enabled' '
-	printf " (master *)" > expected &&
-	echo "dirty" > file &&
+	printf " (master *)" >expected &&
+	echo "dirty" >file &&
 	test_when_finished "git reset --hard" &&
 	test_config bash.showDirtyState true &&
 	(
 		GIT_PS1_SHOWDIRTYSTATE=y &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - dirty status indicator - not shown inside .git directory' '
-	printf " (GIT_DIR!)" > expected &&
-	echo "dirty" > file &&
+	printf " (GIT_DIR!)" >expected &&
+	echo "dirty" >file &&
 	test_when_finished "git reset --hard" &&
 	(
 		GIT_PS1_SHOWDIRTYSTATE=y &&
 		cd .git &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - stash status indicator - no stash' '
-	printf " (master)" > expected &&
+	printf " (master)" >expected &&
 	(
 		GIT_PS1_SHOWSTASHSTATE=y &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - stash status indicator - stash' '
-	printf " (master $)" > expected &&
+	printf " (master $)" >expected &&
 	echo 2 >file &&
 	git stash &&
 	test_when_finished "git stash drop" &&
 	(
 		GIT_PS1_SHOWSTASHSTATE=y &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - stash status indicator - not shown inside .git directory' '
-	printf " (GIT_DIR!)" > expected &&
+	printf " (GIT_DIR!)" >expected &&
 	echo 2 >file &&
 	git stash &&
 	test_when_finished "git stash drop" &&
 	(
 		GIT_PS1_SHOWSTASHSTATE=y &&
 		cd .git &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - untracked files status indicator - no untracked files' '
-	printf " (master)" > expected &&
+	printf " (master)" >expected &&
 	(
 		GIT_PS1_SHOWUNTRACKEDFILES=y &&
 		cd otherrepo &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - untracked files status indicator - untracked files' '
-	printf " (master %%)" > expected &&
+	printf " (master %%)" >expected &&
 	(
 		GIT_PS1_SHOWUNTRACKEDFILES=y &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - untracked files status indicator - shell variable unset with config disabled' '
-	printf " (master)" > expected &&
+	printf " (master)" >expected &&
 	test_config bash.showUntrackedFiles false &&
 	(
 		sane_unset GIT_PS1_SHOWUNTRACKEDFILES &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - untracked files status indicator - shell variable unset with config enabled' '
-	printf " (master)" > expected &&
+	printf " (master)" >expected &&
 	test_config bash.showUntrackedFiles true &&
 	(
 		sane_unset GIT_PS1_SHOWUNTRACKEDFILES &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - untracked files status indicator - shell variable set with config disabled' '
-	printf " (master)" > expected &&
+	printf " (master)" >expected &&
 	test_config bash.showUntrackedFiles false &&
 	(
 		GIT_PS1_SHOWUNTRACKEDFILES=y &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - untracked files status indicator - shell variable set with config enabled' '
-	printf " (master %%)" > expected &&
+	printf " (master %%)" >expected &&
 	test_config bash.showUntrackedFiles true &&
 	(
 		GIT_PS1_SHOWUNTRACKEDFILES=y &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - untracked files status indicator - not shown inside .git directory' '
-	printf " (GIT_DIR!)" > expected &&
+	printf " (GIT_DIR!)" >expected &&
 	(
 		GIT_PS1_SHOWUNTRACKEDFILES=y &&
 		cd .git &&
-		__git_ps1 > "$actual"
+		__git_ps1 >"$actual"
 	) &&
 	test_cmp expected "$actual"
 '
 
 test_expect_success 'prompt - format string starting with dash' '
-	printf -- "-master" > expected &&
-	__git_ps1 "-%s" > "$actual" &&
+	printf -- "-master" >expected &&
+	__git_ps1 "-%s" >"$actual" &&
 	test_cmp expected "$actual"
 '
 

-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:16 UTC · re: SZEDER Gábor · lore

[PATCH v2 02/13] bash prompt: fix here document indentation in interactive rebase test

From: SZEDER Gábor <szeder@ira.uka.de>
Also move the shebang line into the here document.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 t/t9903-bash-prompt.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
Show changes to t/t9903-bash-prompt.sh +6 −6
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 7c7f8b97..b0af5d5f 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -248,12 +248,12 @@ test_expect_success 'prompt - inside bare repository' '
 
 test_expect_success 'prompt - interactive rebase' '
 	printf " (b1|REBASE-i 2/3)" >expected
-	echo "#!$SHELL_PATH" >fake_editor.sh &&
-	cat >>fake_editor.sh <<\EOF &&
-echo "exec echo" >"$1"
-echo "edit $(git log -1 --format="%h")" >>"$1"
-echo "exec echo" >>"$1"
-EOF
+	cat >fake_editor.sh <<-EOF &&
+		#!$SHELL_PATH
+		echo "exec echo" >"\$1"
+		echo "edit \$(git log -1 --format="%h")" >>"\$1"
+		echo "exec echo" >>"\$1"
+	EOF
 	test_when_finished "rm -f fake_editor.sh" &&
 	chmod a+x fake_editor.sh &&
 	test_set_editor "$TRASH_DIRECTORY/fake_editor.sh" &&

-- 
1.8.3.1.487.g8f4672d
Jeff King· Jun 18, 2013, 05:54 UTC · re: SZEDER Gábor · lore

Re: [PATCH v2 02/13] bash prompt: fix here document indentation in interactive rebase test

On Tue, Jun 18, 2013 at 04:16:55AM +0200, SZEDER Gábor wrote:
Show 12 quoted lines
> -	echo "#!$SHELL_PATH" >fake_editor.sh &&
> -	cat >>fake_editor.sh <<\EOF &&
> -echo "exec echo" >"$1"
> -echo "edit $(git log -1 --format="%h")" >>"$1"
> -echo "exec echo" >>"$1"
> -EOF
> +	cat >fake_editor.sh <<-EOF &&
> +		#!$SHELL_PATH
> +		echo "exec echo" >"\$1"
> +		echo "edit \$(git log -1 --format="%h")" >>"\$1"
> +		echo "exec echo" >>"\$1"
> +	EOF

These days we the "write_script" helper, so I think you can write this even nicer as:

Show changes to t/t9903-bash-prompt.sh +5 −8
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 9e51263..91da313 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -248,14 +248,12 @@ EOF
 
 test_expect_success 'prompt - interactive rebase' '
 	printf " (b1|REBASE-i 2/3)" >expected
-	echo "#!$SHELL_PATH" >fake_editor.sh &&
-	cat >>fake_editor.sh <<\EOF &&
-echo "exec echo" >"$1"
-echo "edit $(git log -1 --format="%h")" >>"$1"
-echo "exec echo" >>"$1"
-EOF
+	write_script fake_editor.sh <<-\EOF &&
+	echo "exec echo" >"$1"
+	echo "edit $(git log -1 --format="%h")" >>"$1"
+	echo "exec echo" >>"$1"
+	EOF
 	test_when_finished "rm -f fake_editor.sh" &&
-	chmod a+x fake_editor.sh &&
 	test_set_editor "$TRASH_DIRECTORY/fake_editor.sh" &&
 	git checkout b1 &&
 	test_when_finished "git checkout master" &&

-Peff
SZEDER Gábor· Jun 18, 2013, 02:16 UTC · re: SZEDER Gábor · lore

[PATCH v2 03/13] completion, bash prompt: move __gitdir() tests to completion test suite

From: SZEDER Gábor <szeder@ira.uka.de>

Currently __gitdir() is duplicated in the git completion and prompt scripts, while its tests are in the prompt test suite. This patch series is about to change __git_ps1() in a way that it won't need __gitdir() anymore and __gitdir() will be removed from the prompt script.

So move all __gitdir() test from the prompt test suite over to the completion test suite. Update the setup tests so that they perform only those steps that are necessary for each test suite.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 t/t9902-completion.sh  | 134 +++++++++++++++++++++++++++++++++++++++++++++++++
 t/t9903-bash-prompt.sh | 128 ----------------------------------------------
 2 files changed, 134 insertions(+), 128 deletions(-)
Show changes to 2 files +134 −128

t/t9902-completion.sh, t/t9903-bash-prompt.sh

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 81a1657e..5469dee8 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -122,6 +122,140 @@ test_gitcomp_nl ()
 
 invalid_variable_name='${foo.bar}'
 
+actual="$TRASH_DIRECTORY/actual"
+
+test_expect_success 'setup for __gitdir tests' '
+	mkdir -p subdir/subsubdir &&
+	git init otherrepo
+'
+
+test_expect_success '__gitdir - from command line (through $__git_dir)' '
+	echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
+	(
+		__git_dir="$TRASH_DIRECTORY/otherrepo/.git" &&
+		__gitdir >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - repo as argument' '
+	echo "otherrepo/.git" >expected &&
+	__gitdir "otherrepo" >"$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - remote as argument' '
+	echo "remote" >expected &&
+	__gitdir "remote" >"$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - .git directory in cwd' '
+	echo ".git" >expected &&
+	__gitdir >"$actual" &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - .git directory in parent' '
+	echo "$(pwd -P)/.git" >expected &&
+	(
+		cd subdir/subsubdir &&
+		__gitdir >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - cwd is a .git directory' '
+	echo "." >expected &&
+	(
+		cd .git &&
+		__gitdir >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - parent is a .git directory' '
+	echo "$(pwd -P)/.git" >expected &&
+	(
+		cd .git/refs/heads &&
+		__gitdir >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - $GIT_DIR set while .git directory in cwd' '
+	echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
+	(
+		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
+		export GIT_DIR &&
+		__gitdir >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - $GIT_DIR set while .git directory in parent' '
+	echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
+	(
+		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
+		export GIT_DIR &&
+		cd subdir &&
+		__gitdir >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - non-existing $GIT_DIR' '
+	(
+		GIT_DIR="$TRASH_DIRECTORY/non-existing" &&
+		export GIT_DIR &&
+		test_must_fail __gitdir
+	)
+'
+
+test_expect_success '__gitdir - gitfile in cwd' '
+	echo "$(pwd -P)/otherrepo/.git" >expected &&
+	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" >subdir/.git &&
+	test_when_finished "rm -f subdir/.git" &&
+	(
+		cd subdir &&
+		__gitdir >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - gitfile in parent' '
+	echo "$(pwd -P)/otherrepo/.git" >expected &&
+	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" >subdir/.git &&
+	test_when_finished "rm -f subdir/.git" &&
+	(
+		cd subdir/subsubdir &&
+		__gitdir >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success SYMLINKS '__gitdir - resulting path avoids symlinks' '
+	echo "$(pwd -P)/otherrepo/.git" >expected &&
+	mkdir otherrepo/dir &&
+	test_when_finished "rm -rf otherrepo/dir" &&
+	ln -s otherrepo/dir link &&
+	test_when_finished "rm -f link" &&
+	(
+		cd link &&
+		__gitdir >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__gitdir - not a git repository' '
+	(
+		cd subdir/subsubdir &&
+		GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY" &&
+		export GIT_CEILING_DIRECTORIES &&
+		test_must_fail __gitdir
+	)
+'
+
 test_expect_success '__gitcomp - trailing space - options' '
 	test_gitcomp "--re" "--dry-run --reuse-message= --reedit-message=
 		--reset-author" <<-EOF
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index b0af5d5f..1047c664 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -12,7 +12,6 @@ test_description='test git-specific bash prompt functions'
 actual="$TRASH_DIRECTORY/actual"
 
 test_expect_success 'setup for prompt tests' '
-	mkdir -p subdir/subsubdir &&
 	git init otherrepo &&
 	echo 1 >file &&
 	git add file &&
@@ -35,133 +34,6 @@ test_expect_success 'setup for prompt tests' '
 	git checkout master
 '
 
-test_expect_success 'gitdir - from command line (through $__git_dir)' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
-	(
-		__git_dir="$TRASH_DIRECTORY/otherrepo/.git" &&
-		__gitdir >"$actual"
-	) &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success 'gitdir - repo as argument' '
-	echo "otherrepo/.git" >expected &&
-	__gitdir "otherrepo" >"$actual" &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success 'gitdir - remote as argument' '
-	echo "remote" >expected &&
-	__gitdir "remote" >"$actual" &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success 'gitdir - .git directory in cwd' '
-	echo ".git" >expected &&
-	__gitdir >"$actual" &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success 'gitdir - .git directory in parent' '
-	echo "$(pwd -P)/.git" >expected &&
-	(
-		cd subdir/subsubdir &&
-		__gitdir >"$actual"
-	) &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success 'gitdir - cwd is a .git directory' '
-	echo "." >expected &&
-	(
-		cd .git &&
-		__gitdir >"$actual"
-	) &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success 'gitdir - parent is a .git directory' '
-	echo "$(pwd -P)/.git" >expected &&
-	(
-		cd .git/refs/heads &&
-		__gitdir >"$actual"
-	) &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success 'gitdir - $GIT_DIR set while .git directory in cwd' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
-	(
-		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
-		export GIT_DIR &&
-		__gitdir >"$actual"
-	) &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success 'gitdir - $GIT_DIR set while .git directory in parent' '
-	echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
-	(
-		GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
-		export GIT_DIR &&
-		cd subdir &&
-		__gitdir >"$actual"
-	) &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success 'gitdir - non-existing $GIT_DIR' '
-	(
-		GIT_DIR="$TRASH_DIRECTORY/non-existing" &&
-		export GIT_DIR &&
-		test_must_fail __gitdir
-	)
-'
-
-test_expect_success 'gitdir - gitfile in cwd' '
-	echo "$(pwd -P)/otherrepo/.git" >expected &&
-	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" >subdir/.git &&
-	test_when_finished "rm -f subdir/.git" &&
-	(
-		cd subdir &&
-		__gitdir >"$actual"
-	) &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success 'gitdir - gitfile in parent' '
-	echo "$(pwd -P)/otherrepo/.git" >expected &&
-	echo "gitdir: $TRASH_DIRECTORY/otherrepo/.git" >subdir/.git &&
-	test_when_finished "rm -f subdir/.git" &&
-	(
-		cd subdir/subsubdir &&
-		__gitdir >"$actual"
-	) &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success SYMLINKS 'gitdir - resulting path avoids symlinks' '
-	echo "$(pwd -P)/otherrepo/.git" >expected &&
-	mkdir otherrepo/dir &&
-	test_when_finished "rm -rf otherrepo/dir" &&
-	ln -s otherrepo/dir link &&
-	test_when_finished "rm -f link" &&
-	(
-		cd link &&
-		__gitdir >"$actual"
-	) &&
-	test_cmp expected "$actual"
-'
-
-test_expect_success 'gitdir - not a git repository' '
-	(
-		cd subdir/subsubdir &&
-		GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY" &&
-		export GIT_CEILING_DIRECTORIES &&
-		test_must_fail __gitdir
-	)
-'
-
 test_expect_success 'prompt - branch name' '
 	printf " (master)" >expected &&
 	__git_ps1 >"$actual" &&

-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:16 UTC · re: SZEDER Gábor · lore

[PATCH v2 04/13] bash prompt: add a test for symbolic link symbolic refs

From: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 t/t9903-bash-prompt.sh | 9 +++++++++
 1 file changed, 9 insertions(+)
Show changes to t/t9903-bash-prompt.sh +9 −0
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 1047c664..48460ef6 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -40,6 +40,15 @@ test_expect_success 'prompt - branch name' '
 	test_cmp expected "$actual"
 '
 
+test_expect_success SYMLINKS 'prompt - branch name - symlink symref' '
+	printf " (master)" >expected &&
+	test_when_finished "git checkout master" &&
+	test_config core.preferSymlinkRefs true &&
+	git checkout master &&
+	__git_ps1 >"$actual" &&
+	test_cmp expected "$actual"
+'
+
 test_expect_success 'prompt - detached head' '
 	printf " ((%s...))" $(git log -1 --format="%h" b1^) >expected &&
 	git checkout b1^ &&

-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:16 UTC · re: SZEDER Gábor · lore

[PATCH v2 05/13] bash prompt: return early from __git_ps1() when not in a git repository

From: SZEDER Gábor <szeder@ira.uka.de>
... to gain one level of indentation for the bulk of the function.

(The patch looks quite unreadable, you'd better check it with 'git diff -w'.)

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-prompt.sh | 201 ++++++++++++++++++++-------------------
 1 file changed, 101 insertions(+), 100 deletions(-)
Show changes to contrib/completion/git-prompt.sh +101 −100
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 07a6218d..96b496cc 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -341,121 +341,122 @@ __git_ps1 ()
 			#In PC mode PS1 always needs to be set
 			PS1="$ps1pc_start$ps1pc_end"
 		fi
+		return
+	fi
+
+	local r=""
+	local b=""
+	local step=""
+	local total=""
+	if [ -d "$g/rebase-merge" ]; then
+		b="$(cat "$g/rebase-merge/head-name" 2>/dev/null)"
+		step=$(cat "$g/rebase-merge/msgnum" 2>/dev/null)
+		total=$(cat "$g/rebase-merge/end" 2>/dev/null)
+		if [ -f "$g/rebase-merge/interactive" ]; then
+			r="|REBASE-i"
+		else
+			r="|REBASE-m"
+		fi
 	else
-		local r=""
-		local b=""
-		local step=""
-		local total=""
-		if [ -d "$g/rebase-merge" ]; then
-			b="$(cat "$g/rebase-merge/head-name" 2>/dev/null)"
-			step=$(cat "$g/rebase-merge/msgnum" 2>/dev/null)
-			total=$(cat "$g/rebase-merge/end" 2>/dev/null)
-			if [ -f "$g/rebase-merge/interactive" ]; then
-				r="|REBASE-i"
+		if [ -d "$g/rebase-apply" ]; then
+			step=$(cat "$g/rebase-apply/next" 2>/dev/null)
+			total=$(cat "$g/rebase-apply/last" 2>/dev/null)
+			if [ -f "$g/rebase-apply/rebasing" ]; then
+				b="$(cat "$g/rebase-apply/head-name" 2>/dev/null)"
+				r="|REBASE"
+			elif [ -f "$g/rebase-apply/applying" ]; then
+				r="|AM"
 			else
-				r="|REBASE-m"
-			fi
-		else
-			if [ -d "$g/rebase-apply" ]; then
-				step=$(cat "$g/rebase-apply/next" 2>/dev/null)
-				total=$(cat "$g/rebase-apply/last" 2>/dev/null)
-				if [ -f "$g/rebase-apply/rebasing" ]; then
-					b="$(cat "$g/rebase-apply/head-name" 2>/dev/null)"
-					r="|REBASE"
-				elif [ -f "$g/rebase-apply/applying" ]; then
-					r="|AM"
-				else
-					r="|AM/REBASE"
-				fi
-			elif [ -f "$g/MERGE_HEAD" ]; then
-				r="|MERGING"
-			elif [ -f "$g/CHERRY_PICK_HEAD" ]; then
-				r="|CHERRY-PICKING"
-			elif [ -f "$g/REVERT_HEAD" ]; then
-				r="|REVERTING"
-			elif [ -f "$g/BISECT_LOG" ]; then
-				r="|BISECTING"
+				r="|AM/REBASE"
 			fi
+		elif [ -f "$g/MERGE_HEAD" ]; then
+			r="|MERGING"
+		elif [ -f "$g/CHERRY_PICK_HEAD" ]; then
+			r="|CHERRY-PICKING"
+		elif [ -f "$g/REVERT_HEAD" ]; then
+			r="|REVERTING"
+		elif [ -f "$g/BISECT_LOG" ]; then
+			r="|BISECTING"
+		fi
 
-			test -n "$b" ||
-			b="$(git symbolic-ref HEAD 2>/dev/null)" || {
-				detached=yes
-				b="$(
-				case "${GIT_PS1_DESCRIBE_STYLE-}" in
-				(contains)
-					git describe --contains HEAD ;;
-				(branch)
-					git describe --contains --all HEAD ;;
-				(describe)
-					git describe HEAD ;;
-				(* | default)
-					git describe --tags --exact-match HEAD ;;
-				esac 2>/dev/null)" ||
+		test -n "$b" ||
+		b="$(git symbolic-ref HEAD 2>/dev/null)" || {
+			detached=yes
+			b="$(
+			case "${GIT_PS1_DESCRIBE_STYLE-}" in
+			(contains)
+				git describe --contains HEAD ;;
+			(branch)
+				git describe --contains --all HEAD ;;
+			(describe)
+				git describe HEAD ;;
+			(* | default)
+				git describe --tags --exact-match HEAD ;;
+			esac 2>/dev/null)" ||
 
-				b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
-				b="unknown"
-				b="($b)"
-			}
-		fi
+			b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
+			b="unknown"
+			b="($b)"
+		}
+	fi
 
-		if [ -n "$step" ] && [ -n "$total" ]; then
-			r="$r $step/$total"
-		fi
+	if [ -n "$step" ] && [ -n "$total" ]; then
+		r="$r $step/$total"
+	fi
 
-		local w=""
-		local i=""
-		local s=""
-		local u=""
-		local c=""
-		local p=""
+	local w=""
+	local i=""
+	local s=""
+	local u=""
+	local c=""
+	local p=""
 
-		if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
-			if [ "true" = "$(git rev-parse --is-bare-repository 2>/dev/null)" ]; then
-				c="BARE:"
+	if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
+		if [ "true" = "$(git rev-parse --is-bare-repository 2>/dev/null)" ]; then
+			c="BARE:"
+		else
+			b="GIT_DIR!"
+		fi
+	elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
+		if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ] &&
+		   [ "$(git config --bool bash.showDirtyState)" != "false" ]
+		then
+			git diff --no-ext-diff --quiet --exit-code || w="*"
+			if git rev-parse --quiet --verify HEAD >/dev/null; then
+				git diff-index --cached --quiet HEAD -- || i="+"
 			else
-				b="GIT_DIR!"
-			fi
-		elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
-			if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ] &&
-			   [ "$(git config --bool bash.showDirtyState)" != "false" ]
-			then
-				git diff --no-ext-diff --quiet --exit-code || w="*"
-				if git rev-parse --quiet --verify HEAD >/dev/null; then
-					git diff-index --cached --quiet HEAD -- || i="+"
-				else
-					i="#"
-				fi
-			fi
-			if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
-				git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
+				i="#"
 			fi
+		fi
+		if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
+			git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
+		fi
 
-			if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ] &&
-			   [ "$(git config --bool bash.showUntrackedFiles)" != "false" ] &&
-			   [ -n "$(git ls-files --others --exclude-standard)" ]
-			then
-				u="%${ZSH_VERSION+%}"
-			fi
+		if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ] &&
+		   [ "$(git config --bool bash.showUntrackedFiles)" != "false" ] &&
+		   [ -n "$(git ls-files --others --exclude-standard)" ]
+		then
+			u="%${ZSH_VERSION+%}"
+		fi
 
-			if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
-				__git_ps1_show_upstream
-			fi
+		if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
+			__git_ps1_show_upstream
 		fi
+	fi
 
-		local z="${GIT_PS1_STATESEPARATOR-" "}"
-		local f="$w$i$s$u"
-		if [ $pcmode = yes ]; then
-			local gitstring=
-			if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
-				__git_ps1_colorize_gitstring
-			else
-				gitstring="$c${b##refs/heads/}${f:+$z$f}$r$p"
-			fi
-			gitstring=$(printf -- "$printf_format" "$gitstring")
-			PS1="$ps1pc_start$gitstring$ps1pc_end"
+	local z="${GIT_PS1_STATESEPARATOR-" "}"
+	local f="$w$i$s$u"
+	if [ $pcmode = yes ]; then
+		local gitstring=
+		if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
+			__git_ps1_colorize_gitstring
 		else
-			# NO color option unless in PROMPT_COMMAND mode
-			printf -- "$printf_format" "$c${b##refs/heads/}${f:+$z$f}$r$p"
+			gitstring="$c${b##refs/heads/}${f:+$z$f}$r$p"
 		fi
+		gitstring=$(printf -- "$printf_format" "$gitstring")
+		PS1="$ps1pc_start$gitstring$ps1pc_end"
+	else
+		# NO color option unless in PROMPT_COMMAND mode
+		printf -- "$printf_format" "$c${b##refs/heads/}${f:+$z$f}$r$p"
 	fi
 }

-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:16 UTC · re: SZEDER Gábor · lore

[PATCH v2 06/13] bash prompt: run 'git rev-parse --git-dir' directly instead of __gitdir()

From: SZEDER Gábor <szeder@ira.uka.de>

__git_ps1() finds out the path to the repository by using the __gitdir() helper function. __gitdir() is basically just a wrapper around 'git rev-parse --git-dir', extended with support for recognizing a remote repository given as argument, to use the path given on the command line, and with a few shortcuts to recognize a git repository in cwd or at $GIT_DIR quickly without actually running 'git rev-parse'. However, the former two is only necessary for the completion script but makes no sense for the bash prompt, while the latter shortcuts are performance optimizations __git_ps1() can do without (they just avoid the overhead of fork()+exec()ing a git process).

Run 'git rev-parse --git-dir' directly in __git_ps1(), because it will allow this patch series to combine several $(git rev-parse ...) command substitutions in the main code path, and the overall performance benefit will far outweight the loss of those few shortcuts in __gitdir(). Furthermore, since __gitdir() is not needed anymore for the prompt, remove it from the prompt script finally eliminating its duplication between the prompt and completion scripts. Also remove the comment from the completion script warning about this code duplication.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |  2 --
 contrib/completion/git-prompt.sh       | 28 ++--------------------------
 2 files changed, 2 insertions(+), 28 deletions(-)
Show changes to 2 files +2 −28

contrib/completion/git-completion.bash, contrib/completion/git-prompt.sh

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 56c52c66..e0c8d6a1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -33,8 +33,6 @@ esac
 # returns location of .git repo
 __gitdir ()
 {
-	# Note: this function is duplicated in git-prompt.sh
-	# When updating it, make sure you update the other one to match.
 	if [ -z "${1-}" ]; then
 		if [ -n "${__git_dir-}" ]; then
 			echo "$__git_dir"
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 96b496cc..0fc1d317 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -80,30 +80,6 @@
 # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
 # the colored output of "git status -sb".
 
-# __gitdir accepts 0 or 1 arguments (i.e., location)
-# returns location of .git repo
-__gitdir ()
-{
-	# Note: this function is duplicated in git-completion.bash
-	# When updating it, make sure you update the other one to match.
-	if [ -z "${1-}" ]; then
-		if [ -n "${__git_dir-}" ]; then
-			echo "$__git_dir"
-		elif [ -n "${GIT_DIR-}" ]; then
-			test -d "${GIT_DIR-}" || return 1
-			echo "$GIT_DIR"
-		elif [ -d .git ]; then
-			echo .git
-		else
-			git rev-parse --git-dir 2>/dev/null
-		fi
-	elif [ -d "$1/.git" ]; then
-		echo "$1/.git"
-	else
-		echo "$1"
-	fi
-}
-
 # stores the divergence from upstream in $p
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
@@ -335,8 +311,8 @@ __git_ps1 ()
 		;;
 	esac
 
-	local g="$(__gitdir)"
-	if [ -z "$g" ]; then
+	local g=
+	if ! g="$(git rev-parse --git-dir 2>/dev/null)"; then
 		if [ $pcmode = yes ]; then
 			#In PC mode PS1 always needs to be set
 			PS1="$ps1pc_start$ps1pc_end"

-- 
1.8.3.1.487.g8f4672d
Eric Sunshine· Jun 18, 2013, 04:16 UTC · re: SZEDER Gábor · lore

Re: [PATCH v2 06/13] bash prompt: run 'git rev-parse --git-dir' directly instead of __gitdir()

On Mon, Jun 17, 2013 at 10:16 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 18 quoted lines
> From: SZEDER Gábor <szeder@ira.uka.de>
>
> __git_ps1() finds out the path to the repository by using the
> __gitdir() helper function.  __gitdir() is basically just a wrapper
> around 'git rev-parse --git-dir', extended with support for
> recognizing a remote repository given as argument, to use the path
> given on the command line, and with a few shortcuts to recognize a git
> repository in cwd or at $GIT_DIR quickly without actually running 'git
> rev-parse'.  However, the former two is only necessary for the
> completion script but makes no sense for the bash prompt, while the
> latter shortcuts are performance optimizations __git_ps1() can do
> without (they just avoid the overhead of fork()+exec()ing a git
> process).
>
> Run 'git rev-parse --git-dir' directly in __git_ps1(), because it will
> allow this patch series to combine several $(git rev-parse ...)
> command substitutions in the main code path, and the overall
> performance benefit will far outweight the loss of those few shortcuts
s/outweight/outweigh/
Show 7 quoted lines
> in __gitdir().  Furthermore, since __gitdir() is not needed anymore
> for the prompt, remove it from the prompt script finally eliminating
> its duplication between the prompt and completion scripts.  Also
> remove the comment from the completion script warning about this code
> duplication.
>
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
SZEDER Gábor· Jun 18, 2013, 02:17 UTC · re: SZEDER Gábor · lore

[PATCH v2 07/13] bash prompt: use bash builtins to find out rebase state

From: SZEDER Gábor <szeder@ira.uka.de>

During an ongoing interactive rebase __git_ps1() finds out the name of the rebased branch, the total number of patches and the number of the current patch by executing a '$(cat .git/rebase-merge/<FILE>)' command substitution for each. That is not quite the most efficient way to read single line single word files, because it imposes the overhead of fork()ing a subshell and fork()+exec()ing 'cat' several times.

Use the 'read' bash builtin instead to avoid those overheads.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-prompt.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
Show changes to contrib/completion/git-prompt.sh +6 −6
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 0fc1d317..26380787 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -325,9 +325,9 @@ __git_ps1 ()
 	local step=""
 	local total=""
 	if [ -d "$g/rebase-merge" ]; then
-		b="$(cat "$g/rebase-merge/head-name" 2>/dev/null)"
-		step=$(cat "$g/rebase-merge/msgnum" 2>/dev/null)
-		total=$(cat "$g/rebase-merge/end" 2>/dev/null)
+		read b 2>/dev/null <"$g/rebase-merge/head-name"
+		read step 2>/dev/null <"$g/rebase-merge/msgnum"
+		read total 2>/dev/null <"$g/rebase-merge/end"
 		if [ -f "$g/rebase-merge/interactive" ]; then
 			r="|REBASE-i"
 		else
@@ -335,10 +335,10 @@ __git_ps1 ()
 		fi
 	else
 		if [ -d "$g/rebase-apply" ]; then
-			step=$(cat "$g/rebase-apply/next" 2>/dev/null)
-			total=$(cat "$g/rebase-apply/last" 2>/dev/null)
+			read step 2>/dev/null <"$g/rebase-apply/next"
+			read total 2>/dev/null <"$g/rebase-apply/last"
 			if [ -f "$g/rebase-apply/rebasing" ]; then
-				b="$(cat "$g/rebase-apply/head-name" 2>/dev/null)"
+				read b 2>/dev/null <"$g/rebase-apply/head-name"
 				r="|REBASE"
 			elif [ -f "$g/rebase-apply/applying" ]; then
 				r="|AM"

-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:17 UTC · re: SZEDER Gábor · lore

[PATCH v2 08/13] bash prompt: use bash builtins to find out current branch

From: SZEDER Gábor <szeder@ira.uka.de>

__git_ps1() runs the '$(git symbolic-ref HEAD)' command substitution to find out whether we are on a branch and to find out the name of that branch. This imposes the overhead of fork()ing a subshell and fork()+exec()ing a git process.

Since HEAD is in most cases a single-line file and the symbolic ref format is quite simple to recognize and parse, read and parse it using only bash builtins, thereby sparing all that fork()+exec() overhead. However, HEAD can also be a symlink symbolic ref (due to 'core.preferSymlinkRefs'), so do this only if HEAD is not a symlink.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-prompt.sh | 46 ++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 18 deletions(-)
Show changes to contrib/completion/git-prompt.sh +28 −18
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 26380787..4e5c8efa 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -355,25 +355,35 @@ __git_ps1 ()
 			r="|BISECTING"
 		fi
 
-		test -n "$b" ||
-		b="$(git symbolic-ref HEAD 2>/dev/null)" || {
-			detached=yes
-			b="$(
-			case "${GIT_PS1_DESCRIBE_STYLE-}" in
-			(contains)
-				git describe --contains HEAD ;;
-			(branch)
-				git describe --contains --all HEAD ;;
-			(describe)
-				git describe HEAD ;;
-			(* | default)
-				git describe --tags --exact-match HEAD ;;
-			esac 2>/dev/null)" ||
+		if [ -n "$b" ]; then
+			:
+		elif [ -h "$g/HEAD" ]; then
+			# symlink symbolic ref
+			b="$(git symbolic-ref HEAD 2>/dev/null)"
+		else
+			local head=""
+			read head 2>/dev/null <"$g/HEAD" || return
+			# is it a symbolic ref?
+			b="${head#ref: }"
+			if [ "$head" = "$b" ]; then
+				detached=yes
+				b="$(
+				case "${GIT_PS1_DESCRIBE_STYLE-}" in
+				(contains)
+					git describe --contains HEAD ;;
+				(branch)
+					git describe --contains --all HEAD ;;
+				(describe)
+					git describe HEAD ;;
+				(* | default)
+					git describe --tags --exact-match HEAD ;;
+				esac 2>/dev/null)" ||
 
-			b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
-			b="unknown"
-			b="($b)"
-		}
+				b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
+				b="unknown"
+				b="($b)"
+			fi
+		fi
 	fi
 
 	if [ -n "$step" ] && [ -n "$total" ]; then

-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:17 UTC · re: SZEDER Gábor · lore

[PATCH v2 09/13] bash prompt: use bash builtins to get detached HEAD abbrev. object name

From: SZEDER Gábor <szeder@ira.uka.de>

When describing a detached HEAD according to the $GIT_PS1_DESCRIBE environment variable fails, __git_ps1() runs the '$(cut -c1-7 .git/HEAD)' command substitution to put the 7 hexdigits abbreviated commit object name in the prompt. This imposes the overhead of fork()ing a subshell and fork()+exec()ing 'cut'.

Thanks to the previous patch at this point the contents of HEAD has already been read into a variable, so we can get the 7 hexdigits using only parameter expansions, sparing the fork()+exec() overhead.

Since zsh doesn't implement substring expansion we can't just use ${head:0:7}, hence the "remove everything except the first 7 chars" parameter expansion combination.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-prompt.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Show changes to contrib/completion/git-prompt.sh +2 −3
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 4e5c8efa..10ec6902 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -378,9 +378,8 @@ __git_ps1 ()
 				(* | default)
 					git describe --tags --exact-match HEAD ;;
 				esac 2>/dev/null)" ||
-
-				b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
-				b="unknown"
+				# detached head abbreviated object name
+				b="${head%${head#???????}}..."
 				b="($b)"
 			fi
 		fi

-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:17 UTC · re: SZEDER Gábor · lore

[PATCH v2 10/13] bash prompt: combine 'git rev-parse' executions

From: SZEDER Gábor <szeder@ira.uka.de>
>From the four '$(git rev-parse --<opt>)' command substitutions
__git_ps1() executes three in its main code path:
 - the first to get the path to the .git directory ('--git-dir'),
 - the second to check whether we're inside the .git directory
   ('--is-inside-git-dir'),
 - and the last, depending on the results of the second, either
   * to check whether it's a bare repo ('--is-bare-repository'), or
   * to check whether inside a work tree ('--is-inside-work-tree').

Naturally, this imposes the overhead of fork()ing three subshells and fork()+exec()ing three git commands.

Combine these four 'git rev-parse' queries into one and use bash parameter expansions to parse the combined output, i.e. to separate the path to the .git directory from the true/false of '--is-inside-git-dir', etc. This way we can eliminate two of the three subshells and git commands.

The whole series speeds up the bash prompt on Windows/MSysGit considerably. Here are some timing results in two scenarios, repeated 10 times:

At the top of the work tree, before:
    $ time for i in {0..9} ; do prompt="$(__git_ps1)" ; done
    real    0m1.716s
    user    0m0.301s
    sys     0m0.772s
  After:
    real    0m0.686s
    user    0m0.075s
    sys     0m0.287s

In a subdirectory, during rebase, stash status indicator enabled, before:

    real    0m3.557s
    user    0m0.495s
    sys     0m1.767s
  After:
    real    0m0.702s
    user    0m0.045s
    sys     0m0.409s
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-prompt.sh | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
Show changes to contrib/completion/git-prompt.sh +13 −5
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 10ec6902..b73cebf7 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -311,8 +311,9 @@ __git_ps1 ()
 		;;
 	esac
 
-	local g=
-	if ! g="$(git rev-parse --git-dir 2>/dev/null)"; then
+	local repo_info=
+	if ! repo_info="$(git rev-parse --git-dir --is-inside-git-dir \
+		--is-bare-repository --is-inside-work-tree 2>/dev/null)"; then
 		if [ $pcmode = yes ]; then
 			#In PC mode PS1 always needs to be set
 			PS1="$ps1pc_start$ps1pc_end"
@@ -320,6 +321,13 @@ __git_ps1 ()
 		return
 	fi
 
+	local inside_worktree="${repo_info##*$'\n'}"
+	repo_info="${repo_info%$'\n'*}"
+	local bare_repo="${repo_info##*$'\n'}"
+	repo_info="${repo_info%$'\n'*}"
+	local inside_gitdir="${repo_info##*$'\n'}"
+	local g="${repo_info%$'\n'*}"
+
 	local r=""
 	local b=""
 	local step=""
@@ -396,13 +404,13 @@ __git_ps1 ()
 	local c=""
 	local p=""
 
-	if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
-		if [ "true" = "$(git rev-parse --is-bare-repository 2>/dev/null)" ]; then
+	if [ "true" = "$inside_gitdir" ]; then
+		if [ "true" = "$bare_repo" ]; then
 			c="BARE:"
 		else
 			b="GIT_DIR!"
 		fi
-	elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
+	elif [ "true" = "$inside_worktree" ]; then
 		if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ] &&
 		   [ "$(git config --bool bash.showDirtyState)" != "false" ]
 		then

-- 
1.8.3.1.487.g8f4672d
Jeff King· Jun 18, 2013, 06:05 UTC · re: SZEDER Gábor · lore

Re: [PATCH v2 10/13] bash prompt: combine 'git rev-parse' executions

On Tue, Jun 18, 2013 at 04:17:03AM +0200, SZEDER Gábor wrote:
Show 30 quoted lines
> The whole series speeds up the bash prompt on Windows/MSysGit
> considerably.  Here are some timing results in two scenarios, repeated
> 10 times:
> 
> At the top of the work tree, before:
> 
>     $ time for i in {0..9} ; do prompt="$(__git_ps1)" ; done
> 
>     real    0m1.716s
>     user    0m0.301s
>     sys     0m0.772s
> 
>   After:
> 
>     real    0m0.686s
>     user    0m0.075s
>     sys     0m0.287s
> 
> In a subdirectory, during rebase, stash status indicator enabled,
> before:
> 
>     real    0m3.557s
>     user    0m0.495s
>     sys     0m1.767s
> 
>   After:
> 
>     real    0m0.702s
>     user    0m0.045s
>     sys     0m0.409s

Very nice speedup (or perhaps it is a testament to how bad fork() is on msys). Reading patches 8 and 9, I can't help but feel that "git status" is letting us down a little by making us parse all of this data ourselves. In theory, __git_ps1() could just be something like:

  eval "$(git status --shell)"
  printf ...

and the heavy lifting could be done in a single C process which does not have to worry about fork overhead. But that is quite far from where we are now, so while it might be an interesting place to go in the future, I do not think such dreams would want to hold up current work.

-Peff
SZEDER Gábor· Jun 18, 2013, 09:49 UTC · re: Jeff King · lore

Re: [PATCH v2 10/13] bash prompt: combine 'git rev-parse' executions

On Tue, Jun 18, 2013 at 02:05:35AM -0400, Jeff King wrote:
Show 35 quoted lines
> On Tue, Jun 18, 2013 at 04:17:03AM +0200, SZEDER Gábor wrote:
> 
> > The whole series speeds up the bash prompt on Windows/MSysGit
> > considerably.  Here are some timing results in two scenarios, repeated
> > 10 times:
> > 
> > At the top of the work tree, before:
> > 
> >     $ time for i in {0..9} ; do prompt="$(__git_ps1)" ; done
> > 
> >     real    0m1.716s
> >     user    0m0.301s
> >     sys     0m0.772s
> > 
> >   After:
> > 
> >     real    0m0.686s
> >     user    0m0.075s
> >     sys     0m0.287s
> > 
> > In a subdirectory, during rebase, stash status indicator enabled,
> > before:
> > 
> >     real    0m3.557s
> >     user    0m0.495s
> >     sys     0m1.767s
> > 
> >   After:
> > 
> >     real    0m0.702s
> >     user    0m0.045s
> >     sys     0m0.409s
> 
> Very nice speedup (or perhaps it is a testament to how bad fork() is on
> msys).

Well, it seems it's not just fork() & friends. The latter case on Linux, before:

  $ time for i in {0..99}; do prompt="$(__git_ps1)" ; done
  
    real    0m2.819s
    user    0m0.180s
    sys     0m0.272s
  After:
  
    real    0m0.787s
    user    0m0.000s
    sys     0m0.044s

If you look solely at speedup (Win/MSys: 80%, Linux: 72%), Linux isn't that much better either, but overall it's about an order of magnitude faster to begin with (100 repetitions vs. 10).

Btw, it could still be a bit faster in you would care to change your prompt to run from $PROMPT_COMMAND, because it would avoid that final $(__git_ps1) command sunstitution, too. But I didn't measured that because I find the interface awful ;) (Hmm, speaking of which, the patch reading HEAD might break setups using $PROMPT_COMMAND, because it might do a simple return without updating $PS1...)

Show 11 quoted lines
> Reading patches 8 and 9, I can't help but feel that "git status"
> is letting us down a little by making us parse all of this data
> ourselves. In theory, __git_ps1() could just be something like:
> 
>   eval "$(git status --shell)"
>   printf ...
> 
> and the heavy lifting could be done in a single C process which does not
> have to worry about fork overhead. But that is quite far from where we
> are now, so while it might be an interesting place to go in the future,
> I do not think such dreams would want to hold up current work.

Yeah, that would be one way to go. It would have the added benefit that it could support 'core.abbrev' in the non-describable detached HEAD case without noticable overhead.

OTOH it still requires a command substitution and a git command, so it's less than ideal. The previous version of this series more than a year ago did some tricky things to create a prompt without a single fork(), let alone exec(), e.g. looking for .git directory with bash builtins, comparing pwd's prefix with path to .git dir to find out whether inside .git dir, etc. As a result even that subdir+rebase+stash case could be done in 10ms on Windows. Too bad that that "inside .git dir" check could misfire on case insensitive file systems, so we have to do that check with '$(git rev-parse)'.

  http://thread.gmane.org/gmane.comp.version-control.git/197432/focus=197450
Gábor
SZEDER Gábor· Jun 18, 2013, 10:01 UTC · re: SZEDER Gábor · lore

[PATCH] fixup! bash prompt: use bash builtins to find out current branch

From: SZEDER Gábor <szeder@ira.uka.de>

--- On Tue, Jun 18, 2013 at 11:49:31AM +0200, SZEDER Gábor wrote:

> (Hmm, speaking of which, the
> patch reading HEAD might break setups using $PROMPT_COMMAND, because
> it might do a simple return without updating $PS1...)
Indeed.  Here's the fixup.
 contrib/completion/git-prompt.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Show changes to contrib/completion/git-prompt.sh +6 −1
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 4e5c8efa..3c8c7b0d 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -362,7 +362,12 @@ __git_ps1 ()
 			b="$(git symbolic-ref HEAD 2>/dev/null)"
 		else
 			local head=""
-			read head 2>/dev/null <"$g/HEAD" || return
+			if ! read head 2>/dev/null <"$g/HEAD"; then
+				if [ $pcmode = yes ]; then
+					PS1="$ps1pc_start$ps1pc_end"
+				fi
+				return
+			fi
 			# is it a symbolic ref?
 			b="${head#ref: }"
 			if [ "$head" = "$b" ]; then

-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:17 UTC · re: SZEDER Gábor · lore

[PATCH v2 11/13] bash prompt: use bash builtins to check stash state

From: SZEDER Gábor <szeder@ira.uka.de>

When the environment variable $GIT_PS1_SHOWSTASHSTATE is set __git_ps1() checks the presence of stashes by running 'git rev-parse --verify refs/stash'. This command not only checks that the 'refs/stash' ref exists but also, well, verifies that it's a valid ref.

However, we don't need to be that thorough for the bash prompt. We can omit that verification and only check whether 'refs/stash' exists or not. Since 'git pack-refs' never packs 'refs/stash', it's a matter of checking the existence of a ref file. Perform this check using only bash builtins to spare the overhead of fork()+exec()ing a git process.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-prompt.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Show changes to contrib/completion/git-prompt.sh +3 −2
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index b73cebf7..208e2ec7 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -421,8 +421,9 @@ __git_ps1 ()
 				i="#"
 			fi
 		fi
-		if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
-			git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
+		if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ] &&
+		   [ -r "$g/refs/stash" ]; then
+			s="$"
 		fi
 
 		if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ] &&

-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:17 UTC · re: SZEDER Gábor · lore

[PATCH v2 12/13] bash prompt: avoid command substitution when checking for untracked files

From: SZEDER Gábor <szeder@ira.uka.de>

When enabled, the bash prompt can indicate the presence of untracked files with a '%' sign. __git_ps1() checks for untracked files by running the '$(git ls-files --others --exclude-standard)' command substitution, and displays the indicator when there is no output.

Avoid this command substitution by additionally passing '--error-unmatch *', and checking the command's return value.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-prompt.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to contrib/completion/git-prompt.sh +1 −1
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 208e2ec7..77575b2d 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -428,7 +428,7 @@ __git_ps1 ()
 
 		if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ] &&
 		   [ "$(git config --bool bash.showUntrackedFiles)" != "false" ] &&
-		   [ -n "$(git ls-files --others --exclude-standard)" ]
+		   git ls-files --others --exclude-standard --error-unmatch -- '*' >/dev/null 2>/dev/null
 		then
 			u="%${ZSH_VERSION+%}"
 		fi

-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:17 UTC · re: SZEDER Gábor · lore

[PATCH v2 13/13] bash prompt: avoid command substitution when finalizing gitstring

From: SZEDER Gábor <szeder@ira.uka.de>

Before setting $PS1, __git_ps1() uses a command substitution to redirect the output from a printf into a variable. Spare the overhead of fork()ing a subshell by using 'printf -v <var>' to directly assign the output to a variable.

zsh's printf doesn't support the '-v <var>' option, so stick with the command substitution when under zsh.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-prompt.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
Show changes to contrib/completion/git-prompt.sh +5 −1
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 77575b2d..a8e78f42 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -447,7 +447,11 @@ __git_ps1 ()
 		else
 			gitstring="$c${b##refs/heads/}${f:+$z$f}$r$p"
 		fi
-		gitstring=$(printf -- "$printf_format" "$gitstring")
+		if [[ -n ${ZSH_VERSION-} ]]; then
+			gitstring=$(printf -- "$printf_format" "$gitstring")
+		else
+			printf -v gitstring -- "$printf_format" "$gitstring"
+		fi
 		PS1="$ps1pc_start$gitstring$ps1pc_end"
 	else
 		# NO color option unless in PROMPT_COMMAND mode

-- 
1.8.3.1.487.g8f4672d
SZEDER Gábor· Jun 18, 2013, 02:28 UTC · re: SZEDER Gábor · lore

Re: [PATCH v2 00/13] bash prompt speedup

Forgot to mention that it's built on top of 2847cae8 (prompt: squelch error output from cat, 2013-06-14).

Best, Gábor

On Tue, Jun 18, 2013 at 04:16:53AM +0200, SZEDER Gábor wrote:
Show 42 quoted lines
> Hi,
> 
> displaying the git-specific bash prompt on Windows/MinGW takes quite
> long, long enough to be noticeable.  This is mainly caused by the
> numerous fork()s and exec()s to create subshells and run git or other
> commands, which are rather expensive on Windows.
> 
> This patch series eliminates many command substitutions and commands
> in __git_ps1() from top to bottom by replacing them with bash builtins
> or consolidating them.  A few timing results are shown in the log
> message of patch 10.
> 
> 
> SZEDER Gábor (13):
>   bash prompt: fix redirection coding style in tests
>   bash prompt: fix here document indentation in interactive rebase test
>   completion, bash prompt: move __gitdir() tests to completion test
>     suite
>   bash prompt: add a test for symbolic link symbolic refs
>   bash prompt: return early from __git_ps1() when not in a git
>     repository
>   bash prompt: run 'git rev-parse --git-dir' directly instead of
>     __gitdir()
>   bash prompt: use bash builtins to find out rebase state
>   bash prompt: use bash builtins to find out current branch
>   bash prompt: use bash builtins to get detached HEAD abbrev. object
>     name
>   bash prompt: combine 'git rev-parse' executions
>   bash prompt: use bash builtins to check stash state
>   bash prompt: avoid command substitution when checking for untracked
>     files
>   bash prompt: avoid command substitution when finalizing gitstring
> 
>  contrib/completion/git-completion.bash |   2 -
>  contrib/completion/git-prompt.sh       | 223 ++++++++++++-----------
>  t/t9902-completion.sh                  | 134 ++++++++++++++
>  t/t9903-bash-prompt.sh                 | 319 +++++++++++----------------------
>  4 files changed, 345 insertions(+), 333 deletions(-)
> 
> -- 
> 1.8.3.1.487.g8f4672d
>
Junio C Hamano· Jun 18, 2013, 16:48 UTC · re: SZEDER Gábor · lore

Re: [PATCH v2 00/13] bash prompt speedup

SZEDER Gábor <szeder@ira.uka.de> writes:
> This patch series eliminates many command substitutions and commands
> in __git_ps1() from top to bottom by replacing them with bash builtins
> or consolidating them.  A few timing results are shown in the log
> message of patch 10.

Nice. I think I saw Peff's comment and discussion between you two already resuted in a fixup, so perhaps I'll see a reroll sometime later when the dust settles?

Also, could you help review the other topic by Eduardo R. D'Avila about colored prompt (Sion Oosthoek, who did the color support, CC'ed)?

    http://thread.gmane.org/gmane.comp.version-control.git/228017

The impression I got when the PROMPT_COMMAND series was discussed last October was that you need to use \[...\] pairs to get the cursor position right for the purpose of command line editing, and D'Avila's series seemed to only do so in PROMPT_COMMAND mode.

Simon Oosthoek· Jun 18, 2013, 19:35 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 00/13] bash prompt speedup

* Junio C Hamano <gitster@pobox.com> [2013-06-18 09:48:28 -0700]:
Show 22 quoted lines
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > This patch series eliminates many command substitutions and commands
> > in __git_ps1() from top to bottom by replacing them with bash builtins
> > or consolidating them.  A few timing results are shown in the log
> > message of patch 10.
> 
> Nice.  I think I saw Peff's comment and discussion between you two
> already resuted in a fixup, so perhaps I'll see a reroll sometime
> later when the dust settles?
> 
> Also, could you help review the other topic by Eduardo R. D'Avila
> about colored prompt (Sion Oosthoek, who did the color support,
> CC'ed)?
> 
>     http://thread.gmane.org/gmane.comp.version-control.git/228017
> 
> The impression I got when the PROMPT_COMMAND series was discussed
> last October was that you need to use \[...\] pairs to get the
> cursor position right for the purpose of command line editing, and
> D'Avila's series seemed to only do so in PROMPT_COMMAND mode.
> 
Hi Junio e.a.
Unfortunately I'm very busy with lots of stuff, so hardly time to look at this. (I didn't read the full patch or resulting file)
The PROMPT_COMMAND business was not something I was glad to use, I was unable to get command substitution mode to work with colours and keep bash happy about the prompt string length. The thing to test is not whether or not colours appear when using substitution in PS1, but to see whether bash will not mess up the commandline when browsing through the history. This is especially inconvenient with very long commandlines (as you can imagine).
An optimisation which I've not yet had time to work on would be to do a lot of the processing in separate functions and use the result in separate functions for pcmode and subst. mode. The idea being that the maintainability of the logic for generating the prompt goes up (no duplicate code, clear separation, etc.) and the size of the called functions to gerenate the prompt goes down. And perhaps most important that it would be very easy to build a customised version of __git_ps1() using those functions from the standard code. 
I know this is pretty vague, I wish it wasn't so...
If it's somehow possible to eliminate the PROMPT_COMMAND mode, I'm all for it. But I doubt it can be done while keeping commandline browsing working ok... (please go ahead and prove me wrong!)
Cheers
Simon

← back to recent threads