Re: [PATCH v4] git-prompt.sh: Option to hide prompt for ignored pwd

Richard Hansen [off-list ref] writes:

This is the patch from:

  http://article.gmane.org/gmane.comp.version-control.git/258313

modified to include the changes I suggested in:

  http://article.gmane.org/gmane.comp.version-control.git/258355

I never heard back regarding my suggested changes.  The feature was so
close to ready and I thought it would be a shame for the feature to
silently die, so I'm submitting a re-roll with my suggested changes on
behalf of the original author.

+# If you would like __git_ps1 to do nothing in the case when the current
+# directory is set up to be ignored by git, then set
+# GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
+# repository level by setting bash.hideIfPwdIgnored to "false".

Perhaps nobody had much interest in the original or the update.
Occassionally resending with improvements like this is a good way to
show it to more people who may have missed it the last time to
solicit comments and supports.

I am personally not very interested, as you and the original made it
sound as if this is primarily for those who keep track of $HOME/.dot
files in $HOME/.git, which is one of the ways I would never use Git.

But I do not have to be the target of each and every new feature ;-).

Many existing checks on variables are written this way with the
"subsitutute with default value" syntax

        if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ] &&

to make sure that people with non-standard settings to report
references to unset variables as errors will not have to suffer.
Don't you need to do something similar here?

+	   [ "$(git config --bool bash.hideIfPwdIgnored)" != "false" ] &&
+	   git check-ignore -q .
+	then
+		if [ $pcmode = yes ]; then
+			PS1="$ps1pc_start$ps1pc_end"
+		fi
+		return

There are already two places where "under pcmode, we need to set PS1
to this empty thing" is known, and this patch adds yet another.
Would it be sensible to refactor that into a helper function, or
open coding them this way is necessary for performance or some other
reasons?

Drop the SP after (but not before) redirection operator >>.

From: Jess Austin <redacted>

Optionally set __git_ps1 to display nothing when present working
directory is ignored, triggered by the new environmental variable
GIT_PS1_HIDE_IF_PWD_IGNORED. This environmental variable may be
overridden on any repository by setting bash.hideIfPwdIgnored to
"false". In the absence of GIT_PS1_HIDE_IF_PWD_IGNORED this change
has no effect.

Many people manage e.g. dotfiles in their home directory with git.
This causes the prompt generated by __git_ps1 to refer to that "top
level" repo while working in any descendant directory. That can be
distracting, so this patch helps one shut off that noise.

Signed-off-by: Jess Austin <redacted>
Signed-off-by: Richard Hansen <redacted>
Reviewed-by: Richard Hansen <redacted>
---
 contrib/completion/git-prompt.sh |  13 +++++
 t/t9903-bash-prompt.sh           | 106 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index b0de082..75c3f0f 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh

@@ -84,6 +84,11 @@
 # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
 # the colored output of "git status -sb" and are available only when
 # using __git_ps1 for PROMPT_COMMAND or precmd.
+#
+# If you would like __git_ps1 to do nothing in the case when the current
+# directory is set up to be ignored by git, then set
+# GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
+# repository level by setting bash.hideIfPwdIgnored to "false".
 
 # check whether printf supports -v
 __git_printf_supports_v=

@@ -369,6 +374,14 @@ __git_ps1 ()
 	local inside_gitdir="${repo_info##*$'\n'}"
 	local g="${repo_info%$'\n'*}"
 
+	if [ "true" = "$inside_worktree" ] &&
+	   [ -n "${GIT_PS1_HIDE_IF_PWD_IGNORED-}" ] &&
+	   [ "$(git config --bool bash.hideIfPwdIgnored)" != "false" ] &&
+	   git check-ignore -q .
+	then
+		return
+	fi
+
 	local r=""
 	local b=""
 	local step=""

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 9150984..51ecd3e 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh

@@ -35,6 +35,8 @@ test_expect_success 'setup for prompt tests' '
 	git commit -m "another b2" file &&
 	echo 000 >file &&
 	git commit -m "yet another b2" file &&
