Home » Mailing list archives » git » 2010 » April » 23

Re: [PATCH 2/2] wt-status: take advice.statusHints seriously

Previous thread: [PATCH] Make git log --follow support --find-copies-harder. by Bo Yang on Wednesday, April 21, 2010 - 7:37 am. (4 messages)

Next thread: Git-SVN on Cygwin 1.7: git svn rebase b0rks index if not run from top-level directory by Matthias Andree on Wednesday, April 21, 2010 - 9:56 am. (5 messages)
[view original message]
From: Michael J Gruber
Subject: [RFC/RFD] wt-status: take advice.statusHints seriously
Date: Wednesday, April 21, 2010 - 7:47 am

Currently, status gives a lot of hints even when advice.statusHints is
false. Change this so that all hints depend on the config varaible.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 wt-status.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 8ca59a2..38076ee 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -625,7 +625,7 @@ void wt_status_print(struct wt_status *s)
 	if (s->show_untracked_files)
 		wt_status_print_untracked(s);
 	else if (s->commitable)
-		 fprintf(s->fp, "# Untracked files not listed (use -u option to show untracked files)\n");
+		 fprintf(s->fp, "# Untracked files not listed%s\n", advice_status_hints ? " (use -u option to show untracked files)" : "");
 
 	if (s->verbose)
 		wt_status_print_verbose(s);
@@ -635,15 +635,15 @@ void wt_status_print(struct wt_status *s)
 		else if (s->nowarn)
 			; /* nothing */
 		else if (s->workdir_dirty)
-			printf("no changes added to commit (use \"git add\" and/or \"git commit -a\")\n");
+			printf("no changes added to commit%s\n", advice_status_hints ? " (use \"git add\" and/or \"git commit -a\")" : "");
 		else if (s->untracked.nr)
-			printf("nothing added to commit but untracked files present (use \"git add\" to track)\n");
+			printf("nothing added to commit but untracked files present%s\n", advice_status_hints ? " (use \"git add\" to track)" : "");
 		else if (s->is_initial)
-			printf("nothing to commit (create/copy files and use \"git add\" to track)\n");
+			printf("nothing to commit%s\n", advice_status_hints ? " (create/copy files and use \"git add\" to track)" : "");
 		else if (!s->show_untracked_files)
-			printf("nothing to commit (use -u to show untracked files)\n");
+			printf("nothing to commit%s\n", advice_status_hints ? " (use -u to show untracked files)" : "");
 		else
