threads / patch / 21686

patchbash completion: add space between branch name and status flags

Subject: [PATCH] bash completion: add space between branch name and status flags

## tl;dr

7 messages between Nov 20, 2009 and Jan 6, 2010. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Roman Fietze· Nov 20, 2009, 12:09 UTC · lore
Hello Shawn, hello git list members,

Wouldn't it improve the readability of the bash prompt, if there would be a space between the branch name and the status flags (dirty, stash, untracked)?

Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
---
 contrib/completion/git-completion.bash |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Show changes to contrib/completion/git-completion.bash +4 −2
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-
completion.bash
index bd66639..407176b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -169,10 +169,12 @@ __git_ps1 ()
 			fi
 		fi
 
+		local f="$w$i$s$u$r"
+		f=${f:+ $f}
 		if [ -n "${1-}" ]; then
-			printf "$1" "$c${b##refs/heads/}$w$i$s$u$r"
+			printf "$1" "$c${b##refs/heads/}$f"
 		else
-			printf " (%s)" "$c${b##refs/heads/}$w$i$s$u$r"
+			printf " (%s)" "$c${b##refs/heads/}$f"
 		fi
 	fi
 }
-- 
1.6.4.2


Roman

-- 
Roman Fietze                Telemotive AG Büro Mühlhausen
Breitwiesen                              73347 Mühlhausen
Tel.: +49(0)7335/18493-45        http://www.telemotive.de
Junio C Hamano· Nov 20, 2009, 17:53 UTC · re: Roman Fietze · lore

Re: [PATCH] bash completion: add space between branch name and status flags

Roman Fietze <roman.fietze@telemotive.de> writes:
Show 5 quoted lines
> Hello Shawn, hello git list members,
>
> Wouldn't it improve the readability of the bash prompt, if there would
> be a space between the branch name and the status flags (dirty, stash,
> untracked)?
Perhaps.
Show 15 quoted lines
> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
> ---
>  contrib/completion/git-completion.bash |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-
> completion.bash
> index bd66639..407176b 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -169,10 +169,12 @@ __git_ps1 ()
>  			fi
>  		fi
>  
> +		local f="$w$i$s$u$r"

I think this line and the two changes to printf below is a big improvement (unless we are using f for something else---I didn't look). But I think the next line is wrong.

> +		f=${f:+ $f}

The $r string is designed to be used as suffix from the beginning and always has "|" in front of it as a delimiter, so if there is no w/i/s/u (and I suspect many people do not use GIT_PS1_SHOWDIRTYSTATE and friends, and these are _always_ empty for them) the above will begin with "|". There is no need to steal one column from a typeable width from the command line in such a case.

Show 9 quoted lines
>  		if [ -n "${1-}" ]; then
> -			printf "$1" "$c${b##refs/heads/}$w$i$s$u$r"
> +			printf "$1" "$c${b##refs/heads/}$f"
>  		else
> -			printf " (%s)" "$c${b##refs/heads/}$w$i$s$u$r"
> +			printf " (%s)" "$c${b##refs/heads/}$f"
>  		fi
>  	fi
>  }

To implement your stated goal (dirty/stash/untracked), you probably want to do this instead...

	local f="$w$i$s$u"
        f="${f:+ $f}$r"
Nanako Shiraishi· Dec 30, 2009, 13:41 UTC · re: Junio C Hamano · lore

Re: [PATCH] bash completion: add space between branch name and status flags

Junio, could you tell us what happened to this thread?

In response to a patch from Roman Fietze to outline a better way to do it. Nothing happened.

Shawn O. Pearce· Dec 30, 2009, 14:57 UTC · re: Nanako Shiraishi · lore

Re: [PATCH] bash completion: add space between branch name and status flags

Nanako Shiraishi <nanako3@lavabit.com> wrote:
> Junio, could you tell us what happened to this thread?
> 
> In response to a patch from Roman Fietze to outline a better way
> to do it.  Nothing happened.

Junio responded with a suggestion on how to improve the patch when GIT_PS1_SHOWDIRTYSTATE was not set, but Roman Fietze didn't send a revised patch, so it got dropped.

Here is the revised patch, Junio, still think its a good idea?
--8<--
From: Roman Fietze <roman.fietze@telemotive.de>
Subject: [PATCH] bash completion: add space between branch name and status flags

