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

PATCH In review Brian Hetro 2 participants 5 messages Jun 15, 2016

Share & tools

Brian Hetro

author

2016-06-15 22:43 UTC #0 lore raw

PATCH 1/1 When nothing to git-commit, honor the git-status color setting.

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 <[email protected]>

---
 git-commit.sh |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

git-commit.sh +6 -4

index d7e7028..1d04f1f 100755
@@ -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

Signature · 1 line

1.5.3.rc6.23.g0058-dirty

Jeff King

2016-06-15 22:43 UTC #1 lore raw

Quoted 5 lines · 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.

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:

git-commit.sh run_status () { · line 56

Quoted 5 lines

git-commit.sh +5 -0

+	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

Brian Hetro

author

2016-06-15 22:43 UTC #2 lore raw

Quoted 10 lines · Jeff King

Quoted 5 lines · 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.
A minor nit on the implementation:

Quoted 9 lines

git-commit.sh run_status () { · line 56
Quoted 5 lines
git-commit.sh +5 -0
+	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

Jeff King

2016-06-15 22:43 UTC #3 lore raw

Quoted 3 lines · Brian Hetro

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

---

git-commit.sh +3 -2

index d7e7028..96cec04 100755
@@ -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

Quoted 7 lines

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

Brian Hetro

author

2016-06-15 22:43 UTC #4 lore raw

Quoted 7 lines · Jeff King

Quoted 3 lines · Brian Hetro
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.

Quoted 2 lines

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

Please do.

Quoted 30 lines

---

git-commit.sh +3 -2

index d7e7028..96cec04 100755
@@ -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