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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Matthieu Moy <Matthieu.Moy@xxxxxxx> writes:
>
>> 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.

>> Signed-off-by: Matthieu Moy <Matthieu.Moy@xxxxxxx>
>> ---
>>  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)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]