Improve the readability of the bash prompt by adding a space between the branch name and the status flags (dirty, stash, untracked).

Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 contrib/completion/git-completion.bash |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Show changes to contrib/completion/git-completion.bash +4 −2
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index fbfa5f2..3c8b6df 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -163,10 +163,12 @@ __git_ps1 ()
 			fi
 		fi
 
+		local f="$w$i$s$u"
+		f="${f:+ $f}$r"
 		if [ -n "${1-}" ]; then
-			printf "$1" "$c${b##refs/heads/}$w$i$s$u$r"
+			printf "$1" "$c${b##refs/heads/}$f"
 		else
-			printf " (%s)" "$c${b##refs/heads/}$w$i$s$u$r"
+			printf " (%s)" "$c${b##refs/heads/}$f"
 		fi
 	fi
 }
-- 
1.6.6.307.gba67


-- 
Shawn.
Junio C Hamano· Dec 30, 2009, 19:27 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] bash completion: add space between branch name and status flags

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 11 quoted lines
> Nanako Shiraishi <nanako3@lavabit.com> wrote:
>> Junio, could you tell us what happened to this thread?
>> 
>> In response to a patch from Roman Fietze to outline a better way
>> to do it.  Nothing happened.
>
> Junio responded with a suggestion on how to improve the patch when
> GIT_PS1_SHOWDIRTYSTATE was not set, but Roman Fietze didn't send
> a revised patch, so it got dropped.
>
> Here is the revised patch, Junio, still think its a good idea?
Thanks for following up.

As I don't use $[wisu] myself, I don't have a very strong opinion either way, but the user has spent cycles to compute them, so we'd better present them in a way that is easier to read, I guess.

Show 17 quoted lines
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index fbfa5f2..3c8b6df 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -163,10 +163,12 @@ __git_ps1 ()
>  			fi
>  		fi
>  
> +		local f="$w$i$s$u"
> +		f="${f:+ $f}$r"
>  		if [ -n "${1-}" ]; then
> -			printf "$1" "$c${b##refs/heads/}$w$i$s$u$r"
> +			printf "$1" "$c${b##refs/heads/}$f"
>  		else
> -			printf " (%s)" "$c${b##refs/heads/}$w$i$s$u$r"
> +			printf " (%s)" "$c${b##refs/heads/}$f"
>  		fi

I notice that printf argument look very similar. Maybe we want to do something like

    printf "${1:-" (%s)"}" ...
to avoid duplication?
Shawn O. Pearce· Dec 31, 2009, 03:04 UTC · re: Junio C Hamano · lore

Improve the readability of the bash prompt by adding a space between the branch name and the status flags (dirty, stash, untracked).

While we are cleaning up this section of code, the two cases for formatting the prompt are identical except for the format string, so make them the same.

Suggested-by: Roman Fietze <roman.fietze@telemotive.de>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
   Junio C Hamano <gitster@pobox.com> wrote:
   > I notice that printf argument look very similar.  Maybe we want to do
   > something like
   > 
   >     printf "${1:-" (%s)"}" ...
   > 
   > to avoid duplication?
   
   Ack.
   Because its rather far from the original poster's patch, I've
   taken blame for it.
   
 contrib/completion/git-completion.bash |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
Show changes to contrib/completion/git-completion.bash +3 −5
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index fbfa5f2..9ed7df2 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -163,11 +163,9 @@ __git_ps1 ()
 			fi
 		fi
 
-		if [ -n "${1-}" ]; then
-			printf "$1" "$c${b##refs/heads/}$w$i$s$u$r"
-		else
-			printf " (%s)" "$c${b##refs/heads/}$w$i$s$u$r"
-		fi
+		local f="$w$i$s$u"
+		f="${f:+ $f}$r"
+		printf "${1:- (%s)}" "$c${b##refs/heads/}$f"
 	fi
 }
 
-- 
1.6.6.325.g6f5f

-- 
Shawn.
Roman Fietze· Jan 6, 2010, 11:59 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] bash completion: add space between branch name and status flags

Hello Shawn,
On Wednesday 30 December 2009 15:57:51 Shawn O. Pearce wrote:
> ... but Roman Fietze didn't send
> a revised patch, so it got dropped

Sorry about that, but our Notes "spam filter" has been eating important mail again.

I'l check the version as proposed by you as soon as I'm back.
Roman
-- 
Roman Fietze                Telemotive AG Büro Mühlhausen
Breitwiesen                              73347 Mühlhausen
Tel.: +49(0)7335/18493-45        http://www.telemotive.de

← back to recent threads