{"thread":{"id":"23550","subject":"[RFC/RFD] wt-status: take advice.statusHints seriously","startedAt":"2010-04-21T14:47:51Z","lastAt":"2010-04-26T19:10:56Z","messageCount":9,"participants":["Michael J Gruber","Junio C Hamano","Tay Ray Chuan","Jakub Narebski","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"140034","messageId":"b3168027365b78d47f5b95fc02f7853749075a33.1271861247.git.git@drmicha.warpmail.net","threadId":"23550","inReplyTo":null,"subject":"[RFC/RFD] wt-status: take advice.statusHints seriously","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-04-21T14:47:51Z","receivedAt":"2010-04-21T14:47:51Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, status gives a lot of hints even when advice.statusHints is\nfalse. Change this so that all hints depend on the config varaible.\n\nSigned-off-by: Michael J Gruber \n---\n wt-status.c | 12 ++++++------\n 1 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 8ca59a2..38076ee 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -625,7 +625,7 @@ void wt_status_print(struct wt_status *s)\n \tif (s->show_untracked_files)\n \t\twt_status_print_untracked(s);\n \telse if (s->commitable)\n-\t\t fprintf(s->fp, \"# Untracked files not listed (use -u option to show untracked files)\\n\");\n+\t\t fprintf(s->fp, \"# Untracked files not listed%s\\n\", advice_status_hints ? \" (use -u option to show untracked files)\" : \"\");\n \n \tif (s->verbose)\n \t\twt_status_print_verbose(s);\n@@ -635,15 +635,15 @@ void wt_status_print(struct wt_status *s)\n \t\telse if (s->nowarn)\n \t\t\t; /* nothing */\n \t\telse if (s->workdir_dirty)\n-\t\t\tprintf(\"no changes added to commit (use \\\"git add\\\" and/or \\\"git commit -a\\\")\\n\");\n+\t\t\tprintf(\"no changes added to commit%s\\n\", advice_status_hints ? \" (use \\\"git add\\\" and/or \\\"git commit -a\\\")\" : \"\");\n \t\telse if (s->untracked.nr)\n-\t\t\tprintf(\"nothing added to commit but untracked files present (use \\\"git add\\\" to track)\\n\");\n+\t\t\tprintf(\"nothing added to commit but untracked files present%s\\n\", advice_status_hints ? \" (use \\\"git add\\\" to track)\" : \"\");\n \t\telse if (s->is_initial)\n-\t\t\tprintf(\"nothing to commit (create/copy files and use \\\"git add\\\" to track)\\n\");\n+\t\t\tprintf(\"nothing to commit%s\\n\", advice_status_hints ? \" (create/copy files and use \\\"git add\\\" to track)\" : \"\");\n \t\telse if (!s->show_untracked_files)\n-\t\t\tprintf(\"nothing to commit (use -u to show untracked files)\\n\");\n+\t\t\tprintf(\"nothing to commit%s\\n\", advice_status_hints ? \" (use -u to show untracked files)\" : \"\");\n \t\telse\n-\t\t\tprintf(\"nothing to commit (working directory clean)\\n\");\n+\t\t\tprintf(\"nothing to commit%s\\n\", advice_status_hints ? \" (working directory clean)\" : \"\");\n \t}\n }\n \n-- \n1.7.1.rc1.248.gcefbb\n"},{"id":"140106","messageId":"7v39yo53mt.fsf@alter.siamese.dyndns.org","threadId":"23550","inReplyTo":"b3168027365b78d47f5b95fc02f7853749075a33.1271861247.git.git@drmicha.warpmail.net","subject":"Re: [RFC/RFD] wt-status: take advice.statusHints seriously","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-22T07:48:42Z","receivedAt":"2010-04-22T07:48:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Currently, status gives a lot of hints even when advice.statusHints is\n> false. Change this so that all hints depend on the config varaible.\n>\n> Signed-off-by: Michael J Gruber \n> ---\n> wt-status.c | 12 ++++++------\n> 1 files changed, 6 insertions(+), 6 deletions(-)\n>\n> diff --git a/wt-status.c b/wt-status.c\n> index 8ca59a2..38076ee 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -625,7 +625,7 @@ void wt_status_print(struct wt_status *s)\n> \tif (s->show_untracked_files)\n> \t\twt_status_print_untracked(s);\n> \telse if (s->commitable)\n> -\t\t fprintf(s->fp, \"# Untracked files not listed (use -u option to show untracked files)\\n\");\n> +\t\t fprintf(s->fp, \"# Untracked files not listed%s\\n\", advice_status_hints ? \" (use -u option to show untracked files)\" : \"\");\n\nThis is a good change, but because you are losing the grep-ability with\nthis patch (i.e. \"grep -e 'not listed (use -u'\" no longer works), I'd\nprefer to see this long line split into:\n\n\t\t fprintf(s->fp, \"# Untracked files not listed%s\\n\",\n\t\t\tadvice_status_hints\n\t\t\t? \" (use -u option to show untracked files)\" : \"\");\n\nLikewise for all the other changes. \n\nThanks.\n"},{"id":"140156","messageId":"c7003ec93f4ea01be80fbba08d2ed5544fa0df4d.1271968072.git.git@drmicha.warpmail.net","threadId":"23550","inReplyTo":"7v39yo53mt.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/2] t7508: test advice.statusHints","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-04-22T20:30:19Z","receivedAt":"2010-04-22T20:30:19Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"edf563f (status: make \"how to stage\" messages optional, 2009-09-09)\nintroduced advice.statusHints without tests. Add a few tests to describe\nand test the status quo.\n\nSigned-off-by: Michael J Gruber \n---\n t/t7508-status.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 45 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex 556d0fa..7409a06 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -69,6 +69,34 @@ test_expect_success 'status (2)' '\n '\n \n cat >expect <<\\EOF\n+# On branch master\n+# Changes to be committed:\n+#\tnew file: dir2/added\n+#\n+# Changed but not updated:\n+#\tmodified: dir1/modified\n+#\n+# Untracked files:\n+#\tdir1/untracked\n+#\tdir2/modified\n+#\tdir2/untracked\n+#\texpect\n+#\toutput\n+#\tuntracked\n+EOF\n+\n+git config advice.statusHints false\n+\n+test_expect_success 'status (advice.statusHints false)' '\n+\n+\tgit status >output &&\n+\ttest_cmp expect output\n+\n+'\n+\n+git config --unset advice.statusHints\n+\n+cat >expect <<\\EOF\n M dir1/modified\n A dir2/added\n ?? dir1/untracked\n@@ -115,6 +143,23 @@ test_expect_success 'status (status.showUntrackedFiles no)' '\n \ttest_cmp expect output\n '\n \n+cat >expect <output &&\n+\ttest_cmp expect output\n+'\n+git config --unset advice.statusHints\n+\n cat >expect << EOF\n M dir1/modified\n A dir2/added\n-- \n1.7.1.rc1.248.gcefbb\n"},{"id":"140155","messageId":"86a4d8e5df80008c5c2b3ff4f2a5842ad1f58dc4.1271968072.git.git@drmicha.warpmail.net","threadId":"23550","inReplyTo":"7v39yo53mt.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/2] wt-status: take advice.statusHints seriously","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-04-22T20:30:20Z","receivedAt":"2010-04-22T20:30:20Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, status gives a lot of hints even when advice.statusHints is\nfalse. Change this so that all hints depend on the config variable.\n\nSigned-off-by: Michael J Gruber \n---\n t/t7508-status.sh | 2 +-\n wt-status.c | 21 +++++++++++++++------\n 2 files changed, 16 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex 7409a06..1301ec8 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -151,7 +151,7 @@ cat >expect <show_untracked_files)\n \t\twt_status_print_untracked(s);\n \telse if (s->commitable)\n-\t\t fprintf(s->fp, \"# Untracked files not listed (use -u option to show untracked files)\\n\");\n+\t\tfprintf(s->fp, \"# Untracked files not listed%s\\n\",\n+\t\t\tadvice_status_hints\n+\t\t\t? \" (use -u option to show untracked files)\" : \"\");\n \n \tif (s->verbose)\n \t\twt_status_print_verbose(s);\n@@ -635,15 +637,22 @@ void wt_status_print(struct wt_status *s)\n \t\telse if (s->nowarn)\n \t\t\t; /* nothing */\n \t\telse if (s->workdir_dirty)\n-\t\t\tprintf(\"no changes added to commit (use \\\"git add\\\" and/or \\\"git commit -a\\\")\\n\");\n+\t\t\tprintf(\"no changes added to commit%s\\n\",\n+\t\t\t\tadvice_status_hints\n+\t\t\t\t? \" (use \\\"git add\\\" and/or \\\"git commit -a\\\")\" : \"\");\n \t\telse if (s->untracked.nr)\n-\t\t\tprintf(\"nothing added to commit but untracked files present (use \\\"git add\\\" to track)\\n\");\n+\t\t\tprintf(\"nothing added to commit but untracked files present%s\\n\",\n+\t\t\t\tadvice_status_hints\n+\t\t\t\t? \" (use \\\"git add\\\" to track)\" : \"\");\n \t\telse if (s->is_initial)\n-\t\t\tprintf(\"nothing to commit (create/copy files and use \\\"git add\\\" to track)\\n\");\n+\t\t\tprintf(\"nothing to commit%s\\n\", advice_status_hints\n+\t\t\t\t? \" (create/copy files and use \\\"git add\\\" to track)\" : \"\");\n \t\telse if (!s->show_untracked_files)\n-\t\t\tprintf(\"nothing to commit (use -u to show untracked files)\\n\");\n+\t\t\tprintf(\"nothing to commit%s\\n\", advice_status_hints\n+\t\t\t\t? \" (use -u to show untracked files)\" : \"\");\n \t\telse\n-\t\t\tprintf(\"nothing to commit (working directory clean)\\n\");\n+\t\t\tprintf(\"nothing to commit%s\\n\", advice_status_hints\n+\t\t\t\t? \" (working directory clean)\" : \"\");\n \t}\n }\n \n-- \n1.7.1.rc1.248.gcefbb\n"},{"id":"140184","messageId":"q2ibe6fef0d1004222115gcdd8fd81xce8993508bfa9f5d@mail.gmail.com","threadId":"23550","inReplyTo":"86a4d8e5df80008c5c2b3ff4f2a5842ad1f58dc4.1271968072.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 2/2] wt-status: take advice.statusHints seriously","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-04-23T04:15:56Z","receivedAt":"2010-04-23T04:15:56Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Fri, Apr 23, 2010 at 4:30 AM, Michael J Gruber\n wrote:\n>                else if (s->untracked.nr)\n> -                       printf(\"nothing added to commit but untracked files present (use \\\"git add\\\" to track)\\n\");\n> +                       printf(\"nothing added to commit but untracked files present%s\\n\",\n> +                               advice_status_hints\n> +                               ? \" (use \\\"git add\\\" to track)\" : \"\");\n\nwhile we're at it, perhaps we could put the hints on its own line,\nwith a \"hint: \" prefix:\n\n nothing added to commit but untracked files present\n hint: use \"git add\" to track\n\nThis way, we give future git usability hackers the space to elaborate\nfurther on why a certain flag or command was recommended.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"140193","messageId":"4BD155D0.5080402@drmicha.warpmail.net","threadId":"23550","inReplyTo":"q2ibe6fef0d1004222115gcdd8fd81xce8993508bfa9f5d@mail.gmail.com","subject":"Re: [PATCH 2/2] wt-status: take advice.statusHints seriously","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-04-23T08:09:52Z","receivedAt":"2010-04-23T08:09:52Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Tay Ray Chuan venit, vidit, dixit 23.04.2010 06:15:\n> Hi,\n> \n> On Fri, Apr 23, 2010 at 4:30 AM, Michael J Gruber\n> wrote:\n>> else if (s->untracked.nr)\n>> - printf(\"nothing added to commit but untracked files present (use \\\"git add\\\" to track)\\n\");\n>> + printf(\"nothing added to commit but untracked files present%s\\n\",\n>> + advice_status_hints\n>> + ? \" (use \\\"git add\\\" to track)\" : \"\");\n> \n> while we're at it, perhaps we could put the hints on its own line,\n\nI don't think I'm the only one to turn sour at every encounter with the\nphrase \"while we're at it\". I did fix one extraneous space in code\n\"while I was at it\", yes, but:\n\n> with a \"hint: \" prefix:\n> \n> nothing added to commit but untracked files present\n> hint: use \"git add\" to track\n> \n> This way, we give future git usability hackers the space to elaborate\n> further on why a certain flag or command was recommended.\n> \n\nI'm suggesting a change in (output) behaviour (hint vs. no hint), which\none may even consider to be a bug fix in terms of matching the obvious\nexpections related to advice.statusHints false.\n\nYou're suggesting a different presentation of the output. In fact, I\nnoticed inconsistent capitalisation in the hints which should be\nadjusted, maybe together with the presentation.\n\nBut I deem both changes (overlapping though) unrelated.\n\nCheers,\nMichael\n"},{"id":"140231","messageId":"m3aasudnzy.fsf@localhost.localdomain","threadId":"23550","inReplyTo":"4BD155D0.5080402@drmicha.warpmail.net","subject":"Re: [PATCH 2/2] wt-status: take advice.statusHints seriously","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-04-23T18:31:55Z","receivedAt":"2010-04-23T18:31:55Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Michael J Gruber writes:\n> Tay Ray Chuan venit, vidit, dixit 23.04.2010 06:15:\n>> \n>> On Fri, Apr 23, 2010 at 4:30 AM, Michael J Gruber\n>> wrote:\n>>> else if (s->untracked.nr)\n>>> - printf(\"nothing added to commit but untracked files present (use \\\"git add\\\" to track)\\n\");\n>>> + printf(\"nothing added to commit but untracked files present%s\\n\",\n>>> + advice_status_hints\n>>> + ? \" (use \\\"git add\\\" to track)\" : \"\");\n>> \n>> while we're at it, perhaps we could put the hints on its own line,\n> \n> I don't think I'm the only one to turn sour at every encounter with the\n> phrase \"while we're at it\". I did fix one extraneous space in code\n> \"while I was at it\", yes, but:\n\nI agre that it might be a good idea, but it is not something to do\n\"while we're at it\".\n \nTay Ray Chuan probably meant that he \"while at it\" (while reviewing\nthis patch) had this idea...\n\n>> with a \"hint: \" prefix:\n>> \n>> nothing added to commit but untracked files present\n>> hint: use \"git add\" to track\n>> \n>> This way, we give future git usability hackers the space to elaborate\n>> further on why a certain flag or command was recommended.\n>> \n> \n> I'm suggesting a change in (output) behaviour (hint vs. no hint), which\n> one may even consider to be a bug fix in terms of matching the obvious\n> expections related to advice.statusHints false.\n> \n> You're suggesting a different presentation of the output. In fact, I\n> noticed inconsistent capitalisation in the hints which should be\n> adjusted, maybe together with the presentation.\n> \n> But I deem both changes (overlapping though) unrelated.\n\nPerhaps git should introduce helper commands, similar to 'die', 'warn'\nand 'error', but for hints and advices, e.g. hint(bool, \"\")\nwhich would print advice on its own line (like 'die' and 'warn') with\nappropriate prefix if advices, and e.g. 'advice', which would return\neither empty string or string with advice (a bit like 'error').\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"140446","messageId":"20100426191012.GA10772@coredump.intra.peff.net","threadId":"23550","inReplyTo":"86a4d8e5df80008c5c2b3ff4f2a5842ad1f58dc4.1271968072.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 2/2] wt-status: take advice.statusHints seriously","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-26T19:10:12Z","receivedAt":"2010-04-26T19:10:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 22, 2010 at 10:30:20PM +0200, Michael J Gruber wrote:\n\n> Currently, status gives a lot of hints even when advice.statusHints is\n> false. Change this so that all hints depend on the config variable.\n\nI didn't bother with them because they aren't actually wasting screen\nreal estate, unlike the other hints. But I don't really care either way,\nso it's fine by me.\n\nThough I wonder if you should simply suppress the line entirely. It is\ntotally redundant with the information presented above. But like I said,\nI don't care too much.\n\n-Peff\n"},{"id":"140447","messageId":"20100426191056.GB10772@coredump.intra.peff.net","threadId":"23550","inReplyTo":"4BD155D0.5080402@drmicha.warpmail.net","subject":"Re: [PATCH 2/2] wt-status: take advice.statusHints seriously","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-26T19:10:56Z","receivedAt":"2010-04-26T19:10:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 23, 2010 at 10:09:52AM +0200, Michael J Gruber wrote:\n\n> Tay Ray Chuan venit, vidit, dixit 23.04.2010 06:15:\n> > Hi,\n> > \n> > On Fri, Apr 23, 2010 at 4:30 AM, Michael J Gruber\n> > wrote:\n> >> else if (s->untracked.nr)\n> >> - printf(\"nothing added to commit but untracked files present (use \\\"git add\\\" to track)\\n\");\n> >> + printf(\"nothing added to commit but untracked files present%s\\n\",\n> >> + advice_status_hints\n> >> + ? \" (use \\\"git add\\\" to track)\" : \"\");\n> > \n> > while we're at it, perhaps we could put the hints on its own line,\n> \n> I don't think I'm the only one to turn sour at every encounter with the\n> phrase \"while we're at it\". I did fix one extraneous space in code\n> \"while I was at it\", yes, but:\n\nJust my 2 cents, but I find \"while we're at it\" comments are taken much\nbetter when accompanied by a patch (either squashable or to be applied\non top). :)\n\n-Peff\n"}]}