v2status: add an empty line when there is no hint

> This patch adds empty lines in the above case and some
> similar cases.
> 
> Signed-off-by: John Lin <johnlinp@gmail.com>
> ---
>   t/t7500-commit-template-squash-signoff.sh |  1 +
>   t/t7508-status.sh                         |  5 +++++
>   t/t7512-status-help.sh                    |  1 +
>   wt-status.c                               | 12 ++++++++----
>   4 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
> index 46a5cd4b73..0423e77d1d 100755
> --- a/t/t7500-commit-template-squash-signoff.sh
> +++ b/t/t7500-commit-template-squash-signoff.sh
> @@ -345,6 +345,7 @@ cat >expected-template <<EOF
>   #
>   # On branch commit-template-check
>   # Changes to be committed:
> +#
>   #	new file:   commit-template-check
>   #
>   # Untracked files not listed
> diff --git a/t/t7508-status.sh b/t/t7508-status.sh
> index e1f11293e2..949b1dbcc4 100755
> --- a/t/t7508-status.sh
> +++ b/t/t7508-status.sh
> @@ -204,12 +204,15 @@ Your branch and 'upstream' have diverged,
>   and have 1 and 2 different commits each, respectively.
>   
>   Changes to be committed:
> +
>   	new file:   dir2/added
>   
>   Changes not staged for commit:
> +
>   	modified:   dir1/modified
>   
>   Untracked files:
> +
>   	dir1/untracked
>   	dir2/modified
>   	dir2/untracked
> @@ -449,9 +452,11 @@ Your branch and '\''upstream'\'' have diverged,
>   and have 1 and 2 different commits each, respectively.
>   
>   Changes to be committed:
> +
>   	new file:   dir2/added
>   
>   Changes not staged for commit:
> +
>   	modified:   dir1/modified
>   
>   Untracked files not listed
> diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
> index 458608cc1e..0a29fa66a2 100755
> --- a/t/t7512-status-help.sh
> +++ b/t/t7512-status-help.sh
> @@ -714,6 +714,7 @@ rebase in progress; onto $ONTO
>   You are currently rebasing branch '\''statushints_disabled'\'' on '\''$ONTO'\''.
>   
>   Unmerged paths:
> +
>   	both modified:   main.txt
>   
>   no changes added to commit
> diff --git a/wt-status.c b/wt-status.c
> index 445a36204a..0766e3ee12 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -175,7 +175,7 @@ static void wt_longstatus_print_unmerged_header(struct wt_status *s)
>   	}
>   
>   	if (!s->hints)
> -		return;
> +		goto conclude;
>   	if (s->whence != FROM_COMMIT)
>   		;
>   	else if (!s->is_initial)
> @@ -193,6 +193,7 @@ static void wt_longstatus_print_unmerged_header(struct wt_status *s)
>   	} else {
>   		status_printf_ln(s, c, _("  (use \"git add/rm <file>...\" as appropriate to mark resolution)"));
>   	}
> +conclude:
>   	status_printf_ln(s, c, "%s", "");
>   }
>   
> @@ -202,13 +203,14 @@ static void wt_longstatus_print_cached_header(struct wt_status *s)
>   
>   	status_printf_ln(s, c, _("Changes to be committed:"));
>   	if (!s->hints)
> -		return;
> +		goto conclude;
>   	if (s->whence != FROM_COMMIT)
>   		; /* NEEDSWORK: use "git reset --unresolve"??? */
>   	else if (!s->is_initial)
>   		status_printf_ln(s, c, _("  (use \"git reset %s <file>...\" to unstage)"), s->reference);
>   	else
>   		status_printf_ln(s, c, _("  (use \"git rm --cached <file>...\" to unstage)"));
> +conclude:
>   	status_printf_ln(s, c, "%s", "");
>   }
>   
> @@ -220,7 +222,7 @@ static void wt_longstatus_print_dirty_header(struct wt_status *s,
>   
>   	status_printf_ln(s, c, _("Changes not staged for commit:"));
>   	if (!s->hints)
> -		return;
> +		goto conclude;
>   	if (!has_deleted)
>   		status_printf_ln(s, c, _("  (use \"git add <file>...\" to update what will be committed)"));
>   	else
> @@ -228,6 +230,7 @@ static void wt_longstatus_print_dirty_header(struct wt_status *s,
>   	status_printf_ln(s, c, _("  (use \"git checkout -- <file>...\" to discard changes in working directory)"));
>   	if (has_dirty_submodules)
>   		status_printf_ln(s, c, _("  (commit or discard the untracked or modified content in submodules)"));
> +conclude:
>   	status_printf_ln(s, c, "%s", "");
>   }
>   
> @@ -238,8 +241,9 @@ static void wt_longstatus_print_other_header(struct wt_status *s,
>   	const char *c = color(WT_STATUS_HEADER, s);
>   	status_printf_ln(s, c, "%s:", what);
>   	if (!s->hints)
> -		return;
> +		goto conclude;
>   	status_printf_ln(s, c, _("  (use \"git %s <file>...\" to include in what will be committed)"), how);
> +conclude:
>   	status_printf_ln(s, c, "%s", "");
>   }
>   
> 

>
> Hi John
>
> On 30/04/2019 07:02, John Lin wrote:
> > When typing "git status", there is an empty line between
> > the "Changes not staged for commit:" block and the list
> > of changed files.
>
> I'm a bit confused by this as you change a status test below by
> inserting these blank lines into the expected output, implying they are
> not there now. I think maybe the blank line is only shown when status
> prints advice.
>
> > However, when typing "git commit" with
> > no files added, there are no empty lines between them.
>
> I have to say looking at the changes to the output I prefer the
> original, the lists are nicely indented so there is no need for a blank
> line to separate the header from the list and having the header
> immediately before the list means the blank line at the end of the block
> makes the extent of the block clear. It also saves screen space which is
> useful for small laptop screens. I can see why one might want a blank
> line to separate the advice and list of changes (though even there the
> indention of the list and advice is different) but for a one line header
> I think it is better to start the list on the next line.

>
> Best Wishes
>
> Phillip
>
> > This patch adds empty lines in the above case and some
> > similar cases.
> >
> > Signed-off-by: John Lin <johnlinp@gmail.com>
> > ---
> >   t/t7500-commit-template-squash-signoff.sh |  1 +
> >   t/t7508-status.sh                         |  5 +++++
> >   t/t7512-status-help.sh                    |  1 +
> >   wt-status.c                               | 12 ++++++++----
> >   4 files changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
> > index 46a5cd4b73..0423e77d1d 100755
> > --- a/t/t7500-commit-template-squash-signoff.sh
> > +++ b/t/t7500-commit-template-squash-signoff.sh
> > @@ -345,6 +345,7 @@ cat >expected-template <<EOF
> >   #
> >   # On branch commit-template-check
> >   # Changes to be committed:
> > +#
> >   #   new file:   commit-template-check
> >   #
> >   # Untracked files not listed
> > diff --git a/t/t7508-status.sh b/t/t7508-status.sh
> > index e1f11293e2..949b1dbcc4 100755
> > --- a/t/t7508-status.sh
> > +++ b/t/t7508-status.sh
> > @@ -204,12 +204,15 @@ Your branch and 'upstream' have diverged,
> >   and have 1 and 2 different commits each, respectively.
> >
> >   Changes to be committed:
> > +
> >       new file:   dir2/added
> >
> >   Changes not staged for commit:
> > +
> >       modified:   dir1/modified
> >
> >   Untracked files:
> > +
> >       dir1/untracked
> >       dir2/modified
> >       dir2/untracked
> > @@ -449,9 +452,11 @@ Your branch and '\''upstream'\'' have diverged,
> >   and have 1 and 2 different commits each, respectively.
> >
> >   Changes to be committed:
> > +
> >       new file:   dir2/added
> >
> >   Changes not staged for commit:
> > +
> >       modified:   dir1/modified
> >
> >   Untracked files not listed
> > diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
> > index 458608cc1e..0a29fa66a2 100755
> > --- a/t/t7512-status-help.sh
> > +++ b/t/t7512-status-help.sh
> > @@ -714,6 +714,7 @@ rebase in progress; onto $ONTO
> >   You are currently rebasing branch '\''statushints_disabled'\'' on '\''$ONTO'\''.
> >
> >   Unmerged paths:
> > +
> >       both modified:   main.txt
> >
> >   no changes added to commit
> > diff --git a/wt-status.c b/wt-status.c
> > index 445a36204a..0766e3ee12 100644
> > --- a/wt-status.c
> > +++ b/wt-status.c
> > @@ -175,7 +175,7 @@ static void wt_longstatus_print_unmerged_header(struct wt_status *s)
> >       }
> >
> >       if (!s->hints)
> > -             return;
> > +             goto conclude;
> >       if (s->whence != FROM_COMMIT)
> >               ;
> >       else if (!s->is_initial)
> > @@ -193,6 +193,7 @@ static void wt_longstatus_print_unmerged_header(struct wt_status *s)
> >       } else {
> >               status_printf_ln(s, c, _("  (use \"git add/rm <file>...\" as appropriate to mark resolution)"));
> >       }
> > +conclude:
> >       status_printf_ln(s, c, "%s", "");
> >   }
> >
> > @@ -202,13 +203,14 @@ static void wt_longstatus_print_cached_header(struct wt_status *s)
> >
> >       status_printf_ln(s, c, _("Changes to be committed:"));
> >       if (!s->hints)
> > -             return;
> > +             goto conclude;
> >       if (s->whence != FROM_COMMIT)
> >               ; /* NEEDSWORK: use "git reset --unresolve"??? */
> >       else if (!s->is_initial)
> >               status_printf_ln(s, c, _("  (use \"git reset %s <file>...\" to unstage)"), s->reference);
> >       else
> >               status_printf_ln(s, c, _("  (use \"git rm --cached <file>...\" to unstage)"));
> > +conclude:
> >       status_printf_ln(s, c, "%s", "");
> >   }
> >
> > @@ -220,7 +222,7 @@ static void wt_longstatus_print_dirty_header(struct wt_status *s,
> >
> >       status_printf_ln(s, c, _("Changes not staged for commit:"));
> >       if (!s->hints)
> > -             return;
> > +             goto conclude;
> >       if (!has_deleted)
> >               status_printf_ln(s, c, _("  (use \"git add <file>...\" to update what will be committed)"));
> >       else
> > @@ -228,6 +230,7 @@ static void wt_longstatus_print_dirty_header(struct wt_status *s,
> >       status_printf_ln(s, c, _("  (use \"git checkout -- <file>...\" to discard changes in working directory)"));
> >       if (has_dirty_submodules)
> >               status_printf_ln(s, c, _("  (commit or discard the untracked or modified content in submodules)"));
> > +conclude:
> >       status_printf_ln(s, c, "%s", "");
> >   }
> >
> > @@ -238,8 +241,9 @@ static void wt_longstatus_print_other_header(struct wt_status *s,
> >       const char *c = color(WT_STATUS_HEADER, s);
> >       status_printf_ln(s, c, "%s:", what);
> >       if (!s->hints)
> > -             return;
> > +             goto conclude;
> >       status_printf_ln(s, c, _("  (use \"git %s <file>...\" to include in what will be committed)"), how);
> > +conclude:
> >       status_printf_ln(s, c, "%s", "");
> >   }
> >
> >

> Hi John
> 
> On 30/04/2019 07:02, John Lin wrote:
> > When typing "git status", there is an empty line between
> > the "Changes not staged for commit:" block and the list
> > of changed files.
> 
> I'm a bit confused by this as you change a status test below by inserting
> these blank lines into the expected output, implying they are not there now.
> I think maybe the blank line is only shown when status prints advice.
> 
> > However, when typing "git commit" with
> > no files added, there are no empty lines between them.
> 
> I have to say looking at the changes to the output I prefer the original,
> the lists are nicely indented so there is no need for a blank line to
> separate the header from the list and having the header immediately before
> the list means the blank line at the end of the block makes the extent of
> the block clear. It also saves screen space which is useful for small laptop
> screens. I can see why one might want a blank line to separate the advice
> and list of changes (though even there the indention of the list and advice
> is different) but for a one line header I think it is better to start the
> list on the next line.

>
> On Tue, Apr 30, 2019 at 12:15:37PM +0100, Phillip Wood wrote:
> > Hi John
> >
> > On 30/04/2019 07:02, John Lin wrote:
> > > When typing "git status", there is an empty line between
> > > the "Changes not staged for commit:" block and the list
> > > of changed files.
> >
> > I'm a bit confused by this as you change a status test below by inserting
> > these blank lines into the expected output, implying they are not there now.
> > I think maybe the blank line is only shown when status prints advice.
> >
> > > However, when typing "git commit" with
> > > no files added, there are no empty lines between them.
> >
> > I have to say looking at the changes to the output I prefer the original,
> > the lists are nicely indented so there is no need for a blank line to
> > separate the header from the list and having the header immediately before
> > the list means the blank line at the end of the block makes the extent of
> > the block clear. It also saves screen space which is useful for small laptop
> > screens. I can see why one might want a blank line to separate the advice
> > and list of changes (though even there the indention of the list and advice
> > is different) but for a one line header I think it is better to start the
> > list on the next line.
>
> I actually was going to submit an equivalent patch eventually. The
> inconsistency between status and commit is bothersome to me and I think
> that adding the whitespace improves readability.
>
> I'd love to see this picked up.

>
> On Thu, May 02, 2019 at 08:35:23AM +0800, 林自均 wrote:
> > Thank you for the support! I'll be very happy if my patch can get into
> > the code base. Is there any improvement I can make for this patch?
>
> The patch seems sane to me. There's less duplicate code than the patch I
> started writing; the goto conclude seems to avoid most of it.

>
> Hi Brian,
>
> brian m. carlson <sandals@crustytoothpaste.net> 於 2019年5月3日 週五 上午7:15寫道：
> >
> > On Thu, May 02, 2019 at 08:35:23AM +0800, 林自均 wrote:
> > > Thank you for the support! I'll be very happy if my patch can get into
> > > the code base. Is there any improvement I can make for this patch?
> >
> > The patch seems sane to me. There's less duplicate code than the patch I
> > started writing; the goto conclude seems to avoid most of it.
>
> I see. Let's wait for more comments on my patch. Thank you.
>
> Best,
> John Lin

>
>
> > --
> > brian m. carlson: Houston, Texas, US
> > OpenPGP: https://keybase.io/bk2204

>
> 林自均 <johnlinp@gmail.com> writes:
>
> > Hi Junio,
> >> ...
> > Could you please give me some advice on my patch? Thank you.
>
> I tend to agree with what Phillip said in
> <ae1332b8-a227-e83a-8862-8811b6a81251@gmail.com>.
>
> If the difference between "status" and "commit" bothers you so much,
> i.e.
>
>     When typing "git status", there is an empty line between the
>     "Changes not staged for commit:" block and the list of changed
>     files. However, when typing "git commit" with no files added,
>     there are no empty lines between them.
>
> it may not be a bad idea to try making them consistent by removing
> the blank line that is given after the advice messages, perhaps?

> If the difference between "status" and "commit" bothers you so much,
> i.e.
> 
>     When typing "git status", there is an empty line between the
>     "Changes not staged for commit:" block and the list of changed
>     files. However, when typing "git commit" with no files added,
>     there are no empty lines between them.
> 
> it may not be a bad idea to try making them consistent by removing
> the blank line that is given after the advice messages, perhaps?

> On Mon, May 13, 2019 at 02:51:37PM +0900, Junio C Hamano wrote:
>> If the difference between "status" and "commit" bothers you so much,
>> i.e.
>> 
>>     When typing "git status", there is an empty line between the
>>     "Changes not staged for commit:" block and the list of changed
>>     files. However, when typing "git commit" with no files added,
>>     there are no empty lines between them.
>> 
>> it may not be a bad idea to try making them consistent by removing
>> the blank line that is given after the advice messages, perhaps?
>
> I personally think the extra blank line aids readability, especially on
> screens with small text, but I'll defer to your decision on this.

>
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> > On Mon, May 13, 2019 at 02:51:37PM +0900, Junio C Hamano wrote:
> >> If the difference between "status" and "commit" bothers you so much,
> >> i.e.
> >>
> >>     When typing "git status", there is an empty line between the
> >>     "Changes not staged for commit:" block and the list of changed
> >>     files. However, when typing "git commit" with no files added,
> >>     there are no empty lines between them.
> >>
> >> it may not be a bad idea to try making them consistent by removing
> >> the blank line that is given after the advice messages, perhaps?
> >
> > I personally think the extra blank line aids readability, especially on
> > screens with small text, but I'll defer to your decision on this.
>
> Heh, now we established that this is primarily of personal tastes,
> if you leave it up to me, my preference would be different from what
> was said in the thread so far.
>
>  - I do not mind having an extra blank line in the log message
>    editor session "git commit" gives me, primarily because at that
>    point I am in a full-screen editor that I can scroll up and down
>    at ease.

>
>  - "git status" output, on the other hand, is shown in the context
>    where vertical screen real estate is more precious (I do not say
>    "git -p status"); I'd probably be happier without these empty
>    lines.
>
> But following the above two would mean that the result will still
> leave difference between the commands; the original justificaiton
> will not apply to such a change.

> I was not talking about the messages in the editor session. I was
> talking about "git commit" without "git add" anything.
>
> For example:
>
> ```
> $ touch newfile.txt
> $ git commit
> On branch master
> Untracked files:
>         newfile.txt
>
> nothing added to commit but untracked files present
> ```
>
> My current patch is trying to add an empty line between
> "Untracked files:" and "newfile.txt".

>
> 林自均 <johnlinp@gmail.com> writes:
>
> > I was not talking about the messages in the editor session. I was
> > talking about "git commit" without "git add" anything.
> >
> > For example:
> >
> > ```
> > $ touch newfile.txt
> > $ git commit
> > On branch master
> > Untracked files:
> >         newfile.txt
> >
> > nothing added to commit but untracked files present
> > ```
> >
> > My current patch is trying to add an empty line between
> > "Untracked files:" and "newfile.txt".
>
> I do not think that one is paged, so if you ask me, I'd say we
> shouldn't add an extra blank there.  Is that message also reused in
> the editor session, or do two different codepaths produce a similar
> looking message, one for the above case direct to the terminal and
> the other for the editor session?

>
> But again...
>
> >> At the same time, I think I've been happy enough with the current
> >> output from both commands, so if you let me bikeshed freely, I'd
> >> probably pick "let's not change anything then" ;-)
>

