Re: [PATCH v5 6/6] status: add missing blank line after list of "other" files

Junio C Hamano [off-list ref] writes:

Matthieu Moy [off-list ref] writes:

quoted

List of files in other sections ("Changes to be committed", ...) end with
a blank line.

It is not like we want to add a blank line at the end of each
element; it is rather that we want to have a blank line between each
element, so that they can have a bit of breathing room between them.

The output looks especially bad when there is nothing after the
'untracked' list.

We might want to revisit this later, but I take the above back at
least for now.

If we have absolutely nothing, we get this:

	$ git status
	On branch master
        nothing to commit
	$

And if we have something, we get this:

	$ git status
	On branch master
	Changes to be committed:
	  (use "git reset HEAD <file>..." to unstage)

	        new file:   foo

	$ git status
	On branch master
	Untracked files:
	  (use "git add <file>..." to include in what will be committed)

		bar
	nothing added to commit
	$ git status
	On branch master
	Changes to be committed:
	  (use "git reset HEAD <file>..." to unstage)

	        new file:   foo

	Untracked files:
	  (use "git add <file>..." to include in what will be committed)

		bar
	$

Given that we are already giving an unnecessary trailing blank line
in the "fully added without no untracked" case, it is not too bad to
add the same after the list of untracked files.

To properly "fix" this, we would need to refactor wt_status_print_*
which will get rid of wt_status_print_trailer(), so that routines
that produce each section is told if there is a previous output and
adds a separating line _before_ it emits anything on its own
(i.e. we switch from the "after we spit our stuff out, we leave a
blank line" mental model to "we are going to spit our stuff out but
we need a blank before it to separate our stuff from the previous
output"), and returns if the output now has something to the caller
so that the caller can pass that information to the function that
outputs the next section.

But until that happens, adding a trailing blank line unconditionally
is an OK thing to do, I think.

quoted

Signed-off-by: Matthieu Moy <redacted>
---
 t/t7508-status.sh | 21 +++++++++++++++++++++
 wt-status.c       |  4 +++-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index d0444d3..9bf9701 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh

@@ -84,6 +84,7 @@ test_expect_success 'status --column' '
 #
 #	dir1/untracked dir2/untracked output
 #	dir2/modified  expect         untracked
+#
 EOF
 	COLUMNS=50 git -c status.displayCommentPrefix=true status --column="column dense" >output &&
 	test_i18ncmp expect output

@@ -117,6 +118,7 @@ cat >expect <<\EOF
 #	expect
 #	output
 #	untracked
+#
 EOF
 
 test_expect_success 'status with status.displayCommentPrefix=true' '

@@ -167,6 +169,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 EOF
 
 test_expect_success 'status (advice.statusHints false)' '

@@ -241,6 +244,7 @@ Untracked files:
   (use "git add <file>..." to include in what will be committed)
 
 	dir2/modified
+
 Ignored files:
   (use "git add -f <file>..." to include in what will be committed)
 

@@ -250,6 +254,7 @@ Ignored files:
 	expect
 	output
 	untracked
+
 EOF
 	git status --ignored >output &&
 	test_i18ncmp expect output

@@ -308,6 +313,7 @@ Ignored files:
 	expect
 	output
 	untracked
+
 EOF
 	git status --ignored >output &&
 	test_i18ncmp expect output

@@ -430,6 +436,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 EOF
 	git status -unormal >output &&
 	test_i18ncmp expect output

@@ -488,6 +495,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 EOF
 	git status -uall >output &&
 	test_i18ncmp expect output

@@ -548,6 +556,7 @@ Untracked files:
 	../expect
 	../output
 	../untracked
+
 EOF
 	(cd dir1 && git status) >output &&
 	test_i18ncmp expect output

@@ -618,6 +627,7 @@ Untracked files:
 	<BLUE>expect<RESET>
 	<BLUE>output<RESET>
 	<BLUE>untracked<RESET>
+
 EOF
 	test_config color.ui always &&
 	git status | test_decode_color >output &&

@@ -747,6 +757,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 EOF
 	test_config status.relativePaths false &&
 	(cd dir1 && git status) >output &&

@@ -789,6 +800,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 EOF
 	git commit --dry-run dir1/modified >output &&
 	test_i18ncmp expect output

@@ -838,6 +850,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 EOF
 	git status >output &&
 	test_i18ncmp expect output

@@ -902,6 +915,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 EOF
 	git config status.submodulesummary 10 &&
 	git status >output &&

@@ -952,6 +966,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
 	git commit -m "commit submodule" &&

@@ -1012,6 +1027,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 EOF
 	git config status.submodulesummary 10 &&
 	git commit --dry-run --amend >output &&

@@ -1066,6 +1082,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 EOF
 	echo modified  sm/untracked &&
 	git status --ignore-submodules=untracked >output &&

@@ -1177,6 +1194,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 EOF
 	git status --ignore-submodules=untracked > output &&
 	test_i18ncmp expect output

@@ -1238,6 +1256,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 EOF
 	git status --ignore-submodules=untracked > output &&
 	test_i18ncmp expect output

@@ -1319,6 +1338,7 @@ cat > expect << EOF
 ;	expect
 ;	output
 ;	untracked
+;
 EOF
 
 test_expect_success "status (core.commentchar with submodule summary)" '

@@ -1352,6 +1372,7 @@ Untracked files:
 	expect
 	output
 	untracked
+
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
 	git status --ignore-submodules=all > output &&

diff --git a/wt-status.c b/wt-status.c
index 3c795da..2a9ca0f 100644
--- a/wt-status.c
+++ b/wt-status.c

@@ -751,7 +751,7 @@ static void wt_status_print_other(struct wt_status *s,
 
 	strbuf_release(&buf);
 	if (!column_active(s->colopts))
-		return;
+		goto conclude;
 
 	strbuf_addf(&buf, "%s%s\t%s",
 		    color(WT_STATUS_HEADER, s),

@@ -765,6 +765,8 @@ static void wt_status_print_other(struct wt_status *s,
 	print_columns(&output, s->colopts, &copts);
 	string_list_clear(&output, 0);
 	strbuf_release(&buf);
+conclude:
+	status_printf_ln(s, GIT_COLOR_NORMAL, "");
 }
 
 static void wt_status_print_verbose(struct wt_status *s)