+	mkdir ignored_dir &&
+	echo "ignored_dir/" >>.gitignore &&
 	git checkout master
 '
 

@@ -588,4 +590,108 @@ test_expect_success 'prompt - zsh color pc mode' '
 	test_cmp expected "$actual"
 '
 
+test_expect_success 'prompt - hide if pwd ignored - env var unset, config disabled' '
+	printf " (master)" >expected &&
+	test_config bash.hideIfPwdIgnored false &&
+	(
+		cd ignored_dir &&
+		__git_ps1 >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - hide if pwd ignored - env var unset, config disabled, pc mode' '
+	printf "BEFORE: (\${__git_ps1_branch_name}):AFTER" >expected &&
+	test_config bash.hideIfPwdIgnored false &&
+	(
+		cd ignored_dir &&
+		__git_ps1 "BEFORE:" ":AFTER" &&
+		printf "%s" "$PS1" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - hide if pwd ignored - env var unset, config unset' '
+	printf " (master)" >expected &&
+	(
+		cd ignored_dir &&
+		__git_ps1 >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - hide if pwd ignored - env var unset, config unset, pc mode' '
+	printf "BEFORE: (\${__git_ps1_branch_name}):AFTER" >expected &&
+	(
+		cd ignored_dir &&
+		__git_ps1 "BEFORE:" ":AFTER" &&
+		printf "%s" "$PS1" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - hide if pwd ignored - env var set, config disabled' '
+	printf " (master)" >expected &&
+	test_config bash.hideIfPwdIgnored false &&
+	(
+		cd ignored_dir &&
+		GIT_PS1_HIDE_IF_PWD_IGNORED=y &&
+		__git_ps1 >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - hide if pwd ignored - env var set, config disabled, pc mode' '
+	printf "BEFORE: (\${__git_ps1_branch_name}):AFTER" >expected &&
+	test_config bash.hideIfPwdIgnored false &&
+	(
+		cd ignored_dir &&
+		GIT_PS1_HIDE_IF_PWD_IGNORED=y &&
+		__git_ps1 "BEFORE:" ":AFTER" &&
+		printf "%s" "$PS1" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - hide if pwd ignored - env var set, config unset' '
+	printf "" >expected &&
+	(
+		cd ignored_dir &&
+		GIT_PS1_HIDE_IF_PWD_IGNORED=y &&
+		__git_ps1 >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - hide if pwd ignored - env var set, config unset, pc mode' '
+	printf "BEFORE::AFTER" >expected &&
+	(
+		cd ignored_dir &&
+		GIT_PS1_HIDE_IF_PWD_IGNORED=y &&
+		__git_ps1 "BEFORE:" ":AFTER" &&
+		printf "%s" "$PS1" >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - hide if pwd ignored - inside gitdir (stdout)' '
+	printf " (GIT_DIR!)" >expected &&
+	(
+		GIT_PS1_HIDE_IF_PWD_IGNORED=y &&
+		cd .git &&
+		__git_ps1 >"$actual" 2>/dev/null
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - hide if pwd ignored - inside gitdir (stderr)' '
+	printf "" >expected &&
+	(
+		GIT_PS1_HIDE_IF_PWD_IGNORED=y &&
+		cd .git &&
+		__git_ps1 >/dev/null 2>"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
 test_done

-- 
2.2.1

On 2015-01-06T15:31-08:00, Junio C Hamano wrote:

quoted

This is the patch from:

  http://article.gmane.org/gmane.comp.version-control.git/258313

modified to include the changes I suggested in:

  http://article.gmane.org/gmane.comp.version-control.git/258355

I never heard back regarding my suggested changes.  The feature was so
close to ready and I thought it would be a shame for the feature to
silently die, so I'm submitting a re-roll with my suggested changes on
behalf of the original author.

quoted

+# If you would like __git_ps1 to do nothing in the case when the current
+# directory is set up to be ignored by git, then set
+# GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
+# repository level by setting bash.hideIfPwdIgnored to "false".

Perhaps nobody had much interest in the original or the update.
Occassionally resending with improvements like this is a good way to
show it to more people who may have missed it the last time to
solicit comments and supports.

I am personally not very interested, as you and the original made it
sound as if this is primarily for those who keep track of $HOME/.dot
files in $HOME/.git, which is one of the ways I would never use Git.

I do keep my dotfiles in a Git repository (~/.git exists), yet I
wouldn't use this feature either.  (I just use refs/heads/dotfiles as
HEAD so that my prompt is unique when I'm not in some project working
directory.)

However, it doesn't seem like a very invasive change to me, and at
least one person wants this feature (evidenced by Jess Austin going to
the trouble of submitting a patch), so I thought I'd help it along.

If someone has a reasonable objection to this feature, or even if
there's not enough positive interest, I wouldn't be too sad to see it
not get adopted.

quoted

+	   [ -n "${GIT_PS1_HIDE_IF_PWD_IGNORED}" ] &&

Many existing checks on variables are written this way with the
"subsitutute with default value" syntax

        if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ] &&

to make sure that people with non-standard settings to report
references to unset variables as errors will not have to suffer.
Don't you need to do something similar here?

Yes; fixed.

quoted

+	   [ "$(git config --bool bash.hideIfPwdIgnored)" != "false" ] &&
+	   git check-ignore -q .
+	then
+		if [ $pcmode = yes ]; then
+			PS1="$ps1pc_start$ps1pc_end"
+		fi
+		return

There are already two places where "under pcmode, we need to set PS1
to this empty thing" is known, and this patch adds yet another.
Would it be sensible to refactor that into a helper function, or
open coding them this way is necessary for performance or some other
reasons?

I thought about factoring it out, but didn't because defining a
function with just three lines seemed awkward.  But I thought of a
better way to eliminate the duplicate code without defining a helper
function; see the new prequel patch.

quoted

+	echo "ignored_dir/" >> .gitignore &&

Drop the SP after (but not before) redirection operator >>.

Done.

Thanks for the review,
Richard


Jess Austin (1):
  git-prompt.sh: Option to hide prompt for ignored pwd

Richard Hansen (1):
  git-prompt.sh: if pc mode, immediately set PS1 to a plain prompt

 contrib/completion/git-prompt.sh |  24 ++++++---
 t/t9903-bash-prompt.sh           | 106 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 123 insertions(+), 7 deletions(-)

-- 
2.2.1

At the beginning of __git_ps1, right after determining that the
function is running in pc mode, set PS1 to a plain (undecorated)
prompt.  This makes it possible to simply return early without having
to set PS1 if the prompt should not be decorated.

Signed-off-by: Richard Hansen <redacted>
---
 contrib/completion/git-prompt.sh | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 729f769..b0de082 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh

@@ -299,6 +299,10 @@ __git_ps1 ()
 			ps1pc_start="$1"
 			ps1pc_end="$2"
 			printf_format="${3:-$printf_format}"
+			# set PS1 to a plain prompt so that we can
+			# simply return early if the prompt should not
+			# be decorated
+			PS1="$ps1pc_start$ps1pc_end"
 		;;
 		0|1)	printf_format="${1:-$printf_format}"
 		;;

@@ -350,10 +354,6 @@ __git_ps1 ()
 	rev_parse_exit_code="$?"
 
 	if [ -z "$repo_info" ]; then
-		if [ $pcmode = yes ]; then
-			#In PC mode PS1 always needs to be set
-			PS1="$ps1pc_start$ps1pc_end"
-		fi
 		return
 	fi
 

@@ -412,9 +412,6 @@ __git_ps1 ()
 		else
 			local head=""
 			if ! __git_eread "$g/HEAD" head; then
-				if [ $pcmode = yes ]; then
-					PS1="$ps1pc_start$ps1pc_end"
-				fi
 				return
 			fi
 			# is it a symbolic ref?

-- 
2.2.1

Hi,


Quoting Richard Hansen [off-list ref]:

As the one responsible for the last hunk I really like this change.

Thanks,
Gábor