diff mbox series

completion: prompt: use generic colors

Message ID 20220827000810.2917816-1-felipe.contreras@gmail.com (mailing list archive)
State New, archived
Headers show
Series completion: prompt: use generic colors | expand

Commit Message

Felipe Contreras Aug. 27, 2022, 12:08 a.m. UTC
When the prompt command mode was introduced in 1bfc51ac81 (Allow
__git_ps1 to be used in PROMPT_COMMAND, 2012-10-10) the assumption was
that it was necessary in order to properly add colors to PS1 in bash,
but this wasn't true.

It's true that the \[ \] markers add the information needed to properly
calculate the width of the prompt, and they have to be added directly to
PS1, a function returning them doesn't work.

But that is because bash coverts the \[ \] markers in PS1 to \001 \002,
which is what readline ultimately needs in order to calculate the width.

We don't need bash to do this conversion, we can use \001 \002
ourselves, and then the prompt command mode is not necessary to display
colors.

This is what functions returning colors are supposed to do [1].

[1] http://mywiki.wooledge.org/BashFAQ/053

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

FTR, it's possible zsh will implement these \001 \002 markers too, so
there would be no need for different behavior depending on the shell.

 contrib/completion/git-prompt.sh | 19 +++++++------------
 t/t9903-bash-prompt.sh           |  8 ++++----
 2 files changed, 11 insertions(+), 16 deletions(-)

Comments

Justin Donnelly Aug. 27, 2022, 2:18 a.m. UTC | #1
On Fri, Aug 26, 2022 at 8:08 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> When the prompt command mode was introduced in 1bfc51ac81 (Allow
> __git_ps1 to be used in PROMPT_COMMAND, 2012-10-10) the assumption was
> that it was necessary in order to properly add colors to PS1 in bash,
> but this wasn't true.
>
> It's true that the \[ \] markers add the information needed to properly
> calculate the width of the prompt, and they have to be added directly to
> PS1, a function returning them doesn't work.
>
> But that is because bash coverts the \[ \] markers in PS1 to \001 \002,
> which is what readline ultimately needs in order to calculate the width.
>
> We don't need bash to do this conversion, we can use \001 \002
> ourselves, and then the prompt command mode is not necessary to display
> colors.
>
> This is what functions returning colors are supposed to do [1].
>
> [1] http://mywiki.wooledge.org/BashFAQ/053
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>
> FTR, it's possible zsh will implement these \001 \002 markers too, so
> there would be no need for different behavior depending on the shell.
>
>  contrib/completion/git-prompt.sh | 19 +++++++------------
>  t/t9903-bash-prompt.sh           |  8 ++++----
>  2 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index 1435548e00..01bd807657 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -96,9 +96,7 @@
>  #
>  # If you would like a colored hint about the current dirty state, set
>  # 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 in Bash,
> -# but always available in Zsh.
> +# the colored output of "git status -sb".
>  #
>  # 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
> @@ -255,12 +253,12 @@ __git_ps1_colorize_gitstring ()
>                 local c_lblue='%F{blue}'
>                 local c_clear='%f'
>         else
> -               # Using \[ and \] around colors is necessary to prevent
> +               # Using \001 and \002 around colors is necessary to prevent
>                 # issues with command line editing/browsing/completion!
> -               local c_red='\[\e[31m\]'
> -               local c_green='\[\e[32m\]'
> -               local c_lblue='\[\e[1;34m\]'
> -               local c_clear='\[\e[0m\]'
> +               local c_red=$'\001\e[31m\002'
> +               local c_green=$'\001\e[32m\002'
> +               local c_lblue=$'\001\e[1;34m\002'
> +               local c_clear=$'\001\e[0m\002'
>         fi
>         local bad_color=$c_red
>         local ok_color=$c_green
> @@ -564,11 +562,8 @@ __git_ps1 ()
>                 b="\${__git_ps1_branch_name}"
>         fi
>
> -       # NO color option unless in PROMPT_COMMAND mode or it's Zsh
>         if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
> -               if [ $pcmode = yes ] || [ -n "${ZSH_VERSION-}" ]; then
> -                       __git_ps1_colorize_gitstring
> -               fi
> +               __git_ps1_colorize_gitstring
>         fi
>
>         local f="$h$w$i$s$u$p"
> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
> index 6a30f5719c..594042f562 100755
> --- a/t/t9903-bash-prompt.sh
> +++ b/t/t9903-bash-prompt.sh
> @@ -13,10 +13,10 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  . "$GIT_BUILD_DIR/contrib/completion/git-prompt.sh"
>
>  actual="$TRASH_DIRECTORY/actual"
> -c_red='\\[\\e[31m\\]'
> -c_green='\\[\\e[32m\\]'
> -c_lblue='\\[\\e[1;34m\\]'
> -c_clear='\\[\\e[0m\\]'
> +c_red='\001\e[31m\002'
> +c_green='\001\e[32m\002'
> +c_lblue='\001\e[1;34m\002'
> +c_clear='\001\e[0m\002'
>
>  test_expect_success 'setup for prompt tests' '
>         git init otherrepo &&
> --
> 2.37.2.351.g9bf691b78c.dirty
>

This appeared to work for me with a quick test. I probably don't have
any sway, but FWIW I would use it.

Justin
diff mbox series

Patch

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 1435548e00..01bd807657 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -96,9 +96,7 @@ 
 #
 # If you would like a colored hint about the current dirty state, set
 # 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 in Bash,
-# but always available in Zsh.
+# the colored output of "git status -sb".
 #
 # 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
@@ -255,12 +253,12 @@  __git_ps1_colorize_gitstring ()
 		local c_lblue='%F{blue}'
 		local c_clear='%f'
 	else
-		# Using \[ and \] around colors is necessary to prevent
+		# Using \001 and \002 around colors is necessary to prevent
 		# issues with command line editing/browsing/completion!
-		local c_red='\[\e[31m\]'
-		local c_green='\[\e[32m\]'
-		local c_lblue='\[\e[1;34m\]'
-		local c_clear='\[\e[0m\]'
+		local c_red=$'\001\e[31m\002'
+		local c_green=$'\001\e[32m\002'
+		local c_lblue=$'\001\e[1;34m\002'
+		local c_clear=$'\001\e[0m\002'
 	fi
 	local bad_color=$c_red
 	local ok_color=$c_green
@@ -564,11 +562,8 @@  __git_ps1 ()
 		b="\${__git_ps1_branch_name}"
 	fi
 
-	# NO color option unless in PROMPT_COMMAND mode or it's Zsh
 	if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
-		if [ $pcmode = yes ] || [ -n "${ZSH_VERSION-}" ]; then
-			__git_ps1_colorize_gitstring
-		fi
+		__git_ps1_colorize_gitstring
 	fi
 
 	local f="$h$w$i$s$u$p"
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 6a30f5719c..594042f562 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -13,10 +13,10 @@  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . "$GIT_BUILD_DIR/contrib/completion/git-prompt.sh"
 
 actual="$TRASH_DIRECTORY/actual"
-c_red='\\[\\e[31m\\]'
-c_green='\\[\\e[32m\\]'
-c_lblue='\\[\\e[1;34m\\]'
-c_clear='\\[\\e[0m\\]'
+c_red='\001\e[31m\002'
+c_green='\001\e[32m\002'
+c_lblue='\001\e[1;34m\002'
+c_clear='\001\e[0m\002'
 
 test_expect_success 'setup for prompt tests' '
 	git init otherrepo &&