[PATCH v2] status: add an empty line when there is no hint

[PATCH v2] status: add an empty line when there is no hint

[John Lin]

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.

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", "");
 }
 
-- 
2.21.0

Re: [PATCH v2] status: add an empty line when there is no hint

[Phillip Wood]

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", "");
>   }
>   
> 

Re: [PATCH v2] status: add an empty line when there is no hint

[林自均]

Hi Phillip,

Phillip Wood <phillip.wood123@gmail.com> 於 2019年4月30日 週二 下午7:15寫道：
>
> 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.

Thanks for the review. Your opinion does make sense to me. Please
consider this patch dropped. Thank you.

Best,
John Lin

>
> 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", "");
> >   }
> >
> >

Re: [PATCH v2] status: add an empty line when there is no hint

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]

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.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH v2] status: add an empty line when there is no hint

[林自均]

Hi Brian,

brian m. carlson <sandals@crustytoothpaste.net> 於 2019年5月2日 週四 上午7:45寫道：
>
> 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.

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?

Best,
John Lin

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

Re: [PATCH v2] status: add an empty line when there is no hint

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]

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.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH v2] status: add an empty line when there is no hint

[林自均]

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

Re: [PATCH v2] status: add an empty line when there is no hint

[林自均]

Hi Junio,

林自均 <johnlinp@gmail.com> 於 2019年5月3日 週五 下午12:15寫道：
>
> 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

Could you please give me some advice on my patch? Thank you.

Best,
John Lin

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

Re: [PATCH v2] status: add an empty line when there is no hint

[Junio C Hamano]

林自均 <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?

Re: [PATCH v2] status: add an empty line when there is no hint

[林自均]

Hi Junio,

Junio C Hamano <gitster@pobox.com> 於 2019年5月13日 週一 下午1:51寫道：
>
> 林自均 <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?

Thank you for the review. I guess removing the blank line also work
for me. I'll submit another patch.

Best,
John Lin

Re: [PATCH v2] status: add an empty line when there is no hint

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

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: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH v2] status: add an empty line when there is no hint

[Junio C Hamano]

"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.

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" ;-)

Re: [PATCH v2] status: add an empty line when there is no hint

[林自均]

Hi Junio,

Junio C Hamano <gitster@pobox.com> 於 2019年5月14日 週二 下午3:33寫道：
>
> "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.

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".

>
>  - "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.

With my above clarification, I guess your two use cases will be the
same: the messages will appear on the console screen. That leads
to a conclusion: we should save vertical screen space. I am working
on a new patch to achieve that.

>
> 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" ;-)

Best,
John Lin

Re: [PATCH v2] status: add an empty line when there is no hint

[Junio C Hamano]

林自均 <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" ;-)

Re: [PATCH v2] status: add an empty line when there is no hint

[林自均]

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" ;-)
>

Re: [PATCH v2] status: add an empty line when there is no hint

[林自均]

Hi Junio,

林自均 <johnlinp@gmail.com> 於 2019年5月15日 週三 上午11:02寫道：
>
> 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".

I've submitted the patch with the title
[PATCH 1/1] status: remove the empty line after hints
Please review if you are available. Thank you.

Best,
John Lin

>
> >
> > 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" ;-)
> >