-			printf("nothing to commit (working directory clean)\n");
+			printf("nothing to commit%s\n", advice_status_hints ? " (working directory clean)" : ...
[ message continues ]
[view original message]
From: Junio C Hamano
Subject: Re: [RFC/RFD] wt-status: take advice.statusHints seriously
Date: Thursday, April 22, 2010 - 12:48 am

This is a good change, but because you are losing the grep-ability with
this patch (i.e. "grep -e 'not listed (use -u'" no longer works), I'd
prefer to see this long line split into:

		 fprintf(s->fp, "# Untracked files not listed%s\n",
			advice_status_hints
			? " (use -u option to show untracked files)" : "");

Likewise for all the other changes.                        

Thanks.
--


[ message continues ]
[view original message]
From: Michael J Gruber
Subject: [PATCH 1/2] t7508: test advice.statusHints
Date: Thursday, April 22, 2010 - 1:30 pm

edf563f (status: make "how to stage" messages optional, 2009-09-09)
introduced advice.statusHints without tests. Add a few tests to describe
and test the status quo.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t7508-status.sh |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 556d0fa..7409a06 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -69,6 +69,34 @@ test_expect_success 'status (2)' '
 '
 
 cat >expect <<\EOF
+# On branch master
+# Changes to be committed:
+#	new file:   dir2/added
+#
+# Changed but not updated:
+#	modified:   dir1/modified
+#
+# Untracked files:
+#	dir1/untracked
+#	dir2/modified
+#	dir2/untracked
+#	expect
+#	output
+#	untracked
+EOF
+
+git config advice.statusHints false
+
+test_expect_success 'status (advice.statusHints false)' '
+
+	git status >output &&
+	test_cmp expect output
+
+'
+
+git config --unset advice.statusHints
+
+cat >expect <<\EOF
  M dir1/modified
 A  dir2/added
 ?? dir1/untracked
@@ -115,6 +143,23 @@ test_expect_success 'status (status.showUntrackedFiles no)' '
 	test_cmp expect output
 '
 
+cat >expect <<EOF
+# On branch master
+# Changes to be committed:
+#	new file:   dir2/added
+#
+# Changed but not updated:
+#	modified:   dir1/modified
+#
+# Untracked files not listed (use -u option to show untracked files)
+EOF
+git config advice.statusHints false
+test_expect_success 'status -uno (advice.statusHints false)' '
+	git status -uno >output &&
+	test_cmp expect output
+'
+git config --unset advice.statusHints
+
 cat >expect << EOF
  M dir1/modified
 A  dir2/added
-- 
1.7.1.rc1.248.gcefbb

--


[ message continues ]
[view original message]
From: Michael J Gruber
Subject: [PATCH 2/2] wt-status: take advice.statusHints seriously
Date: Thursday, April 22, 2010 - 1:30 pm

Currently, status gives a lot of hints even when advice.statusHints is
false. Change this so that all hints depend on the config variable.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t7508-status.sh |    2 +-
 wt-status.c       |   21 +++++++++++++++------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 7409a06..1301ec8 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -151,7 +151,7 @@ cat >expect <<EOF
 # Changed but not updated:
 #	modified:   dir1/modified
 #
-# Untracked files not listed (use -u option to show untracked files)
+# Untracked files not listed
 EOF
 git config advice.statusHints false
 test_expect_success 'status -uno (advice.statusHints false)' '
diff --git a/wt-status.c b/wt-status.c
index 8ca59a2..d44486c 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -625,7 +625,9 @@ void wt_status_print(struct wt_status *s)
 	if (s->show_untracked_files)
 		wt_status_print_untracked(s);
 	else if (s->commitable)
-		 fprintf(s->fp, "# Untracked files not listed (use -u option to show untracked files)\n");
+		fprintf(s->fp, "# Untracked files not listed%s\n",
+			advice_status_hints
+			? " (use -u option to show untracked files)" : "");
 
 	if (s->verbose)
 		wt_status_print_verbose(s);
@@ -635,15 +637,22 @@ void wt_status_print(struct wt_status *s)
 		else if (s->nowarn)
 			; /* nothing */
 		else if (s->workdir_dirty)
-			printf("no changes added to commit (use \"git add\" and/or \"git commit -a\")\n");
+			printf("no changes added to commit%s\n",
+				advice_status_hints
+				? " (use \"git add\" and/or \"git commit -a\")" : "");
 		else if (s->untracked.nr)
-			printf("nothing added to commit but untracked files present (use \"git add\" to track)\n");
+			printf("nothing added to commit but untracked files present%s\n",
+				advice_status_hints
+				? " (use \"git add\" to track)" : "");
 		else if (s->is_initial)
-			printf("nothing to commit (create/copy ...
[ message continues ]
[view original message]
From: Tay Ray Chuan
Subject: Re: [PATCH 2/2] wt-status: take advice.statusHints seriously
Date: Thursday, April 22, 2010 - 9:15 pm

Hi,

On Fri, Apr 23, 2010 at 4:30 AM, Michael J Gruber

while we're at it, perhaps we could put the hints on its own line,
with a "hint: " prefix:

  nothing added to commit but untracked files present
  hint: use "git add" to track

This way, we give future git usability hackers the space to elaborate
further on why a certain flag or command was recommended.

-- 
Cheers,
Ray Chuan
--


[ message continues ]
[view original message]
From: Michael J Gruber
Subject: Re: [PATCH 2/2] wt-status: take advice.statusHints seriously
Date: Friday, April 23, 2010 - 1:09 am

I don't think I'm the only one to turn sour at every encounter with the
phrase "while we're at it". I did fix one extraneous space in code

I'm suggesting a change in (output) behaviour (hint vs. no hint), which
one may even consider to be a bug fix in terms of matching the obvious
expections related to advice.statusHints false.

You're suggesting a different presentation of the output. In fact, I
noticed inconsistent capitalisation in the hints which should be
adjusted, maybe together with the presentation.

But I deem both changes (overlapping though) unrelated.

Cheers,
Michael
--


[ message continues ]
[view original message]
From: Jakub Narebski
Subject: Re: [PATCH 2/2] wt-status: take advice.statusHints seriously
Date: Friday, April 23, 2010 - 11:31 am

I agre that it might be a good idea, but it is not something to do
"while we're at it".
 
Tay Ray Chuan probably meant that he "while at it" (while reviewing

Perhaps git should introduce helper commands, similar to 'die', 'warn'
and 'error', but for hints and advices, e.g. hint(bool, "<string>")
which would print advice on its own line (like 'die' and 'warn') with
appropriate prefix if advices, and e.g. 'advice', which would return
either empty string or string with advice (a bit like 'error').

-- 
Jakub Narebski
Poland
ShadeHawk on #git
--


[ message continues ]
[view original message]
From: Jeff King
Subject: Re: [PATCH 2/2] wt-status: take advice.statusHints seriously
Date: Monday, April 26, 2010 - 12:10 pm

Just my 2 cents, but I find "while we're at it" comments are taken much
better when accompanied by a patch (either squashable or to be applied
on top). :)

-Peff
--


[ message continues ]
[view original message]
From: Jeff King
Subject: Re: [PATCH 2/2] wt-status: take advice.statusHints seriously
Date: Monday, April 26, 2010 - 12:10 pm

I didn't bother with them because they aren't actually wasting screen
real estate, unlike the other hints. But I don't really care either way,
so it's fine by me.

Though I wonder if you should simply suppress the line entirely. It is
totally redundant with the information presented above. But like I said,
I don't care too much.

-Peff
--


[ message continues ]
Previous thread: [PATCH] Make git log --follow support --find-copies-harder. by Bo Yang on Wednesday, April 21, 2010 - 7:37 am. (4 messages)

Next thread: Git-SVN on Cygwin 1.7: git svn rebase b0rks index if not run from top-level directory by Matthias Andree on Wednesday, April 21, 2010 - 9:56 am. (5 messages)