>
> Hi Junio,
>
> Junio C Hamano <gitster@pobox.com> 於 2019年5月15日 週三 上午8:48寫道：
> >
> > 林自均 <johnlinp@gmail.com> writes:
> >
> > > I was not talking about the messages in the editor session. I was
> > > talking about "git commit" without "git add" anything.
> > >
> > > For example:
> > >
> > > ```
> > > $ touch newfile.txt
> > > $ git commit
> > > On branch master
> > > Untracked files:
> > >         newfile.txt
> > >
> > > nothing added to commit but untracked files present
> > > ```
> > >
> > > My current patch is trying to add an empty line between
> > > "Untracked files:" and "newfile.txt".
> >
> > I do not think that one is paged, so if you ask me, I'd say we
> > shouldn't add an extra blank there.  Is that message also reused in
> > the editor session, or do two different codepaths produce a similar
> > looking message, one for the above case direct to the terminal and
> > the other for the editor session?
>
> The messages produced in wt-status.c seem to be reused in
> both terminal and editor session. When I tried to modify the
> messages in terminal, the ones in editor session will also
> be modified accordingly.
>
> By the way, my new patch to remove extra blank line is here:
> https://github.com/gitgitgadget/git/pull/196
> I am still waiting for someone to comment "/allow johnlinp".

>
> >
> > But again...
> >
> > >> At the same time, I think I've been happy enough with the current
> > >> output from both commands, so if you let me bikeshed freely, I'd
> > >> probably pick "let's not change anything then" ;-)
> >