[PATCH] When nothing to git-commit, honor the git-status color setting.

[PATCH] When nothing to git-commit, honor the git-status color setting.

[Brian Hetro]

Instead of disabling color all of the time during a git-commit, allow
the user's config preference in the situation where there is nothing
to commit.  In this situation, the status is printed to the terminal
and not sent to COMMIT_EDITMSG, so honoring the status color setting
is expected.

Signed-off-by: Brian Hetro <whee@smaertness.net>
---
 git-commit.sh |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/git-commit.sh b/git-commit.sh
index d7e7028..1d04f1f 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -49,10 +49,11 @@ run_status () {
 		export GIT_INDEX_FILE
 	fi
 
-	case "$status_only" in
-	t) color= ;;
-	*) color=--nocolor ;;
-	esac
+	if test "$status_only" = "t" -o "$use_status_color" = "t"; then
+		color=
+	else
+		color=--nocolor
+	fi
 	git runstatus ${color} \
 		${verbose:+--verbose} \
 		${amend:+--amend} \
@@ -556,6 +557,7 @@ fi
 if [ "$?" != "0" -a ! -f "$GIT_DIR/MERGE_HEAD" -a -z "$amend" ]
 then
 	rm -f "$GIT_DIR/COMMIT_EDITMSG" "$GIT_DIR/SQUASH_MSG"
+	use_status_color=t
 	run_status
 	exit 1
 fi
-- 
1.5.3.rc6.23.g0058-dirty

Re: [PATCH] When nothing to git-commit, honor the git-status color setting.

[Jeff King]

On Sun, Aug 26, 2007 at 02:35:26PM -0400, Brian Hetro wrote:

> Instead of disabling color all of the time during a git-commit, allow
> the user's config preference in the situation where there is nothing
> to commit.  In this situation, the status is printed to the terminal
> and not sent to COMMIT_EDITMSG, so honoring the status color setting
> is expected.

Thanks, this had been annoying me for a while. For some reason, I was
thinking that it was not going to be trivial to fix, because I thought
for efficiency reasons we only ran run_status once and used the output
for either the commit template or for dumping to the user. But I can't
seem to find any revision where that is the case, so obviously somebody
spiked my drink. For a month.

A minor nit on the implementation:

> +	if test "$status_only" = "t" -o "$use_status_color" = "t"; then
> +		color=
> +	else
> +		color=--nocolor
> +	fi

This variable doesn't really say "use color"; it says "don't explicitly
turn off color". So perhaps there is a better name (respect_color or
similar)?

-Peff

Re: [PATCH] When nothing to git-commit, honor the git-status color setting.

[Brian Hetro]

On Mon, Aug 27, 2007 at 04:25:50 -0400, Jeff King wrote:
> On Sun, Aug 26, 2007 at 02:35:26PM -0400, Brian Hetro wrote:
> 
> > Instead of disabling color all of the time during a git-commit, allow
> > the user's config preference in the situation where there is nothing
> > to commit.  In this situation, the status is printed to the terminal
> > and not sent to COMMIT_EDITMSG, so honoring the status color setting
> > is expected.

> A minor nit on the implementation:
> 
> > +	if test "$status_only" = "t" -o "$use_status_color" = "t"; then
> > +		color=
> > +	else
> > +		color=--nocolor
> > +	fi
> 
> This variable doesn't really say "use color"; it says "don't explicitly
> turn off color". So perhaps there is a better name (respect_color or
> similar)?

I was thinking more along the lines of "use color as if you had run
git-status" when I decided on $use_status_color.  Perhaps there is a
better name.

Brian

Re: [PATCH] When nothing to git-commit, honor the git-status color setting.

[Jeff King]

On Mon, Aug 27, 2007 at 05:45:43AM -0400, Brian Hetro wrote:

> I was thinking more along the lines of "use color as if you had run
> git-status" when I decided on $use_status_color.  Perhaps there is a
> better name.

I wonder if the implementation below is slightly more readable.

BTW, I suspect Junio will not apply this until post-1.5.3; I am starting
a queue of such patches that I care about and will re-send after the
release.

-Peff

---
diff --git a/git-commit.sh b/git-commit.sh
index d7e7028..96cec04 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -13,6 +13,7 @@ git rev-parse --verify HEAD >/dev/null 2>&1 || initial_commit=t
 case "$0" in
 *status)
 	status_only=t
+	no_override_color=t
 	;;
 *commit)
 	status_only=
@@ -49,7 +50,7 @@ run_status () {
 		export GIT_INDEX_FILE
 	fi
 
-	case "$status_only" in
+	case "$no_override_color" in
 	t) color= ;;
 	*) color=--nocolor ;;
 	esac
@@ -556,7 +557,7 @@ fi
 if [ "$?" != "0" -a ! -f "$GIT_DIR/MERGE_HEAD" -a -z "$amend" ]
 then
 	rm -f "$GIT_DIR/COMMIT_EDITMSG" "$GIT_DIR/SQUASH_MSG"
-	run_status
+	no_override_color=t run_status
 	exit 1
 fi
 

> 
> Brian
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] When nothing to git-commit, honor the git-status color setting.

[Brian Hetro]

On Mon, Aug 27, 2007 at 06:15:09 -0400, Jeff King wrote:
> On Mon, Aug 27, 2007 at 05:45:43AM -0400, Brian Hetro wrote:
> 
> > I was thinking more along the lines of "use color as if you had run
> > git-status" when I decided on $use_status_color.  Perhaps there is a
> > better name.
> 
> I wonder if the implementation below is slightly more readable.

I do prefer your implementation.  There is no need to depend on
status_only here.

> I am starting a queue of such patches that I care about and will
> re-send after the release.

Please do.

> ---
> diff --git a/git-commit.sh b/git-commit.sh
> index d7e7028..96cec04 100755
> --- a/git-commit.sh
> +++ b/git-commit.sh
> @@ -13,6 +13,7 @@ git rev-parse --verify HEAD >/dev/null 2>&1 || initial_commit=t
>  case "$0" in
>  *status)
>  	status_only=t
> +	no_override_color=t
>  	;;
>  *commit)
>  	status_only=
> @@ -49,7 +50,7 @@ run_status () {
>  		export GIT_INDEX_FILE
>  	fi
>  
> -	case "$status_only" in
> +	case "$no_override_color" in
>  	t) color= ;;
>  	*) color=--nocolor ;;
>  	esac
> @@ -556,7 +557,7 @@ fi
>  if [ "$?" != "0" -a ! -f "$GIT_DIR/MERGE_HEAD" -a -z "$amend" ]
>  then
>  	rm -f "$GIT_DIR/COMMIT_EDITMSG" "$GIT_DIR/SQUASH_MSG"
> -	run_status
> +	no_override_color=t run_status
>  	exit 1
>  fi