{"thread":{"id":"46630","subject":"[PATCH 0/1] Add stash entry count summary to short status output","startedAt":"2017-08-20T04:55:12Z","lastAt":"2017-09-01T03:11:09Z","messageCount":7,"participants":["Sonny Michaud","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"326793","messageId":"c3ac0335-4f41-40d9-00ee-2a4e115bcf7a@gmail.com","threadId":"46630","inReplyTo":null,"subject":"[PATCH 0/1] Add stash entry count summary to short status output","fromName":"Sonny Michaud","fromEmail":"michaud.sonny@gmail.com","sentAt":"2017-08-20T04:53:04Z","receivedAt":"2017-08-20T04:55:12Z","isPatch":true,"sender":{"key":"michaud.sonny@gmail.com","avatar":null},"body":"this patch adds a header in the same style as the one provided by the --branch option when the user supplies both --show-stash and --short.  My attempt at creating a new test is broken[1], and I assume my changes could (and, likely need to) be improved.  Any assistance would be greatly appreciated; I just wanted to get started by hacking first and asking questions later!                         \n                                                                               \nThanks,                                                                        \nSonny                                                                          \n                                                                               \n1. https://travis-ci.org/sonnym/git/builds/266428128\n\n--\nFrom 9e9ffca5c4ed7dda34cad416c3eb68dc94a78b7e Mon Sep 17 00:00:00 2001 \nFrom: Sonny Michaud \nDate: Sat, 19 Aug 2017 23:46:15 -0400 \nSubject: [PATCH 1/1] status: learn to show stash in short output \n\nThis patch extends the functionality of the recently introduced \n--show-stash option to the status command, providing a header similar to \nthe one displayed when using the --branch option. \n--- \n t/t7508-status.sh | 5 +++++ \n wt-status.c | 12 ++++++++++++ \n 2 files changed, 17 insertions(+) \n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh \nindex 43d19a9b2..734001bc6 100755 \n--- a/t/t7508-status.sh \n+++ b/t/t7508-status.sh \n@@ -1619,6 +1619,11 @@ test_expect_success 'show stash info with \"--show-stash\"' ' \n test_i18ngrep \"^Your stash currently has 1 entry$\" expected_with_stash \n ' \n \n+test_expect_success 'show stash info with \"--show-stash\" and \"--short\"' ' \n+ git status --show-stash --short >expected_with_stash && \n+ test_i18ngrep \"Stash entries: 2\" expected_with_stash \n+' \n+ \n test_expect_success 'no stash info with \"--show-stash --no-show-stash\"' ' \n git status --show-stash --no-show-stash >expected_without_stash && \n test_cmp expected_default expected_without_stash \ndiff --git a/wt-status.c b/wt-status.c \nindex 77c27c511..651bb01f0 100644 \n--- a/wt-status.c \n+++ b/wt-status.c \n@@ -1827,6 +1827,15 @@ static void wt_shortstatus_print_tracking(struct wt_status *s) \n fputc(s->null_termination ? '\\0' : '\\n', s->fp); \n } \n \n+static void wt_shortstatus_print_stash_summary(struct wt_status *s) \n+{ \n+ int stash_count = 0; \n+ \n+ for_each_reflog_ent(\"refs/stash\", stash_count_refs, &stash_count); \n+ if (stash_count > 0) \n+ color_fprintf(s->fp, color(WT_STATUS_HEADER, s), \"## Stash entries: %d\", stash_count); \n+} \n+ \n static void wt_shortstatus_print(struct wt_status *s) \n { \n struct string_list_item *it; \n@@ -1834,6 +1843,9 @@ static void wt_shortstatus_print(struct wt_status *s) \n if (s->show_branch) \n wt_shortstatus_print_tracking(s); \n \n+ if (s->show_stash) \n+ wt_shortstatus_print_stash_summary(s); \n+ \n for_each_string_list_item(it, &s->change) { \n struct wt_status_change_data *d = it->util; \n \n-- \n2.14.0\n"},{"id":"326932","messageId":"cd00912b-5b66-f299-87bf-52aa19d90270@gmail.com","threadId":"46630","inReplyTo":"c3ac0335-4f41-40d9-00ee-2a4e115bcf7a@gmail.com","subject":"Re: [PATCH 0/1] Add stash entry count summary to short status output","fromName":"Sonny Michaud","fromEmail":"michaud.sonny@gmail.com","sentAt":"2017-08-22T15:25:51Z","receivedAt":"2017-08-22T15:26:03Z","isPatch":true,"sender":{"key":"michaud.sonny@gmail.com","avatar":null},"body":"I am just bumping this thread; I presume there is something amiss with \nmy submissions, so if someone can let me know how to fix any issues, I \nwill gladly re-submit the patch.\n\nThanks!\n\n- Sonny\n"},{"id":"327135","messageId":"45c23aab-bf20-d234-1e08-e6f0358f1779@gmail.com","threadId":"46630","inReplyTo":"cd00912b-5b66-f299-87bf-52aa19d90270@gmail.com","subject":"Re: [PATCH 0/1] Add stash entry count summary to short status output","fromName":"Sonny Michaud","fromEmail":"michaud.sonny@gmail.com","sentAt":"2017-08-24T15:29:50Z","receivedAt":"2017-08-24T15:29:58Z","isPatch":true,"sender":{"key":"michaud.sonny@gmail.com","avatar":null},"body":"On 08/22/2017 11:25 AM, Sonny Michaud wrote:\n> I am just bumping this thread; I presume there is something amiss with \n> my submissions, so if someone can let me know how to fix any issues, I \n> will gladly re-submit the patch.\n>\n> Thanks!\n>\n> - Sonny\n\nBumping again for visibility.\n"},{"id":"327148","messageId":"xmqq8ti8wyq7.fsf@gitster.mtv.corp.google.com","threadId":"46630","inReplyTo":"45c23aab-bf20-d234-1e08-e6f0358f1779@gmail.com","subject":"Re: [PATCH 0/1] Add stash entry count summary to short status output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-24T19:29:52Z","receivedAt":"2017-08-24T19:30:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sonny Michaud writes:\n\n> On 08/22/2017 11:25 AM, Sonny Michaud wrote:\n>> I am just bumping this thread; I presume there is something amiss\n>> with my submissions, so if someone can let me know how to fix any\n>> issues, I will gladly re-submit the patch.\n>>\n>> Thanks!\n>>\n>> - Sonny\n>\n> Bumping again for visibility.\n\nIt's probably not due to \"something amiss\" but nobody was interested\nin what the patch tries to achieve and had nothing useful to say on\nit (hence everybody was quiet).\n\nSome other people may have skipped seeing only [0/1] without [1/1],\nas reading only the cover letter without an accompanying patch will\nbe waste of their time for busy reviewers---by the time [1/1] comes\nit is likely they forgot what [0/1] said and they need to go back\nand read it again, so they would likely have said \"I'll read both\nwhen [1/1] appears on the list\", moved on, and then have forgotten\nabout it. I am in that camp.\n\n"},{"id":"327156","messageId":"xmqqshggvien.fsf@gitster.mtv.corp.google.com","threadId":"46630","inReplyTo":"c3ac0335-4f41-40d9-00ee-2a4e115bcf7a@gmail.com","subject":"Re: [PATCH 0/1] Add stash entry count summary to short status output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-24T20:07:44Z","receivedAt":"2017-08-24T20:07:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sonny Michaud writes:\n\n> diff --git a/wt-status.c b/wt-status.c \n> index 77c27c511..651bb01f0 100644 \n> --- a/wt-status.c \n> +++ b/wt-status.c \n> @@ -1827,6 +1827,15 @@ static void wt_shortstatus_print_tracking(struct wt_status *s) \n> fputc(s->null_termination ? '\\0' : '\\n', s->fp); \n> } \n> \n> +static void wt_shortstatus_print_stash_summary(struct wt_status *s) \n> +{ \n> + int stash_count = 0; \n> + \n> + for_each_reflog_ent(\"refs/stash\", stash_count_refs, &stash_count); \n\nA singleton instance of this in wt_longstatus_print_stash_summary()\nthing was OK, but let's not duplicate and spread the badness. Have\na simple there-liner helper function \"static int stash_count(void);\"\nthat does the above and returns the stash_count, and use it from\nboth places.\n\n> + if (stash_count > 0) \n> + color_fprintf(s->fp, color(WT_STATUS_HEADER, s), \"## Stash entries: %d\", stash_count); \n\nThat's a funny way to indent (dedent?) a body of an if() statement.\n\nDon't scripts that read this output (I notice that this is also\ncalled by wt_porcelain_print() function) expect that entries that\nare led by \"##\" are about the current branch and its tracking\ninformation? \n\nThis patch would break these script by adding this new line using\nthe same \"##\" leader.\n\n"},{"id":"327495","messageId":"a46f972b-e83f-a08c-72ab-bfd1e7c4b310@gmail.com","threadId":"46630","inReplyTo":"xmqqshggvien.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/1] Add stash entry count summary to short status output","fromName":"Sonny Michaud","fromEmail":"michaud.sonny@gmail.com","sentAt":"2017-08-31T21:49:16Z","receivedAt":"2017-08-31T21:51:16Z","isPatch":true,"sender":{"key":"michaud.sonny@gmail.com","avatar":null},"body":"Junio,\n\nI appreciate you taking the time to look over this patch.  I have some\ncomments below before re-submitting an updated version.\n\nOn 08/24/2017 04:07 PM, Junio C Hamano wrote:\n> Sonny Michaud writes:\n>\n>> diff --git a/wt-status.c b/wt-status.c \n>> index 77c27c511..651bb01f0 100644 \n>> --- a/wt-status.c \n>> +++ b/wt-status.c \n>> @@ -1827,6 +1827,15 @@ static void wt_shortstatus_print_tracking(struct wt_status *s) \n>> fputc(s->null_termination ? '\\0' : '\\n', s->fp); \n>> } \n>> \n>> +static void wt_shortstatus_print_stash_summary(struct wt_status *s) \n>> +{ \n>> + int stash_count = 0; \n>> + \n>> + for_each_reflog_ent(\"refs/stash\", stash_count_refs, &stash_count); \n> A singleton instance of this in wt_longstatus_print_stash_summary()\n> thing was OK, but let's not duplicate and spread the badness. Have\n> a simple there-liner helper function \"static int stash_count(void);\"\n> that does the above and returns the stash_count, and use it from\n> both places.\nThat is reasonable.\n>> + if (stash_count > 0) \n>> + color_fprintf(s->fp, color(WT_STATUS_HEADER, s), \"## Stash entries: %d\", stash_count); \n> That's a funny way to indent (dedent?) a body of an if() statement.\nMy bad, I can clean that up!\n>\n> Don't scripts that read this output (I notice that this is also\n> called by wt_porcelain_print() function) expect that entries that\n> are led by \"##\" are about the current branch and its tracking\n> information? \n>\n> This patch would break these script by adding this new line using\n> the same \"##\" leader.\n>\nI did not consider porcelain output;  it looks like the headers are\nstripped from it, though, so this might not be an issue.  Do you think\nthis is a worthwhile path to continue on?\n\nThanks,\nSonny\n"},{"id":"327497","messageId":"20170901031100.GI30568@aiede.mtv.corp.google.com","threadId":"46630","inReplyTo":"c3ac0335-4f41-40d9-00ee-2a4e115bcf7a@gmail.com","subject":"Re: [PATCH 0/1] Add stash entry count summary to short status output","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-09-01T03:11:00Z","receivedAt":"2017-09-01T03:11:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Sonny,\n\nSonny Michaud wrote:\n\n> Any assistance would be greatly appreciated; I just wanted to get\n> started by hacking first and asking questions later!                         \n\nWelcome to the Git project, and sorry for the silence before. Getting\nto see what people are working on is one of the nice things about\nbeing subscribed to this list. :)\n\n> Subject: [PATCH 0/1] Add stash entry count summary to short status output\n\nAs Junio mentioned, this subject line may discourage casual readers.\nIn a one-patch series like this one, there's no need for a cover\nletter: the patch's commit message should speak for itself anyway.\n\nSee the DISCUSSION section of \"git help format-patch\" for more details\non sending a patch for review.\n\nYou can put [RFC/PATCH] to make it clear that a patch is still a work\nin progress and you're looking for early comments on it.\n\n> this patch adds a header in the same style as the one provided by\n> the --branch option when the user supplies both --show-stash and\n> --short.\n\nFor next time, this information belongs in the commit message.\n\n[...]\n> This patch extends the functionality of the recently introduced\n> --show-stash option to the status command, providing a header similar to\n> the one displayed when using the --branch option.\n\nCan you include an example of output in the commit message? Also,\ncan you say more about the workflow this fits in? What information\ndoes a person expect in the output, and how does this change help a\nperson do what they are trying to do?\n\n[...]\n> --- a/t/t7508-status.sh\n> +++ b/t/t7508-status.sh\n> @@ -1619,6 +1619,11 @@ test_expect_success 'show stash info with \"--show-stash\"' '\n> test_i18ngrep \"^Your stash currently has 1 entry$\" expected_with_stash\n> '\n>\n> +test_expect_success 'show stash info with \"--show-stash\" and \"--short\"' '\n> + git status --show-stash --short >expected_with_stash &&\n> + test_i18ngrep \"Stash entries: 2\" expected_with_stash\n\nInteresting. The idea makes sense.\n\nI have the same questions as Junio about how this affects --porcelain\noutput.\n\nAlso, could this patch also include some documentation in\nDocumentation/git-status.txt about the new behavior?\n\n[...]\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1827,6 +1827,15 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n> fputc(s->null_termination ? '\\0' : '\\n', s->fp);\n> }\n>\n> +static void wt_shortstatus_print_stash_summary(struct wt_status *s)\n> +{\n> + int stash_count = 0;\n> +\n> + for_each_reflog_ent(\"refs/stash\", stash_count_refs, &stash_count);\n> + if (stash_count > 0)\n> + color_fprintf(s->fp, color(WT_STATUS_HEADER, s), \"## Stash entries: %d\", stash_count);\n\nSee the \"For C programs\" section of Documentation/SubmittingPatches\nfor some formatting hints.\n\nIn the \"pu\" branch, there is some experimental support for helping\nwith that. If you merge in the bw/git-clang-format branch from there,\nyou can test your patch for style errors (or for errors in that support\n:)) by running \"make style\".\n\nThanks and hope that helps,\nJonathan\n"}]}