{"thread":{"id":"48321","subject":"[PATCH 0/2] Fix --short and --porcelain options for commit","startedAt":"2018-04-18T16:31:57Z","lastAt":"2018-07-30T22:15:49Z","messageCount":26,"participants":["Samuel Lijin","Martin Ågren","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"344993","messageId":"20180418030655.19378-1-sxlijin@gmail.com","threadId":"48321","inReplyTo":null,"subject":"[PATCH 0/2] Fix --short and --porcelain options for commit","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-04-18T03:06:53Z","receivedAt":"2018-04-18T16:31:57Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Hi all - I last contributed about a year ago and I've finally found the\ntime to start contributing again, and hopefully I'll stick around this\ntime. Figured I'd start with something small :)\n\nSamuel Lijin (2):\n commit: fix --short and --porcelain\n wt-status: const-ify all printf helper methods\n\n t/t7501-commit.sh | 4 ++--\n wt-status.c | 57 +++++++++++++++++++++++++++++++++++--------------------\n wt-status.h | 4 ++--\n 3 files changed, 40 insertions(+), 25 deletions(-)\n\n-- \n2.16.2\n\n"},{"id":"344994","messageId":"20180418030655.19378-2-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180418030655.19378-1-sxlijin@gmail.com","subject":"[PATCH 1/2] commit: fix --short and --porcelain","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-04-18T03:06:54Z","receivedAt":"2018-04-18T16:32:02Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Make invoking `git commit` with `--short` or `--porcelain` return status\ncode zero when there is something to commit.\n\nMark the commitable flag in the wt_status object in the call to\n`wt_status_collect()`, instead of in `wt_longstatus_print_updated()`,\nand simplify the logic in the latter function to take advantage of the\nlogic shifted to the former.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7501-commit.sh | 4 ++--\n wt-status.c | 39 +++++++++++++++++++++++++++------------\n 2 files changed, 29 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex fa61b1a4e..85a8217fd 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -87,12 +87,12 @@ test_expect_success '--dry-run with stuff to commit returns ok' '\n \tgit commit -m next -a --dry-run\n '\n \n-test_expect_failure '--short with stuff to commit returns ok' '\n+test_expect_success '--short with stuff to commit returns ok' '\n \techo bongo bongo bongo >>file &&\n \tgit commit -m next -a --short\n '\n \n-test_expect_failure '--porcelain with stuff to commit returns ok' '\n+test_expect_success '--porcelain with stuff to commit returns ok' '\n \techo bongo bongo bongo >>file &&\n \tgit commit -m next -a --porcelain\n '\ndiff --git a/wt-status.c b/wt-status.c\nindex 50815e5fa..26b0a6221 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -718,6 +718,19 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \t\ts->untracked_in_ms = (getnanotime() - t_begin) / 1000000;\n }\n \n+static void wt_status_mark_commitable(struct wt_status *s) {\n+\tint i;\n+\n+\tfor (i = 0; i < s->change.nr; i++) {\n+\t\tstruct wt_status_change_data *d = (s->change.items[i]).util;\n+\n+\t\tif (d->index_status && d->index_status != DIFF_STATUS_UNMERGED) {\n+\t\t\ts->commitable = 1;\n+\t\t\treturn;\n+\t\t}\n+\t}\n+}\n+\n void wt_status_collect(struct wt_status *s)\n {\n \twt_status_collect_changes_worktree(s);\n@@ -726,7 +739,10 @@ void wt_status_collect(struct wt_status *s)\n \t\twt_status_collect_changes_initial(s);\n \telse\n \t\twt_status_collect_changes_index(s);\n+\n \twt_status_collect_untracked(s);\n+\n+\twt_status_mark_commitable(s);\n }\n \n static void wt_longstatus_print_unmerged(struct wt_status *s)\n@@ -754,26 +770,25 @@ static void wt_longstatus_print_unmerged(struct wt_status *s)\n \n static void wt_longstatus_print_updated(struct wt_status *s)\n {\n-\tint shown_header = 0;\n-\tint i;\n+\tif (!s->commitable) {\n+\t\treturn;\n+\t}\n+\n+\twt_longstatus_print_cached_header(s);\n \n+\tint i;\n \tfor (i = 0; i < s->change.nr; i++) {\n \t\tstruct wt_status_change_data *d;\n \t\tstruct string_list_item *it;\n \t\tit = &(s->change.items[i]);\n \t\td = it->util;\n-\t\tif (!d->index_status ||\n-\t\t d->index_status == DIFF_STATUS_UNMERGED)\n-\t\t\tcontinue;\n-\t\tif (!shown_header) {\n-\t\t\twt_longstatus_print_cached_header(s);\n-\t\t\ts->commitable = 1;\n-\t\t\tshown_header = 1;\n+\t\tif (d->index_status &&\n+\t\t d->index_status != DIFF_STATUS_UNMERGED) {\n+\t\t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n \t\t}\n-\t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n \t}\n-\tif (shown_header)\n-\t\twt_longstatus_print_trailer(s);\n+\n+\twt_longstatus_print_trailer(s);\n }\n \n /*\n-- \n2.16.2\n\n"},{"id":"344995","messageId":"20180418030655.19378-3-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180418030655.19378-1-sxlijin@gmail.com","subject":"[PATCH 2/2] wt-status: const-ify all printf helper methods","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-04-18T03:06:55Z","receivedAt":"2018-04-18T16:32:07Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Change the method signatures of all printf helper methods to take a\n`const struct wt_status *` rather than a `struct wt_status *`.\n\nSigned-off-by: Samuel Lijin \n---\n wt-status.c | 18 +++++++++---------\n wt-status.h | 4 ++--\n 2 files changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 26b0a6221..55d29bc09 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -33,7 +33,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_NIL, /* WT_STATUS_ONBRANCH */\n };\n \n-static const char *color(int slot, struct wt_status *s)\n+static const char *color(int slot, const struct wt_status *s)\n {\n \tconst char *c = \"\";\n \tif (want_color(s->use_color))\n@@ -43,7 +43,7 @@ static const char *color(int slot, struct wt_status *s)\n \treturn c;\n }\n \n-static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n+static void status_vprintf(const struct wt_status *s, int at_bol, const char *color,\n \t\tconst char *fmt, va_list ap, const char *trail)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -89,7 +89,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n \tstrbuf_release(&sb);\n }\n \n-void status_printf_ln(struct wt_status *s, const char *color,\n+void status_printf_ln(const struct wt_status *s, const char *color,\n \t\t\tconst char *fmt, ...)\n {\n \tva_list ap;\n@@ -99,7 +99,7 @@ void status_printf_ln(struct wt_status *s, const char *color,\n \tva_end(ap);\n }\n \n-void status_printf(struct wt_status *s, const char *color,\n+void status_printf(const struct wt_status *s, const char *color,\n \t\t\tconst char *fmt, ...)\n {\n \tva_list ap;\n@@ -109,7 +109,7 @@ void status_printf(struct wt_status *s, const char *color,\n \tva_end(ap);\n }\n \n-static void status_printf_more(struct wt_status *s, const char *color,\n+static void status_printf_more(const struct wt_status *s, const char *color,\n \t\t\t const char *fmt, ...)\n {\n \tva_list ap;\n@@ -192,7 +192,7 @@ static void wt_longstatus_print_unmerged_header(struct wt_status *s)\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_cached_header(struct wt_status *s)\n+static void wt_longstatus_print_cached_header(const struct wt_status *s)\n {\n \tconst char *c = color(WT_STATUS_HEADER, s);\n \n@@ -239,7 +239,7 @@ static void wt_longstatus_print_other_header(struct wt_status *s,\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_trailer(struct wt_status *s)\n+static void wt_longstatus_print_trailer(const struct wt_status *s)\n {\n \tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n }\n@@ -332,7 +332,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n \tstrbuf_release(&onebuf);\n }\n \n-static void wt_longstatus_print_change_data(struct wt_status *s,\n+static void wt_longstatus_print_change_data(const struct wt_status *s,\n \t\t\t\t\t int change_type,\n \t\t\t\t\t struct string_list_item *it)\n {\n@@ -768,7 +768,7 @@ static void wt_longstatus_print_unmerged(struct wt_status *s)\n \n }\n \n-static void wt_longstatus_print_updated(struct wt_status *s)\n+static void wt_longstatus_print_updated(const struct wt_status *s)\n {\n \tif (!s->commitable) {\n \t\treturn;\ndiff --git a/wt-status.h b/wt-status.h\nindex 430770b85..83a1f7c00 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -135,9 +135,9 @@ int wt_status_check_bisect(const struct worktree *wt,\n \t\t\t struct wt_status_state *state);\n \n __attribute__((format (printf, 3, 4)))\n-void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...);\n+void status_printf_ln(const struct wt_status *s, const char *color, const char *fmt, ...);\n __attribute__((format (printf, 3, 4)))\n-void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);\n+void status_printf(const struct wt_status *s, const char *color, const char *fmt, ...);\n \n /* The following functions expect that the caller took care of reading the index. */\n int has_unstaged_changes(int ignore_submodules);\n-- \n2.16.2\n\n"},{"id":"345002","messageId":"CAN0heSqXwVR5cdMwipUdPrnbUyCU8v2GzWK=2-0_ZWoWw3SO2w@mail.gmail.com","threadId":"48321","inReplyTo":"20180418030655.19378-2-sxlijin@gmail.com","subject":"Re: [PATCH 1/2] commit: fix --short and --porcelain","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-04-18T18:38:17Z","receivedAt":"2018-04-18T18:38:23Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Hi Samuel,\n\nWelcome back. :-)\n\nOn 18 April 2018 at 05:06, Samuel Lijin wrote:\n> Make invoking `git commit` with `--short` or `--porcelain` return status\n> code zero when there is something to commit.\n>\n> Mark the commitable flag in the wt_status object in the call to\n> `wt_status_collect()`, instead of in `wt_longstatus_print_updated()`,\n> and simplify the logic in the latter function to take advantage of the\n> logic shifted to the former.\n\nThe subject is sort of vague about what is being fixed. Maybe \"commit:\nfix return code of ...\", or \"wt-status: set `commitable` when\ncollecting, not when printing\". Or something... I can't come up with\nsomething brilliant off the top of my head.\n\nI did not understand the first paragraph until I had read the second and\npeaked at the code. Maybe tell the story the other way around? Something\nlike this:\n\n Mark the `commitable` flag in the wt_status object in\n `wt_status_collect()`, instead of in `wt_longstatus_print_updated()`,\n and simplify the logic in the latter function to take advantage of the\n logic shifted to the former.\n\n This means that callers do need to actually use the printer function\n to collect the `commitable` flag -- it is sufficient to call\n `wt_status_collect()`.\n\n As a result, invoking `git commit` with `--short` or `--porcelain`\n results in return status code zero when there is something to commit.\n This fixes two bugs documented in our test suite.\n\n> t/t7501-commit.sh | 4 ++--\n> wt-status.c | 39 +++++++++++++++++++++++++++------------\n> 2 files changed, 29 insertions(+), 14 deletions(-)\n\nI tried to find somewhere in the documentation where this bug was\ndescribed (git-commit.txt or git-status.txt), but failed. So there\nshould be nothing to update there.\n\n> +static void wt_status_mark_commitable(struct wt_status *s) {\n> + int i;\n> +\n> + for (i = 0; i < s->change.nr; i++) {\n> + struct wt_status_change_data *d = (s->change.items[i]).util;\n> +\n> + if (d->index_status && d->index_status != DIFF_STATUS_UNMERGED) {\n> + s->commitable = 1;\n> + return;\n> + }\n> + }\n> +}\n\nThis helper does exactly what the old code did inside\n`wt_longstatus_print_updated()` with regards to `commitable`. Ok.\n\nThis function does not reset `commitable` to 0, so reusing a `struct\nwt_status` won't necessarily work out. I have not thought about whether\nsuch a caller would be horribly broken for other reasons...\n\n> void wt_status_collect(struct wt_status *s)\n> {\n> wt_status_collect_changes_worktree(s);\n> @@ -726,7 +739,10 @@ void wt_status_collect(struct wt_status *s)\n> wt_status_collect_changes_initial(s);\n> else\n> wt_status_collect_changes_index(s);\n> +\n> wt_status_collect_untracked(s);\n> +\n> + wt_status_mark_commitable(s);\n> }\n\nSo whenever we `..._collect()`, `commitable` is set for us. This is the\nonly caller of the new helper, so in order to be able to trust\n`commitable`, one needs to call `wt_status_collect()`. Seems a\nreasonable assumption to make that the caller will remember to do so\nbefore printing. (And all current users do, so we're not regressing in\nsome user.)\n\n> static void wt_longstatus_print_unmerged(struct wt_status *s)\n> @@ -754,26 +770,25 @@ static void wt_longstatus_print_unmerged(struct wt_status *s)\n>\n> static void wt_longstatus_print_updated(struct wt_status *s)\n> {\n> - int shown_header = 0;\n> - int i;\n> + if (!s->commitable) {\n> + return;\n> + }\n\nRegarding my comment above: If you forget to `..._collect()` first, this\nfunction is a no-op.\n\n> +\n> + wt_longstatus_print_cached_header(s);\n>\n> + int i;\n\nYou should leave this variable declaration at the top of the function.\n\n> for (i = 0; i < s->change.nr; i++) {\n> struct wt_status_change_data *d;\n> struct string_list_item *it;\n> it = &(s->change.items[i]);\n> d = it->util;\n> - if (!d->index_status ||\n> - d->index_status == DIFF_STATUS_UNMERGED)\n> - continue;\n> - if (!shown_header) {\n> - wt_longstatus_print_cached_header(s);\n> - s->commitable = 1;\n> - shown_header = 1;\n> + if (d->index_status &&\n> + d->index_status != DIFF_STATUS_UNMERGED) {\n> + wt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n> }\n> - wt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n> }\n> - if (shown_header)\n> - wt_longstatus_print_trailer(s);\n> +\n> + wt_longstatus_print_trailer(s);\n> }\n\nThis rewrite matches the original logic, assuming we can trust\n`commitable`. The result is a function called `print()` which does not\nmodify the struct it is given for printing. Nice. So you can make the\nargument a `const struct wt_status *`. Except this function uses helpers\nthat are missing the `const`.\n\nYou fix that in patch 2/2. I would probably have made that patch as 1/2,\nthen done this patch as 2/2 ending the commit message with something\nlike \"As a result, we can mark the argument as `const`.\", or even just\nsilently inserting the `const` for this one function. Just a thought.\n\nMartin\n"},{"id":"345058","messageId":"CAJZjrdXci_vAj2LMJOMJstR0ggEpvjROJX2OwMQ1qmkwAEb4TA@mail.gmail.com","threadId":"48321","inReplyTo":"CAJZjrdW3X8eaSit85otKV2HvHmu0NDGcnnnrtxHME03q=eWW-Q@mail.gmail.com","subject":"Re: [PATCH 1/2] commit: fix --short and --porcelain","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-04-19T03:55:57Z","receivedAt":"2018-04-19T03:56:42Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Wed, Apr 18, 2018 at 8:55 PM, Samuel Lijin wrote:\n> Thanks for the quick review!\n>\n> On Wed, Apr 18, 2018 at 11:38 AM, Martin Ågren wrote:\n>> Hi Samuel,\n>>\n>> Welcome back. :-)\n>>\n>> On 18 April 2018 at 05:06, Samuel Lijin wrote:\n>>> Make invoking `git commit` with `--short` or `--porcelain` return status\n>>> code zero when there is something to commit.\n>>>\n>>> Mark the commitable flag in the wt_status object in the call to\n>>> `wt_status_collect()`, instead of in `wt_longstatus_print_updated()`,\n>>> and simplify the logic in the latter function to take advantage of the\n>>> logic shifted to the former.\n>>\n>> The subject is sort of vague about what is being fixed. Maybe \"commit:\n>> fix return code of ...\", or \"wt-status: set `commitable` when\n>> collecting, not when printing\". Or something... I can't come up with\n>> something brilliant off the top of my head.\n>>\n>> I did not understand the first paragraph until I had read the second and\n>> peaked at the code. Maybe tell the story the other way around? Something\n>> like this:\n>>\n>> Mark the `commitable` flag in the wt_status object in\n>> `wt_status_collect()`, instead of in `wt_longstatus_print_updated()`,\n>> and simplify the logic in the latter function to take advantage of the\n>> logic shifted to the former.\n>>\n>> This means that callers do need to actually use the printer function\n>> to collect the `commitable` flag -- it is sufficient to call\n>> `wt_status_collect()`.\n>>\n>> As a result, invoking `git commit` with `--short` or `--porcelain`\n>> results in return status code zero when there is something to commit.\n>> This fixes two bugs documented in our test suite.\n>\n> That definitely works better. Will fix when I reroll.\n>\n>>> t/t7501-commit.sh | 4 ++--\n>>> wt-status.c | 39 +++++++++++++++++++++++++++------------\n>>> 2 files changed, 29 insertions(+), 14 deletions(-)\n>>\n>> I tried to find somewhere in the documentation where this bug was\n>> described (git-commit.txt or git-status.txt), but failed. So there\n>> should be nothing to update there.\n>>\n>>> +static void wt_status_mark_commitable(struct wt_status *s) {\n>>> + int i;\n>>> +\n>>> + for (i = 0; i < s->change.nr; i++) {\n>>> + struct wt_status_change_data *d = (s->change.items[i]).util;\n>>> +\n>>> + if (d->index_status && d->index_status != DIFF_STATUS_UNMERGED) {\n>>> + s->commitable = 1;\n>>> + return;\n>>> + }\n>>> + }\n>>> +}\n>>\n>> This helper does exactly what the old code did inside\n>> `wt_longstatus_print_updated()` with regards to `commitable`. Ok.\n>>\n>> This function does not reset `commitable` to 0, so reusing a `struct\n>> wt_status` won't necessarily work out. I have not thought about whether\n>> such a caller would be horribly broken for other reasons...\n>>\n>>> void wt_status_collect(struct wt_status *s)\n>>> {\n>>> wt_status_collect_changes_worktree(s);\n>>> @@ -726,7 +739,10 @@ void wt_status_collect(struct wt_status *s)\n>>> wt_status_collect_changes_initial(s);\n>>> else\n>>> wt_status_collect_changes_index(s);\n>>> +\n>>> wt_status_collect_untracked(s);\n>>> +\n>>> + wt_status_mark_commitable(s);\n>>> }\n>>\n>> So whenever we `..._collect()`, `commitable` is set for us. This is the\n>> only caller of the new helper, so in order to be able to trust\n>> `commitable`, one needs to call `wt_status_collect()`. Seems a\n>> reasonable assumption to make that the caller will remember to do so\n>> before printing. (And all current users do, so we're not regressing in\n>> some user.)\n>>\n>>> static void wt_longstatus_print_unmerged(struct wt_status *s)\n>>> @@ -754,26 +770,25 @@ static void wt_longstatus_print_unmerged(struct wt_status *s)\n>>>\n>>> static void wt_longstatus_print_updated(struct wt_status *s)\n>>> {\n>>> - int shown_header = 0;\n>>> - int i;\n>>> + if (!s->commitable) {\n>>> + return;\n>>> + }\n>>\n>> Regarding my comment above: If you forget to `..._collect()` first, this\n>> function is a no-op.\n>>\n>>> +\n>>> + wt_longstatus_print_cached_header(s);\n>>>\n>>> + int i;\n>>\n>> You should leave this variable declaration at the top of the function.\n>>\n>>> for (i = 0; i < s->change.nr; i++) {\n>>> struct wt_status_change_data *d;\n>>> struct string_list_item *it;\n>>> it = &(s->change.items[i]);\n>>> d = it->util;\n>>> - if (!d->index_status ||\n>>> - d->index_status == DIFF_STATUS_UNMERGED)\n>>> - continue;\n>>> - if (!shown_header) {\n>>> - wt_longstatus_print_cached_header(s);\n>>> - s->commitable = 1;\n>>> - shown_header = 1;\n>>> + if (d->index_status &&\n>>> + d->index_status != DIFF_STATUS_UNMERGED) {\n>>> + wt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n>>> }\n>>> - wt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n>>> }\n>>> - if (shown_header)\n>>> - wt_longstatus_print_trailer(s);\n>>> +\n>>> + wt_longstatus_print_trailer(s);\n>>> }\n>>\n>> This rewrite matches the original logic, assuming we can trust\n>> `commitable`. The result is a function called `print()` which does not\n>> modify the struct it is given for printing. Nice. So you can make the\n>> argument a `const struct wt_status *`. Except this function uses helpers\n>> that are missing the `const`.\n>>\n>> You fix that in patch 2/2. I would probably have made that patch as 1/2,\n>> then done this patch as 2/2 ending the commit message with something\n>> like \"As a result, we can mark the argument as `const`.\", or even just\n>> silently inserting the `const` for this one function. Just a thought.\n>\n> I originally ordered it the way I did because in the constify-first\n> scenario, \"fix t7501\" and \"const-ify wt_longstatus_print_updated\"\n> seemed like two logically separate patches to me (which would have\n> made the patch series three patches instead of two). I'm happy to\n> reroll in whichever fashion if people care strongly though.\n>\n>> Martin\n"},{"id":"345190","messageId":"CAPig+cQp8c8v5cfaWqPNajcTdxYEj-FrOHykjOi+RAbEa4-VzQ@mail.gmail.com","threadId":"48321","inReplyTo":"20180418030655.19378-2-sxlijin@gmail.com","subject":"Re: [PATCH 1/2] commit: fix --short and --porcelain","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-20T07:08:42Z","receivedAt":"2018-04-20T07:08:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 17, 2018 at 11:06 PM, Samuel Lijin wrote:\n> Make invoking `git commit` with `--short` or `--porcelain` return status\n> code zero when there is something to commit.\n>\n> Mark the commitable flag in the wt_status object in the call to\n> `wt_status_collect()`, instead of in `wt_longstatus_print_updated()`,\n> and simplify the logic in the latter function to take advantage of the\n> logic shifted to the former.\n>\n> Signed-off-by: Samuel Lijin \n> ---\n> diff --git a/wt-status.c b/wt-status.c\n> @@ -754,26 +770,25 @@ static void wt_longstatus_print_unmerged(struct wt_status *s)\n> static void wt_longstatus_print_updated(struct wt_status *s)\n> {\n> - int shown_header = 0;\n> - int i;\n> + if (!s->commitable) {\n> + return;\n> + }\n> +\n> + wt_longstatus_print_cached_header(s);\n>\n> + int i;\n> for (i = 0; i < s->change.nr; i++) {\n\nDeclaration after statement: Declare 'i' at the top of the function as\nit was before this patch.\n"},{"id":"346168","messageId":"20180426092524.25264-1-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180418030655.19378-1-sxlijin@gmail.com","subject":"[PATCH v2 0/2] Fix --short and --porcelain options for commit","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-04-26T09:25:22Z","receivedAt":"2018-04-30T15:56:25Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Rerolling patch series to fix t7501.\n\nSamuel Lijin (2):\n commit: fix --short and --porcelain options\n wt-status: const-ify all printf helper methods\n\n t/t7501-commit.sh | 4 ++--\n wt-status.c | 56 ++++++++++++++++++++++++++++++-----------------\n wt-status.h | 4 ++--\n 3 files changed, 40 insertions(+), 24 deletions(-)\n\n-- \n2.17.0\n\n"},{"id":"346169","messageId":"20180426092524.25264-2-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180426092524.25264-1-sxlijin@gmail.com","subject":"[PATCH v2 1/2] commit: fix --short and --porcelain options","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-04-26T09:25:23Z","receivedAt":"2018-04-30T15:56:33Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Mark the commitable flag in the wt_status object in the call to\n`wt_status_collect()`, instead of in `wt_longstatus_print_updated()`,\nand simplify the logic in the latter function to take advantage of the\nlogic shifted to the former. This means that callers do not need to use\n`wt_longstatus_print_updated()` to collect the `commitable` flag;\ncalling `wt_status_collect()` is sufficient.\n\nAs a result, invoking `git commit` with `--short` or `--porcelain`\n(which imply `--dry-run`, but previously returned an inconsistent error\ncode inconsistent with dry run behavior) correctly returns status code\nzero when there is something to commit. This fixes two bugs documented\nin the test suite.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7501-commit.sh | 4 ++--\n wt-status.c | 38 +++++++++++++++++++++++++++-----------\n 2 files changed, 29 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex fa61b1a4e..85a8217fd 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -87,12 +87,12 @@ test_expect_success '--dry-run with stuff to commit returns ok' '\n \tgit commit -m next -a --dry-run\n '\n \n-test_expect_failure '--short with stuff to commit returns ok' '\n+test_expect_success '--short with stuff to commit returns ok' '\n \techo bongo bongo bongo >>file &&\n \tgit commit -m next -a --short\n '\n \n-test_expect_failure '--porcelain with stuff to commit returns ok' '\n+test_expect_success '--porcelain with stuff to commit returns ok' '\n \techo bongo bongo bongo >>file &&\n \tgit commit -m next -a --porcelain\n '\ndiff --git a/wt-status.c b/wt-status.c\nindex 50815e5fa..2e5452731 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -718,6 +718,19 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \t\ts->untracked_in_ms = (getnanotime() - t_begin) / 1000000;\n }\n \n+static void wt_status_mark_commitable(struct wt_status *s) {\n+\tint i;\n+\n+\tfor (i = 0; i < s->change.nr; i++) {\n+\t\tstruct wt_status_change_data *d = (s->change.items[i]).util;\n+\n+\t\tif (d->index_status && d->index_status != DIFF_STATUS_UNMERGED) {\n+\t\t\ts->commitable = 1;\n+\t\t\treturn;\n+\t\t}\n+\t}\n+}\n+\n void wt_status_collect(struct wt_status *s)\n {\n \twt_status_collect_changes_worktree(s);\n@@ -726,7 +739,10 @@ void wt_status_collect(struct wt_status *s)\n \t\twt_status_collect_changes_initial(s);\n \telse\n \t\twt_status_collect_changes_index(s);\n+\n \twt_status_collect_untracked(s);\n+\n+\twt_status_mark_commitable(s);\n }\n \n static void wt_longstatus_print_unmerged(struct wt_status *s)\n@@ -754,26 +770,26 @@ static void wt_longstatus_print_unmerged(struct wt_status *s)\n \n static void wt_longstatus_print_updated(struct wt_status *s)\n {\n-\tint shown_header = 0;\n \tint i;\n \n+\tif (!s->commitable) {\n+\t\treturn;\n+\t}\n+\n+\twt_longstatus_print_cached_header(s);\n+\n \tfor (i = 0; i < s->change.nr; i++) {\n \t\tstruct wt_status_change_data *d;\n \t\tstruct string_list_item *it;\n \t\tit = &(s->change.items[i]);\n \t\td = it->util;\n-\t\tif (!d->index_status ||\n-\t\t d->index_status == DIFF_STATUS_UNMERGED)\n-\t\t\tcontinue;\n-\t\tif (!shown_header) {\n-\t\t\twt_longstatus_print_cached_header(s);\n-\t\t\ts->commitable = 1;\n-\t\t\tshown_header = 1;\n+\t\tif (d->index_status &&\n+\t\t d->index_status != DIFF_STATUS_UNMERGED) {\n+\t\t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n \t\t}\n-\t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n \t}\n-\tif (shown_header)\n-\t\twt_longstatus_print_trailer(s);\n+\n+\twt_longstatus_print_trailer(s);\n }\n \n /*\n-- \n2.17.0\n\n"},{"id":"346170","messageId":"20180426092524.25264-3-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180426092524.25264-1-sxlijin@gmail.com","subject":"[PATCH v2 2/2] wt-status: const-ify all printf helper methods","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-04-26T09:25:24Z","receivedAt":"2018-04-30T15:56:36Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Change the method signatures of all printf helper methods to take a\n`const struct wt_status *` rather than a `struct wt_status *`.\n\nSigned-off-by: Samuel Lijin \n---\n wt-status.c | 18 +++++++++---------\n wt-status.h | 4 ++--\n 2 files changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 2e5452731..4360bbfd7 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -33,7 +33,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_NIL, /* WT_STATUS_ONBRANCH */\n };\n \n-static const char *color(int slot, struct wt_status *s)\n+static const char *color(int slot, const struct wt_status *s)\n {\n \tconst char *c = \"\";\n \tif (want_color(s->use_color))\n@@ -43,7 +43,7 @@ static const char *color(int slot, struct wt_status *s)\n \treturn c;\n }\n \n-static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n+static void status_vprintf(const struct wt_status *s, int at_bol, const char *color,\n \t\tconst char *fmt, va_list ap, const char *trail)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -89,7 +89,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n \tstrbuf_release(&sb);\n }\n \n-void status_printf_ln(struct wt_status *s, const char *color,\n+void status_printf_ln(const struct wt_status *s, const char *color,\n \t\t\tconst char *fmt, ...)\n {\n \tva_list ap;\n@@ -99,7 +99,7 @@ void status_printf_ln(struct wt_status *s, const char *color,\n \tva_end(ap);\n }\n \n-void status_printf(struct wt_status *s, const char *color,\n+void status_printf(const struct wt_status *s, const char *color,\n \t\t\tconst char *fmt, ...)\n {\n \tva_list ap;\n@@ -109,7 +109,7 @@ void status_printf(struct wt_status *s, const char *color,\n \tva_end(ap);\n }\n \n-static void status_printf_more(struct wt_status *s, const char *color,\n+static void status_printf_more(const struct wt_status *s, const char *color,\n \t\t\t const char *fmt, ...)\n {\n \tva_list ap;\n@@ -192,7 +192,7 @@ static void wt_longstatus_print_unmerged_header(struct wt_status *s)\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_cached_header(struct wt_status *s)\n+static void wt_longstatus_print_cached_header(const struct wt_status *s)\n {\n \tconst char *c = color(WT_STATUS_HEADER, s);\n \n@@ -239,7 +239,7 @@ static void wt_longstatus_print_other_header(struct wt_status *s,\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_trailer(struct wt_status *s)\n+static void wt_longstatus_print_trailer(const struct wt_status *s)\n {\n \tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n }\n@@ -332,7 +332,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n \tstrbuf_release(&onebuf);\n }\n \n-static void wt_longstatus_print_change_data(struct wt_status *s,\n+static void wt_longstatus_print_change_data(const struct wt_status *s,\n \t\t\t\t\t int change_type,\n \t\t\t\t\t struct string_list_item *it)\n {\n@@ -768,7 +768,7 @@ static void wt_longstatus_print_unmerged(struct wt_status *s)\n \n }\n \n-static void wt_longstatus_print_updated(struct wt_status *s)\n+static void wt_longstatus_print_updated(const struct wt_status *s)\n {\n \tint i;\n \ndiff --git a/wt-status.h b/wt-status.h\nindex 430770b85..83a1f7c00 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -135,9 +135,9 @@ int wt_status_check_bisect(const struct worktree *wt,\n \t\t\t struct wt_status_state *state);\n \n __attribute__((format (printf, 3, 4)))\n-void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...);\n+void status_printf_ln(const struct wt_status *s, const char *color, const char *fmt, ...);\n __attribute__((format (printf, 3, 4)))\n-void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);\n+void status_printf(const struct wt_status *s, const char *color, const char *fmt, ...);\n \n /* The following functions expect that the caller took care of reading the index. */\n int has_unstaged_changes(int ignore_submodules);\n-- \n2.17.0\n\n"},{"id":"346427","messageId":"xmqq36zawqr3.fsf@gitster-ct.c.googlers.com","threadId":"48321","inReplyTo":"20180426092524.25264-2-sxlijin@gmail.com","subject":"Re: [PATCH v2 1/2] commit: fix --short and --porcelain options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-02T05:50:08Z","receivedAt":"2018-05-02T05:50:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> Mark the commitable flag in the wt_status object in the call to\n> `wt_status_collect()`, instead of in `wt_longstatus_print_updated()`,\n> and simplify the logic in the latter function to take advantage of the\n> logic shifted to the former. This means that callers do not need to use\n> `wt_longstatus_print_updated()` to collect the `commitable` flag;\n> calling `wt_status_collect()` is sufficient.\n>\n> As a result, invoking `git commit` with `--short` or `--porcelain`\n> (which imply `--dry-run`, but previously returned an inconsistent error\n> code inconsistent with dry run behavior) correctly returns status code\n> zero when there is something to commit. This fixes two bugs documented\n> in the test suite.\n\n\nHmm, I couldn't quite get what the above two paragraphs were trying\nto say, but I think I figured out by looking at wt_status.c before\napplying this patch, so let me see if I correctly understand what\nthis patch is about by thinking a bit aloud.\n\nThere are only two assignments to s->commitable in wt-status.c; one\nhappens in wt_longstatus_print_updated(), when the function notices\nthere is even one record to be shown (i.e. there is an \"updated\"\npath) and the other in show_merge_in_progress() which is called by\nwt_longstatus_prpint_state(). The latter codepath happens when we\nare in a merge and there is no remaining conflicted paths (the code\nallows the contents to be committed to be identical to HEAD). Both\nare called from wt_longstatus_print(), which in turn is called by\nwt_status_print().\n\nThe implication of the above observation is that we do not set\ncommitable bit (by the way, shouldn't we spell it with two 'T's?)\nif we are not doing the long format status. The title hints at it\nbut \"fix\" is too vague. It would be easier to understand if it\nbegan like this (i.e. state problem clearly first, before outlining\nthe solution):\n\n\t[PATCH 1/2] commit: fix exit status under --short/--porcelain options\n\n\tIn wt-status.c, s->commitable bit is set only in the\n\tcodepaths reachable from wt_status_print() when output\n\tformat is STATUS_FORMAT_LONG as a side effect of printing\n\tbits of status. Consequently, when running with --short and\n\t--porcelain options, the bit is not set to reflect if there\n\tis anything to be committed, and \"git commit --short\" or\n\t\"--porcelain\" (both of which imply \"--dry-run\") failed to\n\tsignal if there is anything to commit with its exit status.\n\n\tInstead, update s->commitable bit in wt_status_collect(),\n\tregardless of the output format. ...\n\nIs that what is going on here? Yours made it sound as if moving the\ncode to _collect() was done for the sake of moving code around and\nsimplifying the logic, and bugfix fell out of the move merely as a\nside effect, which probably was the source of my confusion.\n\n> +static void wt_status_mark_commitable(struct wt_status *s) {\n> +\tint i;\n> +\n> +\tfor (i = 0; i < s->change.nr; i++) {\n> +\t\tstruct wt_status_change_data *d = (s->change.items[i]).util;\n> +\n> +\t\tif (d->index_status && d->index_status != DIFF_STATUS_UNMERGED) {\n> +\t\t\ts->commitable = 1;\n> +\t\t\treturn;\n> +\t\t}\n> +\t}\n> +}\n\nI am not sure if this is sufficient. From a cursory look of the\nexisting code (and vague recollection in my ageing brain ;-), I\nthink we say it is committable if\n\n (1) when not merging, there is something to show in the \"to be\n committed\" section (i.e. there must be something changed since\n HEAD in the index).\n\n (2) when merging, no conflicting paths remain (i.e. change.nr being\n zero is fine).\n\nSo it is unclear to me how you are dealing with (2) under \"--short\"\noption, which does not call show_merge_in_progress() to catch that\ncase.\n"},{"id":"346460","messageId":"CAJZjrdVXKx9em=soHSm0P3BCRffyrc23tqNJuFNuT6EG5hivow@mail.gmail.com","threadId":"48321","inReplyTo":"xmqq36zawqr3.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/2] commit: fix --short and --porcelain options","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-05-02T15:52:56Z","receivedAt":"2018-05-02T15:53:37Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Tue, May 1, 2018 at 10:50 PM Junio C Hamano wrote:\n\n> Samuel Lijin writes:\n\n> > Mark the commitable flag in the wt_status object in the call to\n> > `wt_status_collect()`, instead of in `wt_longstatus_print_updated()`,\n> > and simplify the logic in the latter function to take advantage of the\n> > logic shifted to the former. This means that callers do not need to use\n> > `wt_longstatus_print_updated()` to collect the `commitable` flag;\n> > calling `wt_status_collect()` is sufficient.\n> >\n> > As a result, invoking `git commit` with `--short` or `--porcelain`\n> > (which imply `--dry-run`, but previously returned an inconsistent error\n> > code inconsistent with dry run behavior) correctly returns status code\n> > zero when there is something to commit. This fixes two bugs documented\n> > in the test suite.\n\n\n> Hmm, I couldn't quite get what the above two paragraphs were trying\n> to say, but I think I figured out by looking at wt_status.c before\n> applying this patch, so let me see if I correctly understand what\n> this patch is about by thinking a bit aloud.\n\n> There are only two assignments to s->commitable in wt-status.c; one\n> happens in wt_longstatus_print_updated(), when the function notices\n> there is even one record to be shown (i.e. there is an \"updated\"\n> path) and the other in show_merge_in_progress() which is called by\n> wt_longstatus_prpint_state(). The latter codepath happens when we\n> are in a merge and there is no remaining conflicted paths (the code\n> allows the contents to be committed to be identical to HEAD). Both\n> are called from wt_longstatus_print(), which in turn is called by\n> wt_status_print().\n\n> The implication of the above observation is that we do not set\n> commitable bit (by the way, shouldn't we spell it with two 'T's?)\n\nYep, MW confirms: https://www.merriam-webster.com/dictionary/commitable\n\nI didn't think to check how common \"commitable\" is in the codebase, but it\ndoesn't seem to be too many, looking at the output of `git grep\ncommitable`, so I'll add that to the patch series when I reroll.\n\n> if we are not doing the long format status. The title hints at it\n> but \"fix\" is too vague. It would be easier to understand if it\n> began like this (i.e. state problem clearly first, before outlining\n> the solution):\n\n> [PATCH 1/2] commit: fix exit status under --short/--porcelain\noptions\n\n> In wt-status.c, s->commitable bit is set only in the\n> codepaths reachable from wt_status_print() when output\n> format is STATUS_FORMAT_LONG as a side effect of printing\n> bits of status. Consequently, when running with --short and\n> --porcelain options, the bit is not set to reflect if there\n> is anything to be committed, and \"git commit --short\" or\n> \"--porcelain\" (both of which imply \"--dry-run\") failed to\n> signal if there is anything to commit with its exit status.\n\n> Instead, update s->commitable bit in wt_status_collect(),\n> regardless of the output format. ...\n\n> Is that what is going on here? Yours made it sound as if moving the\n> code to _collect() was done for the sake of moving code around and\n> simplifying the logic, and bugfix fell out of the move merely as a\n> side effect, which probably was the source of my confusion.\n\nYep, that's right. I wasn't sure if the imperative tone was required for\nthe whole commit or just the description, hence the awkward structure. (I\nalso wasn't sure how strict the 70 char limit on the description was.)\n\n> > +static void wt_status_mark_commitable(struct wt_status *s) {\n> > + int i;\n> > +\n> > + for (i = 0; i < s->change.nr; i++) {\n> > + struct wt_status_change_data *d =\n(s->change.items[i]).util;\n> > +\n> > + if (d->index_status && d->index_status !=\nDIFF_STATUS_UNMERGED) {\n> > + s->commitable = 1;\n> > + return;\n> > + }\n> > + }\n> > +}\n\n> I am not sure if this is sufficient. From a cursory look of the\n> existing code (and vague recollection in my ageing brain ;-), I\n> think we say it is committable if\n\n> (1) when not merging, there is something to show in the \"to be\n> committed\" section (i.e. there must be something changed since\n> HEAD in the index).\n\n> (2) when merging, no conflicting paths remain (i.e. change.nr being\n> zero is fine).\n\n> So it is unclear to me how you are dealing with (2) under \"--short\"\n> option, which does not call show_merge_in_progress() to catch that\n> case.\n\nAnd the answer there is that I'm not :) I had hoped that there was a test\nto catch mistakes like this but evidently not. Thanks for pointing that\nout, I'll add a test to catch that.\n\nI'm also realizing that I didn't const-ify wt_longstatus_print() in the\nnext patch, which is another reason I didn't catch this.\n\nThis seems a bit trickier to handle. What do you think of this approach:\n(1) move wt_status_state into wt_status and (2) move the call to\nwt_status_get_state from wt_longstatus_print/wt_porcelain_v2_print_tracking\nto wt_status_collect?\n\n(I kind of also want to suggest enum-ifying some of wt_status_state, but\nthat feels beyond the scope of this and also prone to other pitfalls.)\n"},{"id":"352578","messageId":"20180715110807.25544-1-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180426092524.25264-1-sxlijin@gmail.com","subject":"[PATCH v3 0/3] Fix --short/--porcelain options for git commit","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-07-15T11:08:04Z","receivedAt":"2018-07-15T21:56:45Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Take 3. Addressed the issue that Junio turned up the last time I sent\nthis out for review.\n\nI'm not entirely sure I like the way I added the tests in the first\npatch, but it's unclear to me if there's actually a pattern for setting\nup and tearing down the same env for multiple test methods. There are\nalso other tests in t7501 that rely on state left from earlier tests, so\nit's not really clear to me what the best thing to do here is.\n\nAlso added a FIXME in the second patch for something I think should be\nfixed, but doesn't make sense to fix in this patch series.\n\nSamuel Lijin (3):\n t7501: add merge conflict tests for dry run\n wt-status: teach wt_status_collect about merges in progress\n commit: fix exit code for --short/--porcelain\n\n builtin/commit.c | 32 +++---\n ref-filter.c | 3 +-\n t/t7501-commit.sh | 49 +++++++--\n wt-status.c | 260 +++++++++++++++++++++++++---------------------\n wt-status.h | 13 +--\n 5 files changed, 208 insertions(+), 149 deletions(-)\n\n-- \n2.18.0\n\n"},{"id":"352579","messageId":"20180715110807.25544-2-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180426092524.25264-1-sxlijin@gmail.com","subject":"[PATCH v3 1/3] t7501: add merge conflict tests for dry run","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-07-15T11:08:05Z","receivedAt":"2018-07-15T21:56:48Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"The behavior of git commit when doing a dry run changes if there are\nunfixed/fixed merge conflits, but the test suite currently only asserts\nthat `git commit --dry-run` succeeds when all merge conflicts are fixed.\n\nAdd tests to document the behavior of all flags which imply a dry run\nwhen (1) there is at least one unfixed merge conflict and (2) when all\nmerge conflicts are all fixed.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7501-commit.sh | 45 ++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 40 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex fa61b1a4e..be087e73f 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -652,7 +652,8 @@ test_expect_success '--only works on to-be-born branch' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success '--dry-run with conflicts fixed from a merge' '\n+# set up env for tests of --dry-run given fixed/unfixed merge conflicts\n+test_expect_success 'setup env with unfixed merge conflicts' '\n \t# setup two branches with conflicting information\n \t# in the same file, resolve the conflict,\n \t# call commit with --dry-run\n@@ -665,11 +666,45 @@ test_expect_success '--dry-run with conflicts fixed from a merge' '\n \tgit checkout -b branch-2 HEAD^1 &&\n \techo \"commit-2-state\" >test-file &&\n \tgit commit -m \"commit 2\" -i test-file &&\n-\t! $(git merge --no-commit commit-1) &&\n-\techo \"commit-2-state\" >test-file &&\n+\ttest_expect_code 1 git merge --no-commit commit-1\n+'\n+\n+test_expect_success '--dry-run with unfixed merge conflicts' '\n+\ttest_expect_code 1 git commit --dry-run\n+'\n+\n+test_expect_success '--short with unfixed merge conflicts' '\n+\ttest_expect_code 1 git commit --short\n+'\n+\n+test_expect_success '--porcelain with unfixed merge conflicts' '\n+\ttest_expect_code 1 git commit --porcelain\n+'\n+\n+test_expect_success '--long with unfixed merge conflicts' '\n+\ttest_expect_code 1 git commit --long\n+'\n+\n+test_expect_success '--dry-run with conflicts fixed from a merge' '\n+\techo \"merge-conflicts-fixed\" >test-file &&\n \tgit add test-file &&\n-\tgit commit --dry-run &&\n-\tgit commit -m \"conflicts fixed from merge.\"\n+\tgit commit --dry-run\n+'\n+\n+test_expect_failure '--short with conflicts fixed from a merge' '\n+\tgit commit --short\n+'\n+\n+test_expect_failure '--porcelain with conflicts fixed from a merge' '\n+\tgit commit --porcelain\n+'\n+\n+test_expect_success '--long with conflicts fixed from a merge' '\n+\tgit commit --long\n+'\n+\n+test_expect_success '--message with conflicts fixed from a merge' '\n+\tgit commit --message \"conflicts fixed from merge.\"\n '\n \n test_done\n-- \n2.18.0\n\n"},{"id":"352580","messageId":"20180715110807.25544-4-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180426092524.25264-1-sxlijin@gmail.com","subject":"[PATCH v3 3/3] commit: fix exit code for --short/--porcelain","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-07-15T11:08:07Z","receivedAt":"2018-07-15T21:56:55Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"In wt-status.c, the s->commitable bit is set only in the call tree of\nwt_longstatus_print(), which means that when there are changes to be\ncommitted or all merge conflicts have been resolved, --dry-run and\n--long return the correct exit code, but --short and --porcelain do not,\neven though they both imply --dry-run.\n\nTeach wt_status_collect() to set s->committable correctly so that\n--short and --porcelain return the correct exit code in the above\ndescribed situations and mark the documenting tests as fixed.\n\nAlso stop setting s->committable in wt_longstatus_print_updated() and\nshow_merge_in_progress(), and const-ify wt_status_state in the method\nsignatures in those callpaths.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7501-commit.sh | 8 ++---\n wt-status.c | 82 +++++++++++++++++++++++++++++------------------\n 2 files changed, 55 insertions(+), 35 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex be087e73f..b6492322f 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -87,12 +87,12 @@ test_expect_success '--dry-run with stuff to commit returns ok' '\n \tgit commit -m next -a --dry-run\n '\n \n-test_expect_failure '--short with stuff to commit returns ok' '\n+test_expect_success '--short with stuff to commit returns ok' '\n \techo bongo bongo bongo >>file &&\n \tgit commit -m next -a --short\n '\n \n-test_expect_failure '--porcelain with stuff to commit returns ok' '\n+test_expect_success '--porcelain with stuff to commit returns ok' '\n \techo bongo bongo bongo >>file &&\n \tgit commit -m next -a --porcelain\n '\n@@ -691,11 +691,11 @@ test_expect_success '--dry-run with conflicts fixed from a merge' '\n \tgit commit --dry-run\n '\n \n-test_expect_failure '--short with conflicts fixed from a merge' '\n+test_expect_success '--short with conflicts fixed from a merge' '\n \tgit commit --short\n '\n \n-test_expect_failure '--porcelain with conflicts fixed from a merge' '\n+test_expect_success '--porcelain with conflicts fixed from a merge' '\n \tgit commit --porcelain\n '\n \ndiff --git a/wt-status.c b/wt-status.c\nindex 75d389944..4ba657978 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -718,6 +718,39 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \t\ts->untracked_in_ms = (getnanotime() - t_begin) / 1000000;\n }\n \n+static int has_unmerged(const struct wt_status *s)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < s->change.nr; i++) {\n+\t\tstruct wt_status_change_data *d;\n+\t\td = s->change.items[i].util;\n+\t\tif (d->stagemask)\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static void wt_status_mark_committable(\n+\t\tstruct wt_status *s, const struct wt_status_state *state)\n+{\n+\tint i;\n+\n+\tif (state->merge_in_progress && !has_unmerged(s)) {\n+\t\ts->committable = 1;\n+\t\treturn;\n+\t}\n+\n+\tfor (i = 0; i < s->change.nr; i++) {\n+\t\tstruct wt_status_change_data *d = (s->change.items[i]).util;\n+\n+\t\tif (d->index_status && d->index_status != DIFF_STATUS_UNMERGED) {\n+\t\t\ts->committable = 1;\n+\t\t\treturn;\n+\t\t}\n+\t}\n+}\n+\n void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)\n {\n \twt_status_collect_changes_worktree(s);\n@@ -728,6 +761,8 @@ void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)\n \t\twt_status_collect_changes_index(s);\n \n \twt_status_collect_untracked(s);\n+\n+\twt_status_mark_committable(s, state);\n }\n \n static void wt_longstatus_print_unmerged(const struct wt_status *s)\n@@ -753,28 +788,28 @@ static void wt_longstatus_print_unmerged(const struct wt_status *s)\n \n }\n \n-static void wt_longstatus_print_updated(struct wt_status *s)\n+static void wt_longstatus_print_updated(const struct wt_status *s)\n {\n-\tint shown_header = 0;\n \tint i;\n \n+\tif (!s->committable) {\n+\t\treturn;\n+\t}\n+\n+\twt_longstatus_print_cached_header(s);\n+\n \tfor (i = 0; i < s->change.nr; i++) {\n \t\tstruct wt_status_change_data *d;\n \t\tstruct string_list_item *it;\n \t\tit = &(s->change.items[i]);\n \t\td = it->util;\n-\t\tif (!d->index_status ||\n-\t\t d->index_status == DIFF_STATUS_UNMERGED)\n-\t\t\tcontinue;\n-\t\tif (!shown_header) {\n-\t\t\twt_longstatus_print_cached_header(s);\n-\t\t\ts->committable = 1;\n-\t\t\tshown_header = 1;\n+\t\tif (d->index_status &&\n+\t\t d->index_status != DIFF_STATUS_UNMERGED) {\n+\t\t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n \t\t}\n-\t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n \t}\n-\tif (shown_header)\n-\t\twt_longstatus_print_trailer(s);\n+\n+\twt_longstatus_print_trailer(s);\n }\n \n /*\n@@ -1056,21 +1091,7 @@ static void wt_longstatus_print_tracking(const struct wt_status *s)\n \tstrbuf_release(&sb);\n }\n \n-static int has_unmerged(const struct wt_status *s)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < s->change.nr; i++) {\n-\t\tstruct wt_status_change_data *d;\n-\t\td = s->change.items[i].util;\n-\t\tif (d->stagemask)\n-\t\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n-static void show_merge_in_progress(struct wt_status *s,\n-\t\t\t\tconst struct wt_status_state *state,\n+static void show_merge_in_progress(const struct wt_status *s,\n \t\t\t\tconst char *color)\n {\n \tif (has_unmerged(s)) {\n@@ -1082,7 +1103,6 @@ static void show_merge_in_progress(struct wt_status *s,\n \t\t\t\t\t _(\" (use \\\"git merge --abort\\\" to abort the merge)\"));\n \t\t}\n \t} else {\n-\t\ts-> committable = 1;\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"All conflicts fixed but you are still merging.\"));\n \t\tif (s->hints)\n@@ -1576,12 +1596,12 @@ void wt_status_clear_state(struct wt_status_state *state)\n \tfree(state->detached_from);\n }\n \n-static void wt_longstatus_print_state(struct wt_status *s,\n+static void wt_longstatus_print_state(const struct wt_status *s,\n \t\t\t\t const struct wt_status_state *state)\n {\n \tconst char *state_color = color(WT_STATUS_HEADER, s);\n \tif (state->merge_in_progress)\n-\t\tshow_merge_in_progress(s, state, state_color);\n+\t\tshow_merge_in_progress(s, state_color);\n \telse if (state->am_in_progress)\n \t\tshow_am_in_progress(s, state, state_color);\n \telse if (state->rebase_in_progress || state->rebase_interactive_in_progress)\n@@ -1594,7 +1614,7 @@ static void wt_longstatus_print_state(struct wt_status *s,\n \t\tshow_bisect_in_progress(s, state, state_color);\n }\n \n-static void wt_longstatus_print(struct wt_status *s, const struct wt_status_state *state)\n+static void wt_longstatus_print(const struct wt_status *s, const struct wt_status_state *state)\n {\n \tconst char *branch_color = color(WT_STATUS_ONBRANCH, s);\n \tconst char *branch_status_color = color(WT_STATUS_HEADER, s);\n-- \n2.18.0\n\n"},{"id":"352581","messageId":"20180715110807.25544-3-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180426092524.25264-1-sxlijin@gmail.com","subject":"[PATCH v3 2/3] wt-status: teach wt_status_collect about merges in progress","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-07-15T11:08:06Z","receivedAt":"2018-07-15T21:56:55Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"To fix the breakages documented by t7501, the next patch in this series\nwill teach wt_status_collect() to set the committable bit, instead of\nhaving wt_longstatus_print_updated() and show_merge_in_progress() set it\n(which is what currently happens). Unfortunately, wt_status_collect()\nneeds to know whether or not there is a merge in progress to set the bit\ncorrectly, so teach its (two) callers to create, initialize, and pass\nin instances of wt_status_state, which records this metadata.\n\nSince wt_longstatus_print() and show_merge_in_progress() are in the same\ncallpaths and currently create and init copies of wt_status_state,\nremove that logic and instead pass wt_status_state through.\n\nMake wt_status_get_state easier to use, add a helper method to clean up\nwt_status_state, const-ify as many struct pointers in method signatures\nas possible, and add a FIXME for a struct pointer which should be const\nbut isn't (that this patch series will not address).\n\nSigned-off-by: Samuel Lijin \n---\n builtin/commit.c | 32 ++++----\n ref-filter.c | 3 +-\n wt-status.c | 188 +++++++++++++++++++++++------------------------\n wt-status.h | 13 ++--\n 4 files changed, 120 insertions(+), 116 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 37fcb55ab..79ef4f11a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -463,6 +463,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn,\n \t\t struct wt_status *s)\n {\n+\tstruct wt_status_state state;\n \tstruct object_id oid;\n \n \tif (s->relative_paths)\n@@ -482,10 +483,12 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \ts->status_format = status_format;\n \ts->ignore_submodule_arg = ignore_submodule_arg;\n \n-\twt_status_collect(s);\n-\twt_status_print(s);\n+\twt_status_get_state(s, &state);\n+\twt_status_collect(s, &state);\n+\twt_status_print(s, &state);\n+\twt_status_clear_state(&state);\n \n-\treturn s->commitable;\n+\treturn s->committable;\n }\n \n static int is_a_merge(const struct commit *current_head)\n@@ -631,7 +634,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n {\n \tstruct stat statbuf;\n \tstruct strbuf committer_ident = STRBUF_INIT;\n-\tint commitable;\n+\tint committable;\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *hook_arg1 = NULL;\n \tconst char *hook_arg2 = NULL;\n@@ -848,7 +851,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \n \t\tsaved_color_setting = s->use_color;\n \t\ts->use_color = 0;\n-\t\tcommitable = run_status(s->fp, index_file, prefix, 1, s);\n+\t\tcommittable = run_status(s->fp, index_file, prefix, 1, s);\n \t\ts->use_color = saved_color_setting;\n \t} else {\n \t\tstruct object_id oid;\n@@ -866,7 +869,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\tfor (i = 0; i < active_nr; i++)\n \t\t\t\tif (ce_intent_to_add(active_cache[i]))\n \t\t\t\t\tita_nr++;\n-\t\t\tcommitable = active_nr - ita_nr > 0;\n+\t\t\tcommittable = active_nr - ita_nr > 0;\n \t\t} else {\n \t\t\t/*\n \t\t\t * Unless the user did explicitly request a submodule\n@@ -882,7 +885,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\tif (ignore_submodule_arg &&\n \t\t\t !strcmp(ignore_submodule_arg, \"all\"))\n \t\t\t\tflags.ignore_submodules = 1;\n-\t\t\tcommitable = index_differs_from(parent, &flags, 1);\n+\t\t\tcommittable = index_differs_from(parent, &flags, 1);\n \t\t}\n \t}\n \tstrbuf_release(&committer_ident);\n@@ -894,7 +897,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t * explicit --allow-empty. In the cherry-pick case, it may be\n \t * empty due to conflict resolution, which the user should okay.\n \t */\n-\tif (!commitable && whence != FROM_MERGE && !allow_empty &&\n+\tif (!committable && whence != FROM_MERGE && !allow_empty &&\n \t !(amend && is_a_merge(current_head))) {\n \t\ts->display_comment_prefix = old_display_comment_prefix;\n \t\trun_status(stdout, index_file, prefix, 0, s);\n@@ -1164,14 +1167,14 @@ static int parse_and_validate_options(int argc, const char *argv[],\n static int dry_run_commit(int argc, const char **argv, const char *prefix,\n \t\t\t const struct commit *current_head, struct wt_status *s)\n {\n-\tint commitable;\n+\tint committable;\n \tconst char *index_file;\n \n \tindex_file = prepare_index(argc, argv, prefix, current_head, 1);\n-\tcommitable = run_status(stdout, index_file, prefix, 0, s);\n+\tcommittable = run_status(stdout, index_file, prefix, 0, s);\n \trollback_index_files();\n \n-\treturn commitable ? 0 : 1;\n+\treturn committable ? 0 : 1;\n }\n \n static int parse_status_slot(const char *slot)\n@@ -1266,6 +1269,7 @@ static int git_status_config(const char *k, const char *v, void *cb)\n int cmd_status(int argc, const char **argv, const char *prefix)\n {\n \tstatic struct wt_status s;\n+\tstruct wt_status_state state;\n \tint fd;\n \tstruct object_id oid;\n \tstatic struct option builtin_status_options[] = {\n@@ -1338,7 +1342,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \ts.status_format = status_format;\n \ts.verbose = verbose;\n \n-\twt_status_collect(&s);\n+\twt_status_get_state(&s, &state);\n+\twt_status_collect(&s, &state);\n \n \tif (0 <= fd)\n \t\tupdate_index_if_able(&the_index, &index_lock);\n@@ -1346,7 +1351,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \tif (s.relative_paths)\n \t\ts.prefix = prefix;\n \n-\twt_status_print(&s);\n+\twt_status_print(&s, &state);\n+\twt_status_clear_state(&state);\n \treturn 0;\n }\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex 9a333e21b..280ef9713 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1306,8 +1306,7 @@ char *get_head_description(void)\n {\n \tstruct strbuf desc = STRBUF_INIT;\n \tstruct wt_status_state state;\n-\tmemset(&state, 0, sizeof(state));\n-\twt_status_get_state(&state, 1);\n+\twt_status_get_state(NULL, &state);\n \tif (state.rebase_in_progress ||\n \t state.rebase_interactive_in_progress)\n \t\tstrbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\ndiff --git a/wt-status.c b/wt-status.c\nindex 50815e5fa..75d389944 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -33,7 +33,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_NIL, /* WT_STATUS_ONBRANCH */\n };\n \n-static const char *color(int slot, struct wt_status *s)\n+static const char *color(int slot, const struct wt_status *s)\n {\n \tconst char *c = \"\";\n \tif (want_color(s->use_color))\n@@ -43,7 +43,7 @@ static const char *color(int slot, struct wt_status *s)\n \treturn c;\n }\n \n-static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n+static void status_vprintf(const struct wt_status *s, int at_bol, const char *color,\n \t\tconst char *fmt, va_list ap, const char *trail)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -89,7 +89,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n \tstrbuf_release(&sb);\n }\n \n-void status_printf_ln(struct wt_status *s, const char *color,\n+void status_printf_ln(const struct wt_status *s, const char *color,\n \t\t\tconst char *fmt, ...)\n {\n \tva_list ap;\n@@ -99,7 +99,7 @@ void status_printf_ln(struct wt_status *s, const char *color,\n \tva_end(ap);\n }\n \n-void status_printf(struct wt_status *s, const char *color,\n+void status_printf(const struct wt_status *s, const char *color,\n \t\t\tconst char *fmt, ...)\n {\n \tva_list ap;\n@@ -109,7 +109,7 @@ void status_printf(struct wt_status *s, const char *color,\n \tva_end(ap);\n }\n \n-static void status_printf_more(struct wt_status *s, const char *color,\n+static void status_printf_more(const struct wt_status *s, const char *color,\n \t\t\t const char *fmt, ...)\n {\n \tva_list ap;\n@@ -140,7 +140,7 @@ void wt_status_prepare(struct wt_status *s)\n \ts->display_comment_prefix = 0;\n }\n \n-static void wt_longstatus_print_unmerged_header(struct wt_status *s)\n+static void wt_longstatus_print_unmerged_header(const struct wt_status *s)\n {\n \tint i;\n \tint del_mod_conflict = 0;\n@@ -192,7 +192,7 @@ static void wt_longstatus_print_unmerged_header(struct wt_status *s)\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_cached_header(struct wt_status *s)\n+static void wt_longstatus_print_cached_header(const struct wt_status *s)\n {\n \tconst char *c = color(WT_STATUS_HEADER, s);\n \n@@ -208,7 +208,7 @@ static void wt_longstatus_print_cached_header(struct wt_status *s)\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_dirty_header(struct wt_status *s,\n+static void wt_longstatus_print_dirty_header(const struct wt_status *s,\n \t\t\t\t\t int has_deleted,\n \t\t\t\t\t int has_dirty_submodules)\n {\n@@ -227,7 +227,7 @@ static void wt_longstatus_print_dirty_header(struct wt_status *s,\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_other_header(struct wt_status *s,\n+static void wt_longstatus_print_other_header(const struct wt_status *s,\n \t\t\t\t\t const char *what,\n \t\t\t\t\t const char *how)\n {\n@@ -239,7 +239,7 @@ static void wt_longstatus_print_other_header(struct wt_status *s,\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_trailer(struct wt_status *s)\n+static void wt_longstatus_print_trailer(const struct wt_status *s)\n {\n \tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n }\n@@ -305,7 +305,7 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval)\n \treturn result;\n }\n \n-static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n+static void wt_longstatus_print_unmerged_data(const struct wt_status *s,\n \t\t\t\t\t struct string_list_item *it)\n {\n \tconst char *c = color(WT_STATUS_UNMERGED, s);\n@@ -332,7 +332,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n \tstrbuf_release(&onebuf);\n }\n \n-static void wt_longstatus_print_change_data(struct wt_status *s,\n+static void wt_longstatus_print_change_data(const struct wt_status *s,\n \t\t\t\t\t int change_type,\n \t\t\t\t\t struct string_list_item *it)\n {\n@@ -718,7 +718,7 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \t\ts->untracked_in_ms = (getnanotime() - t_begin) / 1000000;\n }\n \n-void wt_status_collect(struct wt_status *s)\n+void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)\n {\n \twt_status_collect_changes_worktree(s);\n \n@@ -726,10 +726,11 @@ void wt_status_collect(struct wt_status *s)\n \t\twt_status_collect_changes_initial(s);\n \telse\n \t\twt_status_collect_changes_index(s);\n+\n \twt_status_collect_untracked(s);\n }\n \n-static void wt_longstatus_print_unmerged(struct wt_status *s)\n+static void wt_longstatus_print_unmerged(const struct wt_status *s)\n {\n \tint shown_header = 0;\n \tint i;\n@@ -767,7 +768,7 @@ static void wt_longstatus_print_updated(struct wt_status *s)\n \t\t\tcontinue;\n \t\tif (!shown_header) {\n \t\t\twt_longstatus_print_cached_header(s);\n-\t\t\ts->commitable = 1;\n+\t\t\ts->committable = 1;\n \t\t\tshown_header = 1;\n \t\t}\n \t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n@@ -781,7 +782,7 @@ static void wt_longstatus_print_updated(struct wt_status *s)\n * 0 : no change\n * 1 : some change but no delete\n */\n-static int wt_status_check_worktree_changes(struct wt_status *s,\n+static int wt_status_check_worktree_changes(const struct wt_status *s,\n \t\t\t\t\t int *dirty_submodules)\n {\n \tint i;\n@@ -805,7 +806,7 @@ static int wt_status_check_worktree_changes(struct wt_status *s,\n \treturn changes;\n }\n \n-static void wt_longstatus_print_changed(struct wt_status *s)\n+static void wt_longstatus_print_changed(const struct wt_status *s)\n {\n \tint i, dirty_submodules;\n \tint worktree_changes = wt_status_check_worktree_changes(s, &dirty_submodules);\n@@ -837,7 +838,7 @@ static int stash_count_refs(struct object_id *ooid, struct object_id *noid,\n \treturn 0;\n }\n \n-static void wt_longstatus_print_stash_summary(struct wt_status *s)\n+static void wt_longstatus_print_stash_summary(const struct wt_status *s)\n {\n \tint stash_count = 0;\n \n@@ -849,7 +850,7 @@ static void wt_longstatus_print_stash_summary(struct wt_status *s)\n \t\t\t\t stash_count);\n }\n \n-static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncommitted)\n+static void wt_longstatus_print_submodule_summary(const struct wt_status *s, int uncommitted)\n {\n \tstruct child_process sm_summary = CHILD_PROCESS_INIT;\n \tstruct strbuf cmd_stdout = STRBUF_INIT;\n@@ -895,8 +896,8 @@ static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncom\n \tstrbuf_release(&summary);\n }\n \n-static void wt_longstatus_print_other(struct wt_status *s,\n-\t\t\t\t struct string_list *l,\n+static void wt_longstatus_print_other(const struct wt_status *s,\n+\t\t\t\t const struct string_list *l,\n \t\t\t\t const char *what,\n \t\t\t\t const char *how)\n {\n@@ -969,7 +970,7 @@ void wt_status_add_cut_line(FILE *fp)\n \tstrbuf_release(&buf);\n }\n \n-static void wt_longstatus_print_verbose(struct wt_status *s)\n+static void wt_longstatus_print_verbose(const struct wt_status *s)\n {\n \tstruct rev_info rev;\n \tstruct setup_revision_opt opt;\n@@ -1000,7 +1001,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \t\trev.diffopt.use_color = 0;\n \t\twt_status_add_cut_line(s->fp);\n \t}\n-\tif (s->verbose > 1 && s->commitable) {\n+\tif (s->verbose > 1 && s->committable) {\n \t\t/* print_updated() printed a header, so do we */\n \t\tif (s->fp != stdout)\n \t\t\twt_longstatus_print_trailer(s);\n@@ -1021,7 +1022,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \t}\n }\n \n-static void wt_longstatus_print_tracking(struct wt_status *s)\n+static void wt_longstatus_print_tracking(const struct wt_status *s)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *cp, *ep, *branch_name;\n@@ -1055,7 +1056,7 @@ static void wt_longstatus_print_tracking(struct wt_status *s)\n \tstrbuf_release(&sb);\n }\n \n-static int has_unmerged(struct wt_status *s)\n+static int has_unmerged(const struct wt_status *s)\n {\n \tint i;\n \n@@ -1069,7 +1070,7 @@ static int has_unmerged(struct wt_status *s)\n }\n \n static void show_merge_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n+\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tif (has_unmerged(s)) {\n@@ -1081,7 +1082,7 @@ static void show_merge_in_progress(struct wt_status *s,\n \t\t\t\t\t _(\" (use \\\"git merge --abort\\\" to abort the merge)\"));\n \t\t}\n \t} else {\n-\t\ts-> commitable = 1;\n+\t\ts-> committable = 1;\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"All conflicts fixed but you are still merging.\"));\n \t\tif (s->hints)\n@@ -1091,8 +1092,8 @@ static void show_merge_in_progress(struct wt_status *s,\n \twt_longstatus_print_trailer(s);\n }\n \n-static void show_am_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n+static void show_am_in_progress(const struct wt_status *s,\n+\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tstatus_printf_ln(s, color,\n@@ -1130,7 +1131,7 @@ static char *read_line_from_git_path(const char *filename)\n \t}\n }\n \n-static int split_commit_in_progress(struct wt_status *s)\n+static int split_commit_in_progress(const struct wt_status *s)\n {\n \tint split_in_progress = 0;\n \tchar *head, *orig_head, *rebase_amend, *rebase_orig_head;\n@@ -1224,8 +1225,8 @@ static int read_rebase_todolist(const char *fname, struct string_list *lines)\n \treturn 0;\n }\n \n-static void show_rebase_information(struct wt_status *s,\n-\t\t\t\t\tstruct wt_status_state *state,\n+static void show_rebase_information(const struct wt_status *s,\n+\t\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\t\tconst char *color)\n {\n \tif (state->rebase_interactive_in_progress) {\n@@ -1278,8 +1279,8 @@ static void show_rebase_information(struct wt_status *s,\n \t}\n }\n \n-static void print_rebase_state(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n+static void print_rebase_state(const struct wt_status *s,\n+\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tif (state->branch)\n@@ -1292,8 +1293,8 @@ static void print_rebase_state(struct wt_status *s,\n \t\t\t\t _(\"You are currently rebasing.\"));\n }\n \n-static void show_rebase_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n+static void show_rebase_in_progress(const struct wt_status *s,\n+\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tstruct stat st;\n@@ -1345,8 +1346,8 @@ static void show_rebase_in_progress(struct wt_status *s,\n \twt_longstatus_print_trailer(s);\n }\n \n-static void show_cherry_pick_in_progress(struct wt_status *s,\n-\t\t\t\t\tstruct wt_status_state *state,\n+static void show_cherry_pick_in_progress(const struct wt_status *s,\n+\t\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\t\tconst char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently cherry-picking commit %s.\"),\n@@ -1364,8 +1365,8 @@ static void show_cherry_pick_in_progress(struct wt_status *s,\n \twt_longstatus_print_trailer(s);\n }\n \n-static void show_revert_in_progress(struct wt_status *s,\n-\t\t\t\t\tstruct wt_status_state *state,\n+static void show_revert_in_progress(const struct wt_status *s,\n+\t\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\t\tconst char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently reverting commit %s.\"),\n@@ -1383,8 +1384,8 @@ static void show_revert_in_progress(struct wt_status *s,\n \twt_longstatus_print_trailer(s);\n }\n \n-static void show_bisect_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n+static void show_bisect_in_progress(const struct wt_status *s,\n+\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tif (state->branch)\n@@ -1538,12 +1539,16 @@ int wt_status_check_bisect(const struct worktree *wt,\n \treturn 0;\n }\n \n-void wt_status_get_state(struct wt_status_state *state,\n-\t\t\t int get_detached_from)\n+void wt_status_get_state(\n+\t\tconst struct wt_status *s, struct wt_status_state *state)\n {\n+\tint get_detached_from =\n+\t\t(s == NULL) || (s->branch && !strcmp(s->branch, \"HEAD\"));\n \tstruct stat st;\n \tstruct object_id oid;\n \n+\tmemset(state, 0, sizeof(*state));\n+\n \tif (!stat(git_path_merge_head(), &st)) {\n \t\tstate->merge_in_progress = 1;\n \t} else if (wt_status_check_rebase(NULL, state)) {\n@@ -1564,8 +1569,15 @@ void wt_status_get_state(struct wt_status_state *state,\n \t\twt_status_get_detached_from(state);\n }\n \n+void wt_status_clear_state(struct wt_status_state *state)\n+{\n+\tfree(state->branch);\n+\tfree(state->onto);\n+\tfree(state->detached_from);\n+}\n+\n static void wt_longstatus_print_state(struct wt_status *s,\n-\t\t\t\t struct wt_status_state *state)\n+\t\t\t\t const struct wt_status_state *state)\n {\n \tconst char *state_color = color(WT_STATUS_HEADER, s);\n \tif (state->merge_in_progress)\n@@ -1582,30 +1594,25 @@ static void wt_longstatus_print_state(struct wt_status *s,\n \t\tshow_bisect_in_progress(s, state, state_color);\n }\n \n-static void wt_longstatus_print(struct wt_status *s)\n+static void wt_longstatus_print(struct wt_status *s, const struct wt_status_state *state)\n {\n \tconst char *branch_color = color(WT_STATUS_ONBRANCH, s);\n \tconst char *branch_status_color = color(WT_STATUS_HEADER, s);\n-\tstruct wt_status_state state;\n-\n-\tmemset(&state, 0, sizeof(state));\n-\twt_status_get_state(&state,\n-\t\t\t s->branch && !strcmp(s->branch, \"HEAD\"));\n \n \tif (s->branch) {\n \t\tconst char *on_what = _(\"On branch \");\n \t\tconst char *branch_name = s->branch;\n \t\tif (!strcmp(branch_name, \"HEAD\")) {\n \t\t\tbranch_status_color = color(WT_STATUS_NOBRANCH, s);\n-\t\t\tif (state.rebase_in_progress || state.rebase_interactive_in_progress) {\n-\t\t\t\tif (state.rebase_interactive_in_progress)\n+\t\t\tif (state->rebase_in_progress || state->rebase_interactive_in_progress) {\n+\t\t\t\tif (state->rebase_interactive_in_progress)\n \t\t\t\t\ton_what = _(\"interactive rebase in progress; onto \");\n \t\t\t\telse\n \t\t\t\t\ton_what = _(\"rebase in progress; onto \");\n-\t\t\t\tbranch_name = state.onto;\n-\t\t\t} else if (state.detached_from) {\n-\t\t\t\tbranch_name = state.detached_from;\n-\t\t\t\tif (state.detached_at)\n+\t\t\t\tbranch_name = state->onto;\n+\t\t\t} else if (state->detached_from) {\n+\t\t\t\tbranch_name = state->detached_from;\n+\t\t\t\tif (state->detached_at)\n \t\t\t\t\ton_what = _(\"HEAD detached at \");\n \t\t\t\telse\n \t\t\t\t\ton_what = _(\"HEAD detached from \");\n@@ -1622,10 +1629,7 @@ static void wt_longstatus_print(struct wt_status *s)\n \t\t\twt_longstatus_print_tracking(s);\n \t}\n \n-\twt_longstatus_print_state(s, &state);\n-\tfree(state.branch);\n-\tfree(state.onto);\n-\tfree(state.detached_from);\n+\twt_longstatus_print_state(s, state);\n \n \tif (s->is_initial) {\n \t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n@@ -1657,14 +1661,14 @@ static void wt_longstatus_print(struct wt_status *s)\n \t\t\t\t\t \"new files yourself (see 'git help status').\"),\n \t\t\t\t\t s->untracked_in_ms / 1000.0);\n \t\t}\n-\t} else if (s->commitable)\n+\t} else if (s->committable)\n \t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, _(\"Untracked files not listed%s\"),\n \t\t\ts->hints\n \t\t\t? _(\" (use -u option to show untracked files)\") : \"\");\n \n \tif (s->verbose)\n \t\twt_longstatus_print_verbose(s);\n-\tif (!s->commitable) {\n+\tif (!s->committable) {\n \t\tif (s->amend)\n \t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, _(\"No changes\"));\n \t\telse if (s->nowarn)\n@@ -1700,7 +1704,7 @@ static void wt_longstatus_print(struct wt_status *s)\n }\n \n static void wt_shortstatus_unmerged(struct string_list_item *it,\n-\t\t\t struct wt_status *s)\n+\t\t\t const struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n \tconst char *how = \"??\";\n@@ -1727,7 +1731,7 @@ static void wt_shortstatus_unmerged(struct string_list_item *it,\n }\n \n static void wt_shortstatus_status(struct string_list_item *it,\n-\t\t\t struct wt_status *s)\n+\t\t\t const struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n \n@@ -1770,7 +1774,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n }\n \n static void wt_shortstatus_other(struct string_list_item *it,\n-\t\t\t\t struct wt_status *s, const char *sign)\n+\t\t\t\t const struct wt_status *s, const char *sign)\n {\n \tif (s->null_termination) {\n \t\tfprintf(stdout, \"%s %s%c\", sign, it->string, 0);\n@@ -1784,7 +1788,7 @@ static void wt_shortstatus_other(struct string_list_item *it,\n \t}\n }\n \n-static void wt_shortstatus_print_tracking(struct wt_status *s)\n+static void wt_shortstatus_print_tracking(const struct wt_status *s)\n {\n \tstruct branch *branch;\n \tconst char *header_color = color(WT_STATUS_HEADER, s);\n@@ -1860,7 +1864,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n \tfputc(s->null_termination ? '\\0' : '\\n', s->fp);\n }\n \n-static void wt_shortstatus_print(struct wt_status *s)\n+static void wt_shortstatus_print(const struct wt_status *s)\n {\n \tstruct string_list_item *it;\n \n@@ -1924,18 +1928,14 @@ static void wt_porcelain_print(struct wt_status *s)\n * upstream. When AHEAD_BEHIND_QUICK is requested and the branches\n * are different, '?' will be substituted for the actual count.\n */\n-static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n+static void wt_porcelain_v2_print_tracking(const struct wt_status *s, const struct wt_status_state *state)\n {\n \tstruct branch *branch;\n \tconst char *base;\n \tconst char *branch_name;\n-\tstruct wt_status_state state;\n \tint ab_info, nr_ahead, nr_behind;\n \tchar eol = s->null_termination ? '\\0' : '\\n';\n \n-\tmemset(&state, 0, sizeof(state));\n-\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n-\n \tfprintf(s->fp, \"# branch.oid %s%c\",\n \t\t\t(s->is_initial ? \"(initial)\" : sha1_to_hex(s->sha1_commit)),\n \t\t\teol);\n@@ -1946,10 +1946,10 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n \t\tif (!strcmp(s->branch, \"HEAD\")) {\n \t\t\tfprintf(s->fp, \"# branch.head %s%c\", \"(detached)\", eol);\n \n-\t\t\tif (state.rebase_in_progress || state.rebase_interactive_in_progress)\n-\t\t\t\tbranch_name = state.onto;\n-\t\t\telse if (state.detached_from)\n-\t\t\t\tbranch_name = state.detached_from;\n+\t\t\tif (state->rebase_in_progress || state->rebase_interactive_in_progress)\n+\t\t\t\tbranch_name = state->onto;\n+\t\t\telse if (state->detached_from)\n+\t\t\t\tbranch_name = state->detached_from;\n \t\t\telse\n \t\t\t\tbranch_name = \"\";\n \t\t} else {\n@@ -1983,10 +1983,6 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n \t\t\t}\n \t\t}\n \t}\n-\n-\tfree(state.branch);\n-\tfree(state.onto);\n-\tfree(state.detached_from);\n }\n \n /*\n@@ -1994,7 +1990,7 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n * fixed-length string of characters in the buffer provided.\n */\n static void wt_porcelain_v2_submodule_state(\n-\tstruct wt_status_change_data *d,\n+\tconst struct wt_status_change_data *d,\n \tchar sub[5])\n {\n \tif (S_ISGITLINK(d->mode_head) ||\n@@ -2017,8 +2013,8 @@ static void wt_porcelain_v2_submodule_state(\n * Fix-up changed entries before we print them.\n */\n static void wt_porcelain_v2_fix_up_changed(\n-\tstruct string_list_item *it,\n-\tstruct wt_status *s)\n+\tconst struct string_list_item *it,\n+\tconst struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n \n@@ -2066,8 +2062,8 @@ static void wt_porcelain_v2_fix_up_changed(\n * Print porcelain v2 info for tracked entries with changes.\n */\n static void wt_porcelain_v2_print_changed_entry(\n-\tstruct string_list_item *it,\n-\tstruct wt_status *s)\n+\tconst struct string_list_item *it,\n+\tconst struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -2130,8 +2126,8 @@ static void wt_porcelain_v2_print_changed_entry(\n * Print porcelain v2 status info for unmerged entries.\n */\n static void wt_porcelain_v2_print_unmerged_entry(\n-\tstruct string_list_item *it,\n-\tstruct wt_status *s)\n+\tconst struct string_list_item *it,\n+\tconst struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n \tconst struct cache_entry *ce;\n@@ -2211,8 +2207,8 @@ static void wt_porcelain_v2_print_unmerged_entry(\n * Print porcelain V2 status info for untracked and ignored entries.\n */\n static void wt_porcelain_v2_print_other(\n-\tstruct string_list_item *it,\n-\tstruct wt_status *s,\n+\tconst struct string_list_item *it,\n+\tconst struct wt_status *s,\n \tchar prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -2242,14 +2238,14 @@ static void wt_porcelain_v2_print_other(\n * []*\n *\n */\n-static void wt_porcelain_v2_print(struct wt_status *s)\n+static void wt_porcelain_v2_print(const struct wt_status *s, const struct wt_status_state *state)\n {\n \tstruct wt_status_change_data *d;\n \tstruct string_list_item *it;\n \tint i;\n \n \tif (s->show_branch)\n-\t\twt_porcelain_v2_print_tracking(s);\n+\t\twt_porcelain_v2_print_tracking(s, state);\n \n \tfor (i = 0; i < s->change.nr; i++) {\n \t\tit = &(s->change.items[i]);\n@@ -2276,7 +2272,9 @@ static void wt_porcelain_v2_print(struct wt_status *s)\n \t}\n }\n \n-void wt_status_print(struct wt_status *s)\n+// FIXME: `struct wt_status *` should be `const struct wt_status` but because\n+// `wt_porcelain_print()` modifies it, that has to first be fixed\n+void wt_status_print(struct wt_status *s, const struct wt_status_state *state)\n {\n \tswitch (s->status_format) {\n \tcase STATUS_FORMAT_SHORT:\n@@ -2286,14 +2284,14 @@ void wt_status_print(struct wt_status *s)\n \t\twt_porcelain_print(s);\n \t\tbreak;\n \tcase STATUS_FORMAT_PORCELAIN_V2:\n-\t\twt_porcelain_v2_print(s);\n+\t\twt_porcelain_v2_print(s, state);\n \t\tbreak;\n \tcase STATUS_FORMAT_UNSPECIFIED:\n \t\tdie(\"BUG: finalize_deferred_config() should have been called\");\n \t\tbreak;\n \tcase STATUS_FORMAT_NONE:\n \tcase STATUS_FORMAT_LONG:\n-\t\twt_longstatus_print(s);\n+\t\twt_longstatus_print(s, state);\n \t\tbreak;\n \t}\n }\ndiff --git a/wt-status.h b/wt-status.h\nindex 430770b85..6cccfd7a0 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -94,7 +94,7 @@ struct wt_status {\n \tunsigned char sha1_commit[GIT_MAX_RAWSZ]; /* when not Initial */\n \n \t/* These are computed during processing of the individual sections */\n-\tint commitable;\n+\tint committable;\n \tint workdir_dirty;\n \tconst char *index_file;\n \tFILE *fp;\n@@ -126,18 +126,19 @@ struct wt_status_state {\n size_t wt_status_locate_end(const char *s, size_t len);\n void wt_status_add_cut_line(FILE *fp);\n void wt_status_prepare(struct wt_status *s);\n-void wt_status_print(struct wt_status *s);\n-void wt_status_collect(struct wt_status *s);\n-void wt_status_get_state(struct wt_status_state *state, int get_detached_from);\n+void wt_status_print(struct wt_status *s, const struct wt_status_state *state);\n+void wt_status_collect(struct wt_status *s, const struct wt_status_state *state);\n+void wt_status_get_state(const struct wt_status *s, struct wt_status_state *state);\n+void wt_status_clear_state(struct wt_status_state *state);\n int wt_status_check_rebase(const struct worktree *wt,\n \t\t\t struct wt_status_state *state);\n int wt_status_check_bisect(const struct worktree *wt,\n \t\t\t struct wt_status_state *state);\n \n __attribute__((format (printf, 3, 4)))\n-void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...);\n+void status_printf_ln(const struct wt_status *s, const char *color, const char *fmt, ...);\n __attribute__((format (printf, 3, 4)))\n-void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);\n+void status_printf(const struct wt_status *s, const char *color, const char *fmt, ...);\n \n /* The following functions expect that the caller took care of reading the index. */\n int has_unstaged_changes(int ignore_submodules);\n-- \n2.18.0\n\n"},{"id":"352798","messageId":"xmqq1sc1rdvz.fsf@gitster-ct.c.googlers.com","threadId":"48321","inReplyTo":"20180715110807.25544-2-sxlijin@gmail.com","subject":"Re: [PATCH v3 1/3] t7501: add merge conflict tests for dry run","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-17T17:05:52Z","receivedAt":"2018-07-17T17:05:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> The behavior of git commit when doing a dry run changes if there are\n> unfixed/fixed merge conflits, but the test suite currently only asserts\n> that `git commit --dry-run` succeeds when all merge conflicts are fixed.\n>\n> Add tests to document the behavior of all flags which imply a dry run\n> when (1) there is at least one unfixed merge conflict and (2) when all\n> merge conflicts are all fixed.\n\ns/conflits/conflicts/\ns/fixed/resolved/g\t(both above and in the patch text)\ns/unfixed/unresolved/g (both above and in the patch text)\n\n> Signed-off-by: Samuel Lijin \n> ---\n> t/t7501-commit.sh | 45 ++++++++++++++++++++++++++++++++++++++++-----\n> 1 file changed, 40 insertions(+), 5 deletions(-)\n>\n> diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\n> index fa61b1a4e..be087e73f 100755\n> --- a/t/t7501-commit.sh\n> +++ b/t/t7501-commit.sh\n> @@ -652,7 +652,8 @@ test_expect_success '--only works on to-be-born branch' '\n> \ttest_cmp expected actual\n> '\n> \n> -test_expect_success '--dry-run with conflicts fixed from a merge' '\n> +# set up env for tests of --dry-run given fixed/unfixed merge conflicts\n> +test_expect_success 'setup env with unfixed merge conflicts' '\n> \t# setup two branches with conflicting information\n> \t# in the same file, resolve the conflict,\n> \t# call commit with --dry-run\n> @@ -665,11 +666,45 @@ test_expect_success '--dry-run with conflicts fixed from a merge' '\n> \tgit checkout -b branch-2 HEAD^1 &&\n> \techo \"commit-2-state\" >test-file &&\n> \tgit commit -m \"commit 2\" -i test-file &&\n> -\t! $(git merge --no-commit commit-1) &&\n> -\techo \"commit-2-state\" >test-file &&\n> +\ttest_expect_code 1 git merge --no-commit commit-1\n\nThe original is bad and also embarrassing. Whatever comes out of\nthe standard output of \"git merge\" is $IFS split and executed as a\nshell command (which likely results in \"no such command\" failure)\nand it tries to make sure that a failure happens.\n\nThe right way to write that line (without your enhancement in this\npatch) would have been:\n\n\ttest_must_fail git merge --no-commit commit-1 &&\n\nI doubt it is a good idea to hardcode exit status of 1 by using\ntest_expect_code, though. \"git merge --help\" does not say anything\nabout \"1 means this failure, 2 means that failure, 3 means that\nother failure\". And my quick forward scan of this series does not\ntell me that you are trying to declare that from here on we _will_\nmake that promise to the end users by carving the exit status(es) in\nstone. The same about \"git commit\"'s exit code in the following\nfour tests.\n\n> +'\n> +\n> +test_expect_success '--dry-run with unfixed merge conflicts' '\n> +\ttest_expect_code 1 git commit --dry-run\n> +'\n> +\n> +test_expect_success '--short with unfixed merge conflicts' '\n> +\ttest_expect_code 1 git commit --short\n> +'\n> +\n> +test_expect_success '--porcelain with unfixed merge conflicts' '\n> +\ttest_expect_code 1 git commit --porcelain\n> +'\n> +\n> +test_expect_success '--long with unfixed merge conflicts' '\n> +\ttest_expect_code 1 git commit --long\n> +'\n> +\n> +test_expect_success '--dry-run with conflicts fixed from a merge' '\n> +\techo \"merge-conflicts-fixed\" >test-file &&\n\nThe original test pretended that we resolved favouring the current\nstate with \"commit-2-state\" in the file, as if we ran \"-s ours\".\nIs there a reason why we now use a different contents, or is this\njust a change based on subjective preference? \n\n Not saying that the latter is necessrily bad; just trying to\n understand why we are making this change.\n\n> \tgit add test-file &&\n> -\tgit commit --dry-run &&\n> -\tgit commit -m \"conflicts fixed from merge.\"\n> +\tgit commit --dry-run\n\nOK, the original tried --dry-run to ensure it exited with 0 status\n(i.e. have something to commit) and then did a commit to record the\nupdated state with a message. You are checking only the dry-run\npart, leaving the check of the final commit's status to another\ntest.\n\n> +'\n> +\n> +test_expect_failure '--short with conflicts fixed from a merge' '\n> +\tgit commit --short\n> +'\n\nWith \"test_expect_failure\", you are saying that \"--short\" _should_\nexit with 0 but currently it does not. An untold expectation is\nthat even with the breakage with the exit code, the command still\nhonors the (implicit) --dry-run correctly and does not create a\nnew commit.\n\nThat was actually tested in the original. By &&-chaining like this\n\n\tgit commit --dry-run &&\n\tgit commit -m \"conflicts fixed from merge.\"\n\nwe would have noticed if a newly introduced bug caused the first\nstep \"commit --dry-run\" to return non-zero status (because then the\nstep would fail), or if it stopped being dry-run and made a commit\n(because then the next step would fail with \"nothing to commit\").\n\nBut by splitting these into separate tests, the patch makes such a\npotential failure with \"git commit --short\" break the later steps.\n\nNot very nice.\n\nIt may be a better change to just do in the original one\n\n\tgit add test-file &&\n\tgit commit --dry-run &&\n+\tgit commit --short &&\n+\tgit commit --long &&\n+\tgit commit --porcelain &&\n\tgit commit -m \"conflicts fixed from merge.\"\n\nwithout adding these new and separate tests, and then mark that one\nto expect a failure (because it would pass up to the --dry-run\ncommit, but the --short commit would fail) at this step, perhaps?\n\n> +test_expect_failure '--porcelain with conflicts fixed from a merge' '\n> +\tgit commit --porcelain\n> +'\n> +\n> +test_expect_success '--long with conflicts fixed from a merge' '\n> +\tgit commit --long\n> +'\n> +\n> +test_expect_success '--message with conflicts fixed from a merge' '\n> +\tgit commit --message \"conflicts fixed from merge.\"\n> '\n> \n> test_done\n"},{"id":"352800","messageId":"xmqqpnzlpyux.fsf@gitster-ct.c.googlers.com","threadId":"48321","inReplyTo":"20180715110807.25544-3-sxlijin@gmail.com","subject":"Re: [PATCH v3 2/3] wt-status: teach wt_status_collect about merges in progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-17T17:15:50Z","receivedAt":"2018-07-17T17:15:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> To fix the breakages documented by t7501, the next patch in this series\n> will teach wt_status_collect() to set the committable bit, instead of\n> having wt_longstatus_print_updated() and show_merge_in_progress() set it\n> (which is what currently happens). Unfortunately, wt_status_collect()\n> needs to know whether or not there is a merge in progress to set the bit\n> correctly,\n\ns/correctly,/correctly (a brief desription of why),/\n\nwould be nicer. The description might be\n\n\t(after a merge, it is OK for the result to be identical to HEAD,\n\twhich usually causes a \"nothing to commit\" error)\n\nor something like that.\n\n> so teach its (two) callers to create, initialize, and pass\n> in instances of wt_status_state, which records this metadata.\n>\n> Since wt_longstatus_print() and show_merge_in_progress() are in the same\n> callpaths and currently create and init copies of wt_status_state,\n> remove that logic and instead pass wt_status_state through.\n\nOK. Sounds like a good clean-up.\n\n> Make wt_status_get_state easier to use, add a helper method to clean up\n\nYour description so far marked function names with trailing ();\nlet's do so consistently for wt_status_get_state(), too.\n\n> wt_status_state, const-ify as many struct pointers in method signatures\n> as possible, and add a FIXME for a struct pointer which should be const\n> but isn't (that this patch series will not address).\n\n\"should be but isn't\" because...? I am wondering if it is better to\nleave _all_ constifying to a later effort, if we are leaving some of\nthem behind anyway. It would be better only if it will make this\npatch easier to read if we did so.\n\nAlso you did s/commitable/committable/ everywhere, which was\nsomewhat distracting. It would have been nicer to follow if that\nwere a separate preparatory clean-up patch.\n\n>\n> Signed-off-by: Samuel Lijin \n> ---\n> builtin/commit.c | 32 ++++----\n> ref-filter.c | 3 +-\n> wt-status.c | 188 +++++++++++++++++++++++------------------------\n> wt-status.h | 13 ++--\n> 4 files changed, 120 insertions(+), 116 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 37fcb55ab..79ef4f11a 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -463,6 +463,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n> static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn,\n> \t\t struct wt_status *s)\n> {\n> +\tstruct wt_status_state state;\n> \tstruct object_id oid;\n> \n> \tif (s->relative_paths)\n> @@ -482,10 +483,12 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n> \ts->status_format = status_format;\n> \ts->ignore_submodule_arg = ignore_submodule_arg;\n> \n> -\twt_status_collect(s);\n> -\twt_status_print(s);\n> +\twt_status_get_state(s, &state);\n> +\twt_status_collect(s, &state);\n> +\twt_status_print(s, &state);\n> +\twt_status_clear_state(&state);\n> \n> -\treturn s->commitable;\n> +\treturn s->committable;\n> }\n> \n> static int is_a_merge(const struct commit *current_head)\n> @@ -631,7 +634,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> {\n> \tstruct stat statbuf;\n> \tstruct strbuf committer_ident = STRBUF_INIT;\n> -\tint commitable;\n> +\tint committable;\n> \tstruct strbuf sb = STRBUF_INIT;\n> \tconst char *hook_arg1 = NULL;\n> \tconst char *hook_arg2 = NULL;\n> @@ -848,7 +851,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \n> \t\tsaved_color_setting = s->use_color;\n> \t\ts->use_color = 0;\n> -\t\tcommitable = run_status(s->fp, index_file, prefix, 1, s);\n> +\t\tcommittable = run_status(s->fp, index_file, prefix, 1, s);\n> \t\ts->use_color = saved_color_setting;\n> \t} else {\n> \t\tstruct object_id oid;\n> @@ -866,7 +869,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \t\t\tfor (i = 0; i < active_nr; i++)\n> \t\t\t\tif (ce_intent_to_add(active_cache[i]))\n> \t\t\t\t\tita_nr++;\n> -\t\t\tcommitable = active_nr - ita_nr > 0;\n> +\t\t\tcommittable = active_nr - ita_nr > 0;\n> \t\t} else {\n> \t\t\t/*\n> \t\t\t * Unless the user did explicitly request a submodule\n> @@ -882,7 +885,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \t\t\tif (ignore_submodule_arg &&\n> \t\t\t !strcmp(ignore_submodule_arg, \"all\"))\n> \t\t\t\tflags.ignore_submodules = 1;\n> -\t\t\tcommitable = index_differs_from(parent, &flags, 1);\n> +\t\t\tcommittable = index_differs_from(parent, &flags, 1);\n> \t\t}\n> \t}\n> \tstrbuf_release(&committer_ident);\n> @@ -894,7 +897,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \t * explicit --allow-empty. In the cherry-pick case, it may be\n> \t * empty due to conflict resolution, which the user should okay.\n> \t */\n> -\tif (!commitable && whence != FROM_MERGE && !allow_empty &&\n> +\tif (!committable && whence != FROM_MERGE && !allow_empty &&\n> \t !(amend && is_a_merge(current_head))) {\n> \t\ts->display_comment_prefix = old_display_comment_prefix;\n> \t\trun_status(stdout, index_file, prefix, 0, s);\n> @@ -1164,14 +1167,14 @@ static int parse_and_validate_options(int argc, const char *argv[],\n> static int dry_run_commit(int argc, const char **argv, const char *prefix,\n> \t\t\t const struct commit *current_head, struct wt_status *s)\n> {\n> -\tint commitable;\n> +\tint committable;\n> \tconst char *index_file;\n> \n> \tindex_file = prepare_index(argc, argv, prefix, current_head, 1);\n> -\tcommitable = run_status(stdout, index_file, prefix, 0, s);\n> +\tcommittable = run_status(stdout, index_file, prefix, 0, s);\n> \trollback_index_files();\n> \n> -\treturn commitable ? 0 : 1;\n> +\treturn committable ? 0 : 1;\n> }\n> \n> static int parse_status_slot(const char *slot)\n> @@ -1266,6 +1269,7 @@ static int git_status_config(const char *k, const char *v, void *cb)\n> int cmd_status(int argc, const char **argv, const char *prefix)\n> {\n> \tstatic struct wt_status s;\n> +\tstruct wt_status_state state;\n> \tint fd;\n> \tstruct object_id oid;\n> \tstatic struct option builtin_status_options[] = {\n> @@ -1338,7 +1342,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n> \ts.status_format = status_format;\n> \ts.verbose = verbose;\n> \n> -\twt_status_collect(&s);\n> +\twt_status_get_state(&s, &state);\n> +\twt_status_collect(&s, &state);\n> \n> \tif (0 <= fd)\n> \t\tupdate_index_if_able(&the_index, &index_lock);\n> @@ -1346,7 +1351,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n> \tif (s.relative_paths)\n> \t\ts.prefix = prefix;\n> \n> -\twt_status_print(&s);\n> +\twt_status_print(&s, &state);\n> +\twt_status_clear_state(&state);\n> \treturn 0;\n> }\n> \n> diff --git a/ref-filter.c b/ref-filter.c\n> index 9a333e21b..280ef9713 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1306,8 +1306,7 @@ char *get_head_description(void)\n> {\n> \tstruct strbuf desc = STRBUF_INIT;\n> \tstruct wt_status_state state;\n> -\tmemset(&state, 0, sizeof(state));\n> -\twt_status_get_state(&state, 1);\n> +\twt_status_get_state(NULL, &state);\n> \tif (state.rebase_in_progress ||\n> \t state.rebase_interactive_in_progress)\n> \t\tstrbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n> diff --git a/wt-status.c b/wt-status.c\n> index 50815e5fa..75d389944 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -33,7 +33,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {\n> \tGIT_COLOR_NIL, /* WT_STATUS_ONBRANCH */\n> };\n> \n> -static const char *color(int slot, struct wt_status *s)\n> +static const char *color(int slot, const struct wt_status *s)\n> {\n> \tconst char *c = \"\";\n> \tif (want_color(s->use_color))\n> @@ -43,7 +43,7 @@ static const char *color(int slot, struct wt_status *s)\n> \treturn c;\n> }\n> \n> -static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n> +static void status_vprintf(const struct wt_status *s, int at_bol, const char *color,\n> \t\tconst char *fmt, va_list ap, const char *trail)\n> {\n> \tstruct strbuf sb = STRBUF_INIT;\n> @@ -89,7 +89,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n> \tstrbuf_release(&sb);\n> }\n> \n> -void status_printf_ln(struct wt_status *s, const char *color,\n> +void status_printf_ln(const struct wt_status *s, const char *color,\n> \t\t\tconst char *fmt, ...)\n> {\n> \tva_list ap;\n> @@ -99,7 +99,7 @@ void status_printf_ln(struct wt_status *s, const char *color,\n> \tva_end(ap);\n> }\n> \n> -void status_printf(struct wt_status *s, const char *color,\n> +void status_printf(const struct wt_status *s, const char *color,\n> \t\t\tconst char *fmt, ...)\n> {\n> \tva_list ap;\n> @@ -109,7 +109,7 @@ void status_printf(struct wt_status *s, const char *color,\n> \tva_end(ap);\n> }\n> \n> -static void status_printf_more(struct wt_status *s, const char *color,\n> +static void status_printf_more(const struct wt_status *s, const char *color,\n> \t\t\t const char *fmt, ...)\n> {\n> \tva_list ap;\n> @@ -140,7 +140,7 @@ void wt_status_prepare(struct wt_status *s)\n> \ts->display_comment_prefix = 0;\n> }\n> \n> -static void wt_longstatus_print_unmerged_header(struct wt_status *s)\n> +static void wt_longstatus_print_unmerged_header(const struct wt_status *s)\n> {\n> \tint i;\n> \tint del_mod_conflict = 0;\n> @@ -192,7 +192,7 @@ static void wt_longstatus_print_unmerged_header(struct wt_status *s)\n> \tstatus_printf_ln(s, c, \"%s\", \"\");\n> }\n> \n> -static void wt_longstatus_print_cached_header(struct wt_status *s)\n> +static void wt_longstatus_print_cached_header(const struct wt_status *s)\n> {\n> \tconst char *c = color(WT_STATUS_HEADER, s);\n> \n> @@ -208,7 +208,7 @@ static void wt_longstatus_print_cached_header(struct wt_status *s)\n> \tstatus_printf_ln(s, c, \"%s\", \"\");\n> }\n> \n> -static void wt_longstatus_print_dirty_header(struct wt_status *s,\n> +static void wt_longstatus_print_dirty_header(const struct wt_status *s,\n> \t\t\t\t\t int has_deleted,\n> \t\t\t\t\t int has_dirty_submodules)\n> {\n> @@ -227,7 +227,7 @@ static void wt_longstatus_print_dirty_header(struct wt_status *s,\n> \tstatus_printf_ln(s, c, \"%s\", \"\");\n> }\n> \n> -static void wt_longstatus_print_other_header(struct wt_status *s,\n> +static void wt_longstatus_print_other_header(const struct wt_status *s,\n> \t\t\t\t\t const char *what,\n> \t\t\t\t\t const char *how)\n> {\n> @@ -239,7 +239,7 @@ static void wt_longstatus_print_other_header(struct wt_status *s,\n> \tstatus_printf_ln(s, c, \"%s\", \"\");\n> }\n> \n> -static void wt_longstatus_print_trailer(struct wt_status *s)\n> +static void wt_longstatus_print_trailer(const struct wt_status *s)\n> {\n> \tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n> }\n> @@ -305,7 +305,7 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval)\n> \treturn result;\n> }\n> \n> -static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n> +static void wt_longstatus_print_unmerged_data(const struct wt_status *s,\n> \t\t\t\t\t struct string_list_item *it)\n> {\n> \tconst char *c = color(WT_STATUS_UNMERGED, s);\n> @@ -332,7 +332,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n> \tstrbuf_release(&onebuf);\n> }\n> \n> -static void wt_longstatus_print_change_data(struct wt_status *s,\n> +static void wt_longstatus_print_change_data(const struct wt_status *s,\n> \t\t\t\t\t int change_type,\n> \t\t\t\t\t struct string_list_item *it)\n> {\n> @@ -718,7 +718,7 @@ static void wt_status_collect_untracked(struct wt_status *s)\n> \t\ts->untracked_in_ms = (getnanotime() - t_begin) / 1000000;\n> }\n> \n> -void wt_status_collect(struct wt_status *s)\n> +void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)\n> {\n> \twt_status_collect_changes_worktree(s);\n> \n> @@ -726,10 +726,11 @@ void wt_status_collect(struct wt_status *s)\n> \t\twt_status_collect_changes_initial(s);\n> \telse\n> \t\twt_status_collect_changes_index(s);\n> +\n> \twt_status_collect_untracked(s);\n> }\n> \n> -static void wt_longstatus_print_unmerged(struct wt_status *s)\n> +static void wt_longstatus_print_unmerged(const struct wt_status *s)\n> {\n> \tint shown_header = 0;\n> \tint i;\n> @@ -767,7 +768,7 @@ static void wt_longstatus_print_updated(struct wt_status *s)\n> \t\t\tcontinue;\n> \t\tif (!shown_header) {\n> \t\t\twt_longstatus_print_cached_header(s);\n> -\t\t\ts->commitable = 1;\n> +\t\t\ts->committable = 1;\n> \t\t\tshown_header = 1;\n> \t\t}\n> \t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n> @@ -781,7 +782,7 @@ static void wt_longstatus_print_updated(struct wt_status *s)\n> * 0 : no change\n> * 1 : some change but no delete\n> */\n> -static int wt_status_check_worktree_changes(struct wt_status *s,\n> +static int wt_status_check_worktree_changes(const struct wt_status *s,\n> \t\t\t\t\t int *dirty_submodules)\n> {\n> \tint i;\n> @@ -805,7 +806,7 @@ static int wt_status_check_worktree_changes(struct wt_status *s,\n> \treturn changes;\n> }\n> \n> -static void wt_longstatus_print_changed(struct wt_status *s)\n> +static void wt_longstatus_print_changed(const struct wt_status *s)\n> {\n> \tint i, dirty_submodules;\n> \tint worktree_changes = wt_status_check_worktree_changes(s, &dirty_submodules);\n> @@ -837,7 +838,7 @@ static int stash_count_refs(struct object_id *ooid, struct object_id *noid,\n> \treturn 0;\n> }\n> \n> -static void wt_longstatus_print_stash_summary(struct wt_status *s)\n> +static void wt_longstatus_print_stash_summary(const struct wt_status *s)\n> {\n> \tint stash_count = 0;\n> \n> @@ -849,7 +850,7 @@ static void wt_longstatus_print_stash_summary(struct wt_status *s)\n> \t\t\t\t stash_count);\n> }\n> \n> -static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncommitted)\n> +static void wt_longstatus_print_submodule_summary(const struct wt_status *s, int uncommitted)\n> {\n> \tstruct child_process sm_summary = CHILD_PROCESS_INIT;\n> \tstruct strbuf cmd_stdout = STRBUF_INIT;\n> @@ -895,8 +896,8 @@ static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncom\n> \tstrbuf_release(&summary);\n> }\n> \n> -static void wt_longstatus_print_other(struct wt_status *s,\n> -\t\t\t\t struct string_list *l,\n> +static void wt_longstatus_print_other(const struct wt_status *s,\n> +\t\t\t\t const struct string_list *l,\n> \t\t\t\t const char *what,\n> \t\t\t\t const char *how)\n> {\n> @@ -969,7 +970,7 @@ void wt_status_add_cut_line(FILE *fp)\n> \tstrbuf_release(&buf);\n> }\n> \n> -static void wt_longstatus_print_verbose(struct wt_status *s)\n> +static void wt_longstatus_print_verbose(const struct wt_status *s)\n> {\n> \tstruct rev_info rev;\n> \tstruct setup_revision_opt opt;\n> @@ -1000,7 +1001,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n> \t\trev.diffopt.use_color = 0;\n> \t\twt_status_add_cut_line(s->fp);\n> \t}\n> -\tif (s->verbose > 1 && s->commitable) {\n> +\tif (s->verbose > 1 && s->committable) {\n> \t\t/* print_updated() printed a header, so do we */\n> \t\tif (s->fp != stdout)\n> \t\t\twt_longstatus_print_trailer(s);\n> @@ -1021,7 +1022,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n> \t}\n> }\n> \n> -static void wt_longstatus_print_tracking(struct wt_status *s)\n> +static void wt_longstatus_print_tracking(const struct wt_status *s)\n> {\n> \tstruct strbuf sb = STRBUF_INIT;\n> \tconst char *cp, *ep, *branch_name;\n> @@ -1055,7 +1056,7 @@ static void wt_longstatus_print_tracking(struct wt_status *s)\n> \tstrbuf_release(&sb);\n> }\n> \n> -static int has_unmerged(struct wt_status *s)\n> +static int has_unmerged(const struct wt_status *s)\n> {\n> \tint i;\n> \n> @@ -1069,7 +1070,7 @@ static int has_unmerged(struct wt_status *s)\n> }\n> \n> static void show_merge_in_progress(struct wt_status *s,\n> -\t\t\t\tstruct wt_status_state *state,\n> +\t\t\t\tconst struct wt_status_state *state,\n> \t\t\t\tconst char *color)\n> {\n> \tif (has_unmerged(s)) {\n> @@ -1081,7 +1082,7 @@ static void show_merge_in_progress(struct wt_status *s,\n> \t\t\t\t\t _(\" (use \\\"git merge --abort\\\" to abort the merge)\"));\n> \t\t}\n> \t} else {\n> -\t\ts-> commitable = 1;\n> +\t\ts-> committable = 1;\n> \t\tstatus_printf_ln(s, color,\n> \t\t\t_(\"All conflicts fixed but you are still merging.\"));\n> \t\tif (s->hints)\n> @@ -1091,8 +1092,8 @@ static void show_merge_in_progress(struct wt_status *s,\n> \twt_longstatus_print_trailer(s);\n> }\n> \n> -static void show_am_in_progress(struct wt_status *s,\n> -\t\t\t\tstruct wt_status_state *state,\n> +static void show_am_in_progress(const struct wt_status *s,\n> +\t\t\t\tconst struct wt_status_state *state,\n> \t\t\t\tconst char *color)\n> {\n> \tstatus_printf_ln(s, color,\n> @@ -1130,7 +1131,7 @@ static char *read_line_from_git_path(const char *filename)\n> \t}\n> }\n> \n> -static int split_commit_in_progress(struct wt_status *s)\n> +static int split_commit_in_progress(const struct wt_status *s)\n> {\n> \tint split_in_progress = 0;\n> \tchar *head, *orig_head, *rebase_amend, *rebase_orig_head;\n> @@ -1224,8 +1225,8 @@ static int read_rebase_todolist(const char *fname, struct string_list *lines)\n> \treturn 0;\n> }\n> \n> -static void show_rebase_information(struct wt_status *s,\n> -\t\t\t\t\tstruct wt_status_state *state,\n> +static void show_rebase_information(const struct wt_status *s,\n> +\t\t\t\t\tconst struct wt_status_state *state,\n> \t\t\t\t\tconst char *color)\n> {\n> \tif (state->rebase_interactive_in_progress) {\n> @@ -1278,8 +1279,8 @@ static void show_rebase_information(struct wt_status *s,\n> \t}\n> }\n> \n> -static void print_rebase_state(struct wt_status *s,\n> -\t\t\t\tstruct wt_status_state *state,\n> +static void print_rebase_state(const struct wt_status *s,\n> +\t\t\t\tconst struct wt_status_state *state,\n> \t\t\t\tconst char *color)\n> {\n> \tif (state->branch)\n> @@ -1292,8 +1293,8 @@ static void print_rebase_state(struct wt_status *s,\n> \t\t\t\t _(\"You are currently rebasing.\"));\n> }\n> \n> -static void show_rebase_in_progress(struct wt_status *s,\n> -\t\t\t\tstruct wt_status_state *state,\n> +static void show_rebase_in_progress(const struct wt_status *s,\n> +\t\t\t\tconst struct wt_status_state *state,\n> \t\t\t\tconst char *color)\n> {\n> \tstruct stat st;\n> @@ -1345,8 +1346,8 @@ static void show_rebase_in_progress(struct wt_status *s,\n> \twt_longstatus_print_trailer(s);\n> }\n> \n> -static void show_cherry_pick_in_progress(struct wt_status *s,\n> -\t\t\t\t\tstruct wt_status_state *state,\n> +static void show_cherry_pick_in_progress(const struct wt_status *s,\n> +\t\t\t\t\tconst struct wt_status_state *state,\n> \t\t\t\t\tconst char *color)\n> {\n> \tstatus_printf_ln(s, color, _(\"You are currently cherry-picking commit %s.\"),\n> @@ -1364,8 +1365,8 @@ static void show_cherry_pick_in_progress(struct wt_status *s,\n> \twt_longstatus_print_trailer(s);\n> }\n> \n> -static void show_revert_in_progress(struct wt_status *s,\n> -\t\t\t\t\tstruct wt_status_state *state,\n> +static void show_revert_in_progress(const struct wt_status *s,\n> +\t\t\t\t\tconst struct wt_status_state *state,\n> \t\t\t\t\tconst char *color)\n> {\n> \tstatus_printf_ln(s, color, _(\"You are currently reverting commit %s.\"),\n> @@ -1383,8 +1384,8 @@ static void show_revert_in_progress(struct wt_status *s,\n> \twt_longstatus_print_trailer(s);\n> }\n> \n> -static void show_bisect_in_progress(struct wt_status *s,\n> -\t\t\t\tstruct wt_status_state *state,\n> +static void show_bisect_in_progress(const struct wt_status *s,\n> +\t\t\t\tconst struct wt_status_state *state,\n> \t\t\t\tconst char *color)\n> {\n> \tif (state->branch)\n> @@ -1538,12 +1539,16 @@ int wt_status_check_bisect(const struct worktree *wt,\n> \treturn 0;\n> }\n> \n> -void wt_status_get_state(struct wt_status_state *state,\n> -\t\t\t int get_detached_from)\n> +void wt_status_get_state(\n> +\t\tconst struct wt_status *s, struct wt_status_state *state)\n> {\n> +\tint get_detached_from =\n> +\t\t(s == NULL) || (s->branch && !strcmp(s->branch, \"HEAD\"));\n> \tstruct stat st;\n> \tstruct object_id oid;\n> \n> +\tmemset(state, 0, sizeof(*state));\n> +\n> \tif (!stat(git_path_merge_head(), &st)) {\n> \t\tstate->merge_in_progress = 1;\n> \t} else if (wt_status_check_rebase(NULL, state)) {\n> @@ -1564,8 +1569,15 @@ void wt_status_get_state(struct wt_status_state *state,\n> \t\twt_status_get_detached_from(state);\n> }\n> \n> +void wt_status_clear_state(struct wt_status_state *state)\n> +{\n> +\tfree(state->branch);\n> +\tfree(state->onto);\n> +\tfree(state->detached_from);\n> +}\n> +\n> static void wt_longstatus_print_state(struct wt_status *s,\n> -\t\t\t\t struct wt_status_state *state)\n> +\t\t\t\t const struct wt_status_state *state)\n> {\n> \tconst char *state_color = color(WT_STATUS_HEADER, s);\n> \tif (state->merge_in_progress)\n> @@ -1582,30 +1594,25 @@ static void wt_longstatus_print_state(struct wt_status *s,\n> \t\tshow_bisect_in_progress(s, state, state_color);\n> }\n> \n> -static void wt_longstatus_print(struct wt_status *s)\n> +static void wt_longstatus_print(struct wt_status *s, const struct wt_status_state *state)\n> {\n> \tconst char *branch_color = color(WT_STATUS_ONBRANCH, s);\n> \tconst char *branch_status_color = color(WT_STATUS_HEADER, s);\n> -\tstruct wt_status_state state;\n> -\n> -\tmemset(&state, 0, sizeof(state));\n> -\twt_status_get_state(&state,\n> -\t\t\t s->branch && !strcmp(s->branch, \"HEAD\"));\n> \n> \tif (s->branch) {\n> \t\tconst char *on_what = _(\"On branch \");\n> \t\tconst char *branch_name = s->branch;\n> \t\tif (!strcmp(branch_name, \"HEAD\")) {\n> \t\t\tbranch_status_color = color(WT_STATUS_NOBRANCH, s);\n> -\t\t\tif (state.rebase_in_progress || state.rebase_interactive_in_progress) {\n> -\t\t\t\tif (state.rebase_interactive_in_progress)\n> +\t\t\tif (state->rebase_in_progress || state->rebase_interactive_in_progress) {\n> +\t\t\t\tif (state->rebase_interactive_in_progress)\n> \t\t\t\t\ton_what = _(\"interactive rebase in progress; onto \");\n> \t\t\t\telse\n> \t\t\t\t\ton_what = _(\"rebase in progress; onto \");\n> -\t\t\t\tbranch_name = state.onto;\n> -\t\t\t} else if (state.detached_from) {\n> -\t\t\t\tbranch_name = state.detached_from;\n> -\t\t\t\tif (state.detached_at)\n> +\t\t\t\tbranch_name = state->onto;\n> +\t\t\t} else if (state->detached_from) {\n> +\t\t\t\tbranch_name = state->detached_from;\n> +\t\t\t\tif (state->detached_at)\n> \t\t\t\t\ton_what = _(\"HEAD detached at \");\n> \t\t\t\telse\n> \t\t\t\t\ton_what = _(\"HEAD detached from \");\n> @@ -1622,10 +1629,7 @@ static void wt_longstatus_print(struct wt_status *s)\n> \t\t\twt_longstatus_print_tracking(s);\n> \t}\n> \n> -\twt_longstatus_print_state(s, &state);\n> -\tfree(state.branch);\n> -\tfree(state.onto);\n> -\tfree(state.detached_from);\n> +\twt_longstatus_print_state(s, state);\n> \n> \tif (s->is_initial) {\n> \t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n> @@ -1657,14 +1661,14 @@ static void wt_longstatus_print(struct wt_status *s)\n> \t\t\t\t\t \"new files yourself (see 'git help status').\"),\n> \t\t\t\t\t s->untracked_in_ms / 1000.0);\n> \t\t}\n> -\t} else if (s->commitable)\n> +\t} else if (s->committable)\n> \t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, _(\"Untracked files not listed%s\"),\n> \t\t\ts->hints\n> \t\t\t? _(\" (use -u option to show untracked files)\") : \"\");\n> \n> \tif (s->verbose)\n> \t\twt_longstatus_print_verbose(s);\n> -\tif (!s->commitable) {\n> +\tif (!s->committable) {\n> \t\tif (s->amend)\n> \t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, _(\"No changes\"));\n> \t\telse if (s->nowarn)\n> @@ -1700,7 +1704,7 @@ static void wt_longstatus_print(struct wt_status *s)\n> }\n> \n> static void wt_shortstatus_unmerged(struct string_list_item *it,\n> -\t\t\t struct wt_status *s)\n> +\t\t\t const struct wt_status *s)\n> {\n> \tstruct wt_status_change_data *d = it->util;\n> \tconst char *how = \"??\";\n> @@ -1727,7 +1731,7 @@ static void wt_shortstatus_unmerged(struct string_list_item *it,\n> }\n> \n> static void wt_shortstatus_status(struct string_list_item *it,\n> -\t\t\t struct wt_status *s)\n> +\t\t\t const struct wt_status *s)\n> {\n> \tstruct wt_status_change_data *d = it->util;\n> \n> @@ -1770,7 +1774,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n> }\n> \n> static void wt_shortstatus_other(struct string_list_item *it,\n> -\t\t\t\t struct wt_status *s, const char *sign)\n> +\t\t\t\t const struct wt_status *s, const char *sign)\n> {\n> \tif (s->null_termination) {\n> \t\tfprintf(stdout, \"%s %s%c\", sign, it->string, 0);\n> @@ -1784,7 +1788,7 @@ static void wt_shortstatus_other(struct string_list_item *it,\n> \t}\n> }\n> \n> -static void wt_shortstatus_print_tracking(struct wt_status *s)\n> +static void wt_shortstatus_print_tracking(const struct wt_status *s)\n> {\n> \tstruct branch *branch;\n> \tconst char *header_color = color(WT_STATUS_HEADER, s);\n> @@ -1860,7 +1864,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n> \tfputc(s->null_termination ? '\\0' : '\\n', s->fp);\n> }\n> \n> -static void wt_shortstatus_print(struct wt_status *s)\n> +static void wt_shortstatus_print(const struct wt_status *s)\n> {\n> \tstruct string_list_item *it;\n> \n> @@ -1924,18 +1928,14 @@ static void wt_porcelain_print(struct wt_status *s)\n> * upstream. When AHEAD_BEHIND_QUICK is requested and the branches\n> * are different, '?' will be substituted for the actual count.\n> */\n> -static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n> +static void wt_porcelain_v2_print_tracking(const struct wt_status *s, const struct wt_status_state *state)\n> {\n> \tstruct branch *branch;\n> \tconst char *base;\n> \tconst char *branch_name;\n> -\tstruct wt_status_state state;\n> \tint ab_info, nr_ahead, nr_behind;\n> \tchar eol = s->null_termination ? '\\0' : '\\n';\n> \n> -\tmemset(&state, 0, sizeof(state));\n> -\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n> -\n> \tfprintf(s->fp, \"# branch.oid %s%c\",\n> \t\t\t(s->is_initial ? \"(initial)\" : sha1_to_hex(s->sha1_commit)),\n> \t\t\teol);\n> @@ -1946,10 +1946,10 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n> \t\tif (!strcmp(s->branch, \"HEAD\")) {\n> \t\t\tfprintf(s->fp, \"# branch.head %s%c\", \"(detached)\", eol);\n> \n> -\t\t\tif (state.rebase_in_progress || state.rebase_interactive_in_progress)\n> -\t\t\t\tbranch_name = state.onto;\n> -\t\t\telse if (state.detached_from)\n> -\t\t\t\tbranch_name = state.detached_from;\n> +\t\t\tif (state->rebase_in_progress || state->rebase_interactive_in_progress)\n> +\t\t\t\tbranch_name = state->onto;\n> +\t\t\telse if (state->detached_from)\n> +\t\t\t\tbranch_name = state->detached_from;\n> \t\t\telse\n> \t\t\t\tbranch_name = \"\";\n> \t\t} else {\n> @@ -1983,10 +1983,6 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n> \t\t\t}\n> \t\t}\n> \t}\n> -\n> -\tfree(state.branch);\n> -\tfree(state.onto);\n> -\tfree(state.detached_from);\n> }\n> \n> /*\n> @@ -1994,7 +1990,7 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n> * fixed-length string of characters in the buffer provided.\n> */\n> static void wt_porcelain_v2_submodule_state(\n> -\tstruct wt_status_change_data *d,\n> +\tconst struct wt_status_change_data *d,\n> \tchar sub[5])\n> {\n> \tif (S_ISGITLINK(d->mode_head) ||\n> @@ -2017,8 +2013,8 @@ static void wt_porcelain_v2_submodule_state(\n> * Fix-up changed entries before we print them.\n> */\n> static void wt_porcelain_v2_fix_up_changed(\n> -\tstruct string_list_item *it,\n> -\tstruct wt_status *s)\n> +\tconst struct string_list_item *it,\n> +\tconst struct wt_status *s)\n> {\n> \tstruct wt_status_change_data *d = it->util;\n> \n> @@ -2066,8 +2062,8 @@ static void wt_porcelain_v2_fix_up_changed(\n> * Print porcelain v2 info for tracked entries with changes.\n> */\n> static void wt_porcelain_v2_print_changed_entry(\n> -\tstruct string_list_item *it,\n> -\tstruct wt_status *s)\n> +\tconst struct string_list_item *it,\n> +\tconst struct wt_status *s)\n> {\n> \tstruct wt_status_change_data *d = it->util;\n> \tstruct strbuf buf = STRBUF_INIT;\n> @@ -2130,8 +2126,8 @@ static void wt_porcelain_v2_print_changed_entry(\n> * Print porcelain v2 status info for unmerged entries.\n> */\n> static void wt_porcelain_v2_print_unmerged_entry(\n> -\tstruct string_list_item *it,\n> -\tstruct wt_status *s)\n> +\tconst struct string_list_item *it,\n> +\tconst struct wt_status *s)\n> {\n> \tstruct wt_status_change_data *d = it->util;\n> \tconst struct cache_entry *ce;\n> @@ -2211,8 +2207,8 @@ static void wt_porcelain_v2_print_unmerged_entry(\n> * Print porcelain V2 status info for untracked and ignored entries.\n> */\n> static void wt_porcelain_v2_print_other(\n> -\tstruct string_list_item *it,\n> -\tstruct wt_status *s,\n> +\tconst struct string_list_item *it,\n> +\tconst struct wt_status *s,\n> \tchar prefix)\n> {\n> \tstruct strbuf buf = STRBUF_INIT;\n> @@ -2242,14 +2238,14 @@ static void wt_porcelain_v2_print_other(\n> * []*\n> *\n> */\n> -static void wt_porcelain_v2_print(struct wt_status *s)\n> +static void wt_porcelain_v2_print(const struct wt_status *s, const struct wt_status_state *state)\n> {\n> \tstruct wt_status_change_data *d;\n> \tstruct string_list_item *it;\n> \tint i;\n> \n> \tif (s->show_branch)\n> -\t\twt_porcelain_v2_print_tracking(s);\n> +\t\twt_porcelain_v2_print_tracking(s, state);\n> \n> \tfor (i = 0; i < s->change.nr; i++) {\n> \t\tit = &(s->change.items[i]);\n> @@ -2276,7 +2272,9 @@ static void wt_porcelain_v2_print(struct wt_status *s)\n> \t}\n> }\n> \n> -void wt_status_print(struct wt_status *s)\n> +// FIXME: `struct wt_status *` should be `const struct wt_status` but because\n> +// `wt_porcelain_print()` modifies it, that has to first be fixed\n> +void wt_status_print(struct wt_status *s, const struct wt_status_state *state)\n> {\n> \tswitch (s->status_format) {\n> \tcase STATUS_FORMAT_SHORT:\n> @@ -2286,14 +2284,14 @@ void wt_status_print(struct wt_status *s)\n> \t\twt_porcelain_print(s);\n> \t\tbreak;\n> \tcase STATUS_FORMAT_PORCELAIN_V2:\n> -\t\twt_porcelain_v2_print(s);\n> +\t\twt_porcelain_v2_print(s, state);\n> \t\tbreak;\n> \tcase STATUS_FORMAT_UNSPECIFIED:\n> \t\tdie(\"BUG: finalize_deferred_config() should have been called\");\n> \t\tbreak;\n> \tcase STATUS_FORMAT_NONE:\n> \tcase STATUS_FORMAT_LONG:\n> -\t\twt_longstatus_print(s);\n> +\t\twt_longstatus_print(s, state);\n> \t\tbreak;\n> \t}\n> }\n> diff --git a/wt-status.h b/wt-status.h\n> index 430770b85..6cccfd7a0 100644\n> --- a/wt-status.h\n> +++ b/wt-status.h\n> @@ -94,7 +94,7 @@ struct wt_status {\n> \tunsigned char sha1_commit[GIT_MAX_RAWSZ]; /* when not Initial */\n> \n> \t/* These are computed during processing of the individual sections */\n> -\tint commitable;\n> +\tint committable;\n> \tint workdir_dirty;\n> \tconst char *index_file;\n> \tFILE *fp;\n> @@ -126,18 +126,19 @@ struct wt_status_state {\n> size_t wt_status_locate_end(const char *s, size_t len);\n> void wt_status_add_cut_line(FILE *fp);\n> void wt_status_prepare(struct wt_status *s);\n> -void wt_status_print(struct wt_status *s);\n> -void wt_status_collect(struct wt_status *s);\n> -void wt_status_get_state(struct wt_status_state *state, int get_detached_from);\n> +void wt_status_print(struct wt_status *s, const struct wt_status_state *state);\n> +void wt_status_collect(struct wt_status *s, const struct wt_status_state *state);\n> +void wt_status_get_state(const struct wt_status *s, struct wt_status_state *state);\n> +void wt_status_clear_state(struct wt_status_state *state);\n> int wt_status_check_rebase(const struct worktree *wt,\n> \t\t\t struct wt_status_state *state);\n> int wt_status_check_bisect(const struct worktree *wt,\n> \t\t\t struct wt_status_state *state);\n> \n> __attribute__((format (printf, 3, 4)))\n> -void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...);\n> +void status_printf_ln(const struct wt_status *s, const char *color, const char *fmt, ...);\n> __attribute__((format (printf, 3, 4)))\n> -void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);\n> +void status_printf(const struct wt_status *s, const char *color, const char *fmt, ...);\n> \n> /* The following functions expect that the caller took care of reading the index. */\n> int has_unstaged_changes(int ignore_submodules);\n"},{"id":"352803","messageId":"xmqqh8kxpy21.fsf@gitster-ct.c.googlers.com","threadId":"48321","inReplyTo":"20180715110807.25544-4-sxlijin@gmail.com","subject":"Re: [PATCH v3 3/3] commit: fix exit code for --short/--porcelain","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-17T17:33:10Z","receivedAt":"2018-07-17T17:33:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> diff --git a/wt-status.c b/wt-status.c\n> index 75d389944..4ba657978 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -718,6 +718,39 @@ static void wt_status_collect_untracked(struct wt_status *s)\n> \t\ts->untracked_in_ms = (getnanotime() - t_begin) / 1000000;\n> }\n> \n> +static int has_unmerged(const struct wt_status *s)\n> +{\n> +\tint i;\n> +\n> +\tfor (i = 0; i < s->change.nr; i++) {\n> +\t\tstruct wt_status_change_data *d;\n> +\t\td = s->change.items[i].util;\n> +\t\tif (d->stagemask)\n> +\t\t\treturn 1;\n> +\t}\n> +\treturn 0;\n> +}\n> +\n> +static void wt_status_mark_committable(\n> +\t\tstruct wt_status *s, const struct wt_status_state *state)\n> +{\n> +\tint i;\n> +\n> +\tif (state->merge_in_progress && !has_unmerged(s)) {\n> +\t\ts->committable = 1;\n> +\t\treturn;\n> +\t}\n\nIs this trying to say:\n\n\tDuring/after a merge, if there is no higher stage entry in\n\tthe index, we can commit.\n\nI am wondering if we also should say:\n\n\tDuring/after a merge, if there is any unresolved conflict in\n\tthe index, we cannot commit.\n\t\nin which case the above becomes more like this:\n\n\tif (state->merge_in_progress) {\n\t\ts->committable = !has_unmerged(s);\n\t\treturn;\n\t}\n\nBut with your patch, with no remaining conflict in the index during\na merge, the control comes here and goes into the next loop.\n\n> +\tfor (i = 0; i < s->change.nr; i++) {\n> +\t\tstruct wt_status_change_data *d = (s->change.items[i]).util;\n> +\n> +\t\tif (d->index_status && d->index_status != DIFF_STATUS_UNMERGED) {\n> +\t\t\ts->committable = 1;\n> +\t\t\treturn;\n> +\t\t}\n> +\t}\n\nThe loop seems to say \"As long as there is one entry in the index\nthat is not in conflict and is different from the HEAD, then we can\ncommit\". Is that correct? \n\nImagine there are two paths A and B in the branches involved in a\nmerge, and A cleanly resolves (say, we take their version because\nour history did not touch it since we diverged) while B has\nconflict. We'll come to this loop (because we are in a merge but\nhave some unmerged paths) and we find that A is different from HEAD,\nhappily set committable bit and return.\n\nI _think_ with the change to \"what happens during merge\" above that\nI suggested, this loop automatically becomes correct, but I didn't\nthink it through. If there are ways other than .merge_in_progress\nthat place conflicted entries in the index, then this loop is still\nincorrect and would want to be more like:\n\n\tfor (i = 0; i < s->change.nr; i++) {\n\t\tstruct wt_status_change_data *d = (s->change.items[i]).util;\n\n\t\tif (d->index_status == DIFF_STATUS_UNMERGED) {\n\t\t\ts->committable = 0;\n\t\t\treturn;\n\t\t}\n\t\tif (d->index_status)\n\t\t\ts->committable = 1;\n\t}\n\ni.e. we declare \"not ready to commit\" if there is *any* conflicted\nentry, but otherwise set committable to 1 if we see any entry that\nis different from HEAD (to declare succcess once we successfully\nloop through to the last entry without seeing any conflict).\n\n> void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)\n> {\n> \twt_status_collect_changes_worktree(s);\n> @@ -728,6 +761,8 @@ void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)\n> \t\twt_status_collect_changes_index(s);\n> \n> \twt_status_collect_untracked(s);\n> +\n> +\twt_status_mark_committable(s, state);\n> }\n> \n> static void wt_longstatus_print_unmerged(const struct wt_status *s)\n> @@ -753,28 +788,28 @@ static void wt_longstatus_print_unmerged(const struct wt_status *s)\n> \n> }\n> \n> -static void wt_longstatus_print_updated(struct wt_status *s)\n> +static void wt_longstatus_print_updated(const struct wt_status *s)\n> {\n> -\tint shown_header = 0;\n> \tint i;\n> \n> +\tif (!s->committable) {\n> +\t\treturn;\n> +\t}\n\nNo need to have {} around a single statement. Especially when you\nknow you won't be touching the line (e.g. to later add more\nstatements in the block) in this last patch in a series.\n\n> +\twt_longstatus_print_cached_header(s);\n> +\n"},{"id":"352805","messageId":"xmqqd0vlpxhy.fsf@gitster-ct.c.googlers.com","threadId":"48321","inReplyTo":"xmqq1sc1rdvz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 1/3] t7501: add merge conflict tests for dry run","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-17T17:45:13Z","receivedAt":"2018-07-17T17:45:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> But by splitting these into separate tests, the patch makes such a\n> potential failure with \"git commit --short\" break the later steps.\n>\n> Not very nice.\n>\n> It may be a better change to just do in the original one\n>\n> \tgit add test-file &&\n> \tgit commit --dry-run &&\n> +\tgit commit --short &&\n> +\tgit commit --long &&\n> +\tgit commit --porcelain &&\n> \tgit commit -m \"conflicts fixed from merge.\"\n>\n> without adding these new and separate tests, and then mark that one\n> to expect a failure (because it would pass up to the --dry-run\n> commit, but the --short commit would fail) at this step, perhaps?\n\nOf course, if you want to be more thorough, anticipating that other\npeople in their future updates may break --short but not --long or\n--porcelain, testing each option in separate test_expect_success is\na necessary way to do so, but then you'd need to actually be more\nthorough, by not merely running each of them in separate\ntest_expect_success block but also arranging that each of them start\nin an expected state to try the thing we want it to try. That is\n\n\tfor opt in --dry-run --short --long --porcelain\n\tdo\n\t\ttest_expect_success \"commit $opt\" '\n\t\t\tset up the conflicted state after merge &&\n\t\t\tgit commit $opt\n\t\t'\n\tdone\n\nwhere the \"set up the state\" part makes sure it can tolerate\npotential mistakes of previous run of \"git commit $opt\" (e.g. it\nby mistake made a commit, making the index identical to HEAD and\ntaking us out of \"merge in progress\" state).\n\nBut from your 1/3 I did not get the impression that you particularly\nwant to be more thorough, and from your 3/3 I did not get the\nimpression that you anticipate --short/--long/--porcelain may get\nbroken independently. And if that is the case, then chaining all of\nthem together like the above is a more honest way to express that we\nare only doing a minimum set of testing.\n\nThanks.\n"},{"id":"353020","messageId":"CAJZjrdVj55W39fxK2ebYTgwO-N-ez=WcEvyepK4wqddiiQct3A@mail.gmail.com","threadId":"48321","inReplyTo":"xmqqh8kxpy21.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 3/3] commit: fix exit code for --short/--porcelain","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-07-19T09:31:33Z","receivedAt":"2018-07-19T09:32:22Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Thanks for the review.\n\nOn Tue, Jul 17, 2018 at 10:33 AM Junio C Hamano wrote:\n>\n> Samuel Lijin writes:\n>\n> > diff --git a/wt-status.c b/wt-status.c\n> > index 75d389944..4ba657978 100644\n> > --- a/wt-status.c\n> > +++ b/wt-status.c\n> > @@ -718,6 +718,39 @@ static void wt_status_collect_untracked(struct wt_status *s)\n> > s->untracked_in_ms = (getnanotime() - t_begin) / 1000000;\n> > }\n> >\n> > +static int has_unmerged(const struct wt_status *s)\n> > +{\n> > + int i;\n> > +\n> > + for (i = 0; i < s->change.nr; i++) {\n> > + struct wt_status_change_data *d;\n> > + d = s->change.items[i].util;\n> > + if (d->stagemask)\n> > + return 1;\n> > + }\n> > + return 0;\n> > +}\n> > +\n> > +static void wt_status_mark_committable(\n> > + struct wt_status *s, const struct wt_status_state *state)\n> > +{\n> > + int i;\n> > +\n> > + if (state->merge_in_progress && !has_unmerged(s)) {\n> > + s->committable = 1;\n> > + return;\n> > + }\n>\n> Is this trying to say:\n>\n> During/after a merge, if there is no higher stage entry in\n> the index, we can commit.\n>\n> I am wondering if we also should say:\n>\n> During/after a merge, if there is any unresolved conflict in\n> the index, we cannot commit.\n>\n> in which case the above becomes more like this:\n>\n> if (state->merge_in_progress) {\n> s->committable = !has_unmerged(s);\n> return;\n> }\n>\n> But with your patch, with no remaining conflict in the index during\n> a merge, the control comes here and goes into the next loop.\n>\n> > + for (i = 0; i < s->change.nr; i++) {\n> > + struct wt_status_change_data *d = (s->change.items[i]).util;\n> > +\n> > + if (d->index_status && d->index_status != DIFF_STATUS_UNMERGED) {\n> > + s->committable = 1;\n> > + return;\n> > + }\n> > + }\n>\n> The loop seems to say \"As long as there is one entry in the index\n> that is not in conflict and is different from the HEAD, then we can\n> commit\". Is that correct?\n>\n> Imagine there are two paths A and B in the branches involved in a\n> merge, and A cleanly resolves (say, we take their version because\n> our history did not touch it since we diverged) while B has\n> conflict. We'll come to this loop (because we are in a merge but\n> have some unmerged paths) and we find that A is different from HEAD,\n> happily set committable bit and return.\n\nI'll be honest: when I wrote this, I didn't think too much about what\nthe code was actually doing, semantically speaking: I was assuming\nthat the behavior that set the commitable bit in the call tree of\nwt_longstatus_print() was correct, and that it was just a matter of\nmechanically copying that logic over to the --short/--porcelain call\npaths.\n\nLooking into this more deeply, I think you're right, but more\nproblematically, this is technically a bug with the current Git code\nthat seems to be cancelled out by another bug: wt_status_state\napparently does not correctly reflect the state of the index when it\nreaches wt_longstatus_print_updated(). Working from master\n(f55ff46c9), I modified the last test in t7501 to look like this:\n\n→.echo \"Initial contents, unimportant\" | tee test-file1 test-file2 &&\n→.git add test-file1 test-file2 &&\n→.git commit -m \"Initial commit\" &&\n→.echo \"commit-1-state\" | tee test-file1 test-file2 &&\n→.git commit -m \"commit 1\" -i test-file1 test-file2 &&\n→.git tag commit-1 &&\n→.git checkout -b branch-2 HEAD^1 &&\n→.echo \"commit-2-state\" | tee test-file1 test-file2 &&\n→.git commit -m \"commit 2\" -i test-file1 test-file2 &&\n→.! $(git merge --no-commit commit-1) &&\n→.echo \"commit-2-state\" | tee test-file1 &&\n→.git add test-file1 &&\n→.git commit --dry-run &&\n→.git commit -m \"conflicts fixed from merge.\"\n\nAnd once inside gdb did this:\n\n(gdb) b wt-status.c:766\nBreakpoint 1 at 0x205d73: file wt-status.c, line 766.\n(gdb) r\nStarting program: /home/pockets/git/git commit --dry-run\n[Thread debugging using libthread_db enabled]\nUsing host libthread_db library \"/usr/lib/libthread_db.so.1\".\nOn branch branch-2\nYou have unmerged paths.\n (fix conflicts and run \"git commit\")\n (use \"git merge --abort\" to abort the merge)\n\n\nBreakpoint 1, wt_longstatus_print_updated (s=0x555555a29960 ) at\nwt-status.c:766\nwarning: Source file is more recent than executable.\n760 for (i = 0; i < s->change.nr; i++) {\n(gdb) print s->change.nr\n$1 = 1\n\nCan you confirm I'm not crazy, and am analyzing this correctly?\n\n> I _think_ with the change to \"what happens during merge\" above that\n> I suggested, this loop automatically becomes correct, but I didn't\n> think it through. If there are ways other than .merge_in_progress\n> that place conflicted entries in the index, then this loop is still\n> incorrect and would want to be more like:\n>\n> for (i = 0; i < s->change.nr; i++) {\n> struct wt_status_change_data *d = (s->change.items[i]).util;\n>\n> if (d->index_status == DIFF_STATUS_UNMERGED) {\n> s->committable = 0;\n> return;\n> }\n> if (d->index_status)\n> s->committable = 1;\n> }\n>\n> i.e. we declare \"not ready to commit\" if there is *any* conflicted\n> entry, but otherwise set committable to 1 if we see any entry that\n> is different from HEAD (to declare succcess once we successfully\n> loop through to the last entry without seeing any conflict).\n>\n> > void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)\n> > {\n> > wt_status_collect_changes_worktree(s);\n> > @@ -728,6 +761,8 @@ void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)\n> > wt_status_collect_changes_index(s);\n> >\n> > wt_status_collect_untracked(s);\n> > +\n> > + wt_status_mark_committable(s, state);\n> > }\n> >\n> > static void wt_longstatus_print_unmerged(const struct wt_status *s)\n> > @@ -753,28 +788,28 @@ static void wt_longstatus_print_unmerged(const struct wt_status *s)\n> >\n> > }\n> >\n> > -static void wt_longstatus_print_updated(struct wt_status *s)\n> > +static void wt_longstatus_print_updated(const struct wt_status *s)\n> > {\n> > - int shown_header = 0;\n> > int i;\n> >\n> > + if (!s->committable) {\n> > + return;\n> > + }\n>\n> No need to have {} around a single statement. Especially when you\n> know you won't be touching the line (e.g. to later add more\n> statements in the block) in this last patch in a series.\n>\n> > + wt_longstatus_print_cached_header(s);\n> > +\n"},{"id":"353805","messageId":"20180723020903.22435-1-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180715110807.25544-1-sxlijin@gmail.com","subject":"[PATCH v4 0/4] Rerolling patch series to fix t7501","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-07-23T02:08:59Z","receivedAt":"2018-07-29T00:42:38Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Following up on Junio's review from last time.\n\nSamuel Lijin (4):\n t7501: add coverage for flags which imply dry runs\n wt-status: rename commitable to committable\n wt-status: teach wt_status_collect about merges in progress\n commit: fix exit code when doing a dry run\n\n builtin/commit.c | 32 +++---\n ref-filter.c | 3 +-\n t/t7501-commit.sh | 150 ++++++++++++++++++++++++---\n wt-status.c | 258 ++++++++++++++++++++++++----------------------\n wt-status.h | 13 +--\n 5 files changed, 298 insertions(+), 158 deletions(-)\n\n-- \n2.18.0\n\n"},{"id":"353806","messageId":"20180723020903.22435-2-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180715110807.25544-1-sxlijin@gmail.com","subject":"[PATCH v4 1/4] t7501: add coverage for flags which imply dry runs","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-07-23T02:09:00Z","receivedAt":"2018-07-29T00:43:10Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"The behavior of git commit, when doing a dry run, changes if there are\nunresolved/resolved merge conflicts, but the test suite currently only\nasserts that `git commit --dry-run` succeeds when all merge conflicts\nare resolved.\n\nAdd tests to document the behavior of all flags (i.e. `--dry-run`,\n`--short`, `--porcelain`, and `--long`) which imply a dry run when (1)\nthere are only unresolved merge conflicts, (2) when there are both\nunresolved and resolved merge conflicts, and (3) when all merge\nconflicts are resolved.\n\nWhen testing behavior involving resolved merge conflicts, resolve merge\nconflicts by replacing each merge conflict with completely new contents,\nrather than choosing the contents associated with one of the parent\ncommits, since the latter decision has no bearing on the behavior of a\ndry run commit invocation.\n\nVerify that a dry run invocation of git commit does not create a new\ncommit by asserting that HEAD has not changed, instead of by crafting\nthe commit.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7501-commit.sh | 146 +++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 132 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex 9dbbd01fc..e49dfd0a2 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -664,24 +664,142 @@ test_expect_success '--only works on to-be-born branch' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success '--dry-run with conflicts fixed from a merge' '\n-\t# setup two branches with conflicting information\n-\t# in the same file, resolve the conflict,\n-\t# call commit with --dry-run\n-\techo \"Initial contents, unimportant\" >test-file &&\n-\tgit add test-file &&\n+test_expect_success 'prepare commits that can be used to trigger a merge conflict' '\n+\t# setup two branches with conflicting contents in two paths\n+\techo \"Initial contents, unimportant\" | tee test-file1 test-file2 &&\n+\tgit add test-file1 test-file2 &&\n \tgit commit -m \"Initial commit\" &&\n-\techo \"commit-1-state\" >test-file &&\n-\tgit commit -m \"commit 1\" -i test-file &&\n+\techo \"commit-1-state\" | tee test-file1 test-file2 &&\n+\tgit commit -m \"commit 1\" -i test-file1 test-file2 &&\n \tgit tag commit-1 &&\n \tgit checkout -b branch-2 HEAD^1 &&\n-\techo \"commit-2-state\" >test-file &&\n-\tgit commit -m \"commit 2\" -i test-file &&\n-\t! $(git merge --no-commit commit-1) &&\n-\techo \"commit-2-state\" >test-file &&\n-\tgit add test-file &&\n+\techo \"commit-2-state\" | tee test-file1 test-file2 &&\n+\tgit commit -m \"commit 2\" -i test-file1 test-file2 &&\n+\tgit tag commit-2\n+'\n+\n+test_expect_success '--dry-run with only unresolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\ttest_must_fail git commit --dry-run &&\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--short with only unresolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\ttest_must_fail git commit --short &&\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--porcelain with only unresolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\ttest_must_fail git commit --porcelain &&\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--long with only unresolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\ttest_must_fail git commit --long &&\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_failure '--dry-run with resolved and unresolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\techo \"resolve one merge conflict\" >test-file1 &&\n+\tgit add test-file1 &&\n+\ttest_must_fail git commit --dry-run &&\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--short with resolved and unresolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\techo \"resolve one merge conflict\" >test-file1 &&\n+\tgit add test-file1 &&\n+\ttest_must_fail git commit --short &&\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--porcelain with resolved and unresolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\techo \"resolve one merge conflict\" >test-file1 &&\n+\tgit add test-file1 &&\n+\ttest_must_fail git commit --porcelain &&\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_failure '--long with resolved and unresolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\techo \"resolve one merge conflict\" >test-file1 &&\n+\tgit add test-file1 &&\n+\ttest_must_fail git commit --long &&\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--dry-run with only resolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\techo \"resolve all merge conflicts\" | tee test-file1 test-file2 &&\n+\tgit add test-file1 test-file2 &&\n \tgit commit --dry-run &&\n-\tgit commit -m \"conflicts fixed from merge.\"\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_failure '--short with only resolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\techo \"resolve all merge conflicts\" | tee test-file1 test-file2 &&\n+\tgit add test-file1 test-file2 &&\n+\tgit commit --short &&\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_failure '--porcelain with only resolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\techo \"resolve all merge conflicts\" | tee test-file1 test-file2 &&\n+\tgit add test-file1 test-file2 &&\n+\tgit commit --porcelain &&\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--long with only resolved merge conflicts' '\n+\tgit reset --hard commit-2 &&\n+\ttest_must_fail git merge --no-commit commit-1 &&\n+\techo \"resolve all merge conflicts\" | tee test-file1 test-file2 &&\n+\tgit add test-file1 test-file2 &&\n+\tgit commit --long &&\n+\tgit rev-parse commit-2 >expected &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expected actual\n '\n \n test_done\n-- \n2.18.0\n\n"},{"id":"353807","messageId":"20180723020903.22435-5-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180715110807.25544-1-sxlijin@gmail.com","subject":"[PATCH v4 4/4] commit: fix exit code when doing a dry run","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-07-23T02:09:03Z","receivedAt":"2018-07-29T00:43:10Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"In wt-status.c, the s->committable bit is set only in the call tree of\nwt_longstatus_print(), and it is not always set correctly. This means\nthat in normal cases, if there are changes to be committed, or if there\nis a merge in progress and all conflicts have been resolved, `--dry-run`\nand `--long` return the correct exit code but `--short` and\n`--porcelain` do not, even though all four flags imply dry run.\nMoreover, if there is a merge in progress and some but not all conflicts\nhave been resolved, `--short` and `--porcelain` only return the correct\nexit code by coincidence (because the codepaths they follow never touch\nthe committable bit), whereas `--dry-run` and `--long` return the wrong\nexit code.\n\nTeach wt_status_collect() to set s->committable correctly (if a merge is\nin progress, committable should be set iff there are no unmerged\nchanges; otherwise, committable should be set iff there are changes in\nthe index) so that all four flags which imply dry runs return the\ncorrect exit code in the above described situations and mark the\ndocumenting tests as fixed.\n\nUse the index_status field in the wt_status_change_data structs in\nhas_unmerged() to determine whether or not there are unmerged paths,\ninstead of the stagemask field, to improve readability.\n\nAlso stop setting s->committable in wt_longstatus_print_updated() and\nshow_merge_in_progress(), and const-ify wt_status_state in the method\nsignatures in those callpaths.\n\nSigned-off-by: Samuel Lijin \n---\n t/t7501-commit.sh | 12 +++----\n wt-status.c | 80 +++++++++++++++++++++++++++++------------------\n 2 files changed, 55 insertions(+), 37 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex e49dfd0a2..6dba526e6 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -99,12 +99,12 @@ test_expect_success '--dry-run with stuff to commit returns ok' '\n \tgit commit -m next -a --dry-run\n '\n \n-test_expect_failure '--short with stuff to commit returns ok' '\n+test_expect_success '--short with stuff to commit returns ok' '\n \techo bongo bongo bongo >>file &&\n \tgit commit -m next -a --short\n '\n \n-test_expect_failure '--porcelain with stuff to commit returns ok' '\n+test_expect_success '--porcelain with stuff to commit returns ok' '\n \techo bongo bongo bongo >>file &&\n \tgit commit -m next -a --porcelain\n '\n@@ -714,7 +714,7 @@ test_expect_success '--long with only unresolved merge conflicts' '\n \ttest_cmp expected actual\n '\n \n-test_expect_failure '--dry-run with resolved and unresolved merge conflicts' '\n+test_expect_success '--dry-run with resolved and unresolved merge conflicts' '\n \tgit reset --hard commit-2 &&\n \ttest_must_fail git merge --no-commit commit-1 &&\n \techo \"resolve one merge conflict\" >test-file1 &&\n@@ -747,7 +747,7 @@ test_expect_success '--porcelain with resolved and unresolved merge conflicts' '\n \ttest_cmp expected actual\n '\n \n-test_expect_failure '--long with resolved and unresolved merge conflicts' '\n+test_expect_success '--long with resolved and unresolved merge conflicts' '\n \tgit reset --hard commit-2 &&\n \ttest_must_fail git merge --no-commit commit-1 &&\n \techo \"resolve one merge conflict\" >test-file1 &&\n@@ -769,7 +769,7 @@ test_expect_success '--dry-run with only resolved merge conflicts' '\n \ttest_cmp expected actual\n '\n \n-test_expect_failure '--short with only resolved merge conflicts' '\n+test_expect_success '--short with only resolved merge conflicts' '\n \tgit reset --hard commit-2 &&\n \ttest_must_fail git merge --no-commit commit-1 &&\n \techo \"resolve all merge conflicts\" | tee test-file1 test-file2 &&\n@@ -780,7 +780,7 @@ test_expect_failure '--short with only resolved merge conflicts' '\n \ttest_cmp expected actual\n '\n \n-test_expect_failure '--porcelain with only resolved merge conflicts' '\n+test_expect_success '--porcelain with only resolved merge conflicts' '\n \tgit reset --hard commit-2 &&\n \ttest_must_fail git merge --no-commit commit-1 &&\n \techo \"resolve all merge conflicts\" | tee test-file1 test-file2 &&\ndiff --git a/wt-status.c b/wt-status.c\nindex af83fae68..fc239f61c 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -724,6 +724,38 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \t\ts->untracked_in_ms = (getnanotime() - t_begin) / 1000000;\n }\n \n+static int has_unmerged(const struct wt_status *s)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < s->change.nr; i++) {\n+\t\tstruct wt_status_change_data *d = (s->change.items[i]).util;\n+\t\tif (d->index_status == DIFF_STATUS_UNMERGED)\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static void wt_status_mark_committable(\n+\t\tstruct wt_status *s, const struct wt_status_state *state)\n+{\n+\tint i;\n+\n+\tif (state->merge_in_progress) {\n+\t\ts->committable = !has_unmerged(s);\n+\t\treturn;\n+\t}\n+\n+\tfor (i = 0; i < s->change.nr; i++) {\n+\t\tstruct wt_status_change_data *d = (s->change.items[i]).util;\n+\n+\t\tif (d->index_status) {\n+\t\t\ts->committable = 1;\n+\t\t\treturn;\n+\t\t}\n+\t}\n+}\n+\n void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)\n {\n \twt_status_collect_changes_worktree(s);\n@@ -734,6 +766,8 @@ void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)\n \t\twt_status_collect_changes_index(s);\n \n \twt_status_collect_untracked(s);\n+\n+\twt_status_mark_committable(s, state);\n }\n \n static void wt_longstatus_print_unmerged(const struct wt_status *s)\n@@ -759,28 +793,27 @@ static void wt_longstatus_print_unmerged(const struct wt_status *s)\n \n }\n \n-static void wt_longstatus_print_updated(struct wt_status *s)\n+static void wt_longstatus_print_updated(const struct wt_status *s)\n {\n-\tint shown_header = 0;\n \tint i;\n \n+\tif (!s->committable)\n+\t\treturn;\n+\n+\twt_longstatus_print_cached_header(s);\n+\n \tfor (i = 0; i < s->change.nr; i++) {\n \t\tstruct wt_status_change_data *d;\n \t\tstruct string_list_item *it;\n \t\tit = &(s->change.items[i]);\n \t\td = it->util;\n-\t\tif (!d->index_status ||\n-\t\t d->index_status == DIFF_STATUS_UNMERGED)\n-\t\t\tcontinue;\n-\t\tif (!shown_header) {\n-\t\t\twt_longstatus_print_cached_header(s);\n-\t\t\ts->committable = 1;\n-\t\t\tshown_header = 1;\n+\t\tif (d->index_status &&\n+\t\t d->index_status != DIFF_STATUS_UNMERGED) {\n+\t\t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n \t\t}\n-\t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n \t}\n-\tif (shown_header)\n-\t\twt_longstatus_print_trailer(s);\n+\n+\twt_longstatus_print_trailer(s);\n }\n \n /*\n@@ -1064,21 +1097,7 @@ static void wt_longstatus_print_tracking(const struct wt_status *s)\n \tstrbuf_release(&sb);\n }\n \n-static int has_unmerged(const struct wt_status *s)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < s->change.nr; i++) {\n-\t\tstruct wt_status_change_data *d;\n-\t\td = s->change.items[i].util;\n-\t\tif (d->stagemask)\n-\t\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n-static void show_merge_in_progress(struct wt_status *s,\n-\t\t\t\tconst struct wt_status_state *state,\n+static void show_merge_in_progress(const struct wt_status *s,\n \t\t\t\tconst char *color)\n {\n \tif (has_unmerged(s)) {\n@@ -1090,7 +1109,6 @@ static void show_merge_in_progress(struct wt_status *s,\n \t\t\t\t\t _(\" (use \\\"git merge --abort\\\" to abort the merge)\"));\n \t\t}\n \t} else {\n-\t\ts-> committable = 1;\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"All conflicts fixed but you are still merging.\"));\n \t\tif (s->hints)\n@@ -1584,12 +1602,12 @@ void wt_status_clear_state(struct wt_status_state *state)\n \tfree(state->detached_from);\n }\n \n-static void wt_longstatus_print_state(struct wt_status *s,\n+static void wt_longstatus_print_state(const struct wt_status *s,\n \t\t\t\t const struct wt_status_state *state)\n {\n \tconst char *state_color = color(WT_STATUS_HEADER, s);\n \tif (state->merge_in_progress)\n-\t\tshow_merge_in_progress(s, state, state_color);\n+\t\tshow_merge_in_progress(s, state_color);\n \telse if (state->am_in_progress)\n \t\tshow_am_in_progress(s, state, state_color);\n \telse if (state->rebase_in_progress || state->rebase_interactive_in_progress)\n@@ -1602,7 +1620,7 @@ static void wt_longstatus_print_state(struct wt_status *s,\n \t\tshow_bisect_in_progress(s, state, state_color);\n }\n \n-static void wt_longstatus_print(struct wt_status *s, const struct wt_status_state *state)\n+static void wt_longstatus_print(const struct wt_status *s, const struct wt_status_state *state)\n {\n \tconst char *branch_color = color(WT_STATUS_ONBRANCH, s);\n \tconst char *branch_status_color = color(WT_STATUS_HEADER, s);\n-- \n2.18.0\n\n"},{"id":"353808","messageId":"20180723020903.22435-3-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180715110807.25544-1-sxlijin@gmail.com","subject":"[PATCH v4 2/4] wt-status: rename commitable to committable","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-07-23T02:09:01Z","receivedAt":"2018-07-29T00:43:10Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"Fix a typo in the name of the committable bit in wt_status_state.\n\nSigned-off-by: Samuel Lijin \n---\n builtin/commit.c | 18 +++++++++---------\n wt-status.c | 10 +++++-----\n wt-status.h | 2 +-\n 3 files changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 158e3f843..32f9db33b 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -507,7 +507,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \twt_status_collect(s);\n \twt_status_print(s);\n \n-\treturn s->commitable;\n+\treturn s->committable;\n }\n \n static int is_a_merge(const struct commit *current_head)\n@@ -653,7 +653,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n {\n \tstruct stat statbuf;\n \tstruct strbuf committer_ident = STRBUF_INIT;\n-\tint commitable;\n+\tint committable;\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *hook_arg1 = NULL;\n \tconst char *hook_arg2 = NULL;\n@@ -870,7 +870,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \n \t\tsaved_color_setting = s->use_color;\n \t\ts->use_color = 0;\n-\t\tcommitable = run_status(s->fp, index_file, prefix, 1, s);\n+\t\tcommittable = run_status(s->fp, index_file, prefix, 1, s);\n \t\ts->use_color = saved_color_setting;\n \t} else {\n \t\tstruct object_id oid;\n@@ -888,7 +888,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\tfor (i = 0; i < active_nr; i++)\n \t\t\t\tif (ce_intent_to_add(active_cache[i]))\n \t\t\t\t\tita_nr++;\n-\t\t\tcommitable = active_nr - ita_nr > 0;\n+\t\t\tcommittable = active_nr - ita_nr > 0;\n \t\t} else {\n \t\t\t/*\n \t\t\t * Unless the user did explicitly request a submodule\n@@ -904,7 +904,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\tif (ignore_submodule_arg &&\n \t\t\t !strcmp(ignore_submodule_arg, \"all\"))\n \t\t\t\tflags.ignore_submodules = 1;\n-\t\t\tcommitable = index_differs_from(parent, &flags, 1);\n+\t\t\tcommittable = index_differs_from(parent, &flags, 1);\n \t\t}\n \t}\n \tstrbuf_release(&committer_ident);\n@@ -916,7 +916,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t * explicit --allow-empty. In the cherry-pick case, it may be\n \t * empty due to conflict resolution, which the user should okay.\n \t */\n-\tif (!commitable && whence != FROM_MERGE && !allow_empty &&\n+\tif (!committable && whence != FROM_MERGE && !allow_empty &&\n \t !(amend && is_a_merge(current_head))) {\n \t\ts->display_comment_prefix = old_display_comment_prefix;\n \t\trun_status(stdout, index_file, prefix, 0, s);\n@@ -1186,14 +1186,14 @@ static int parse_and_validate_options(int argc, const char *argv[],\n static int dry_run_commit(int argc, const char **argv, const char *prefix,\n \t\t\t const struct commit *current_head, struct wt_status *s)\n {\n-\tint commitable;\n+\tint committable;\n \tconst char *index_file;\n \n \tindex_file = prepare_index(argc, argv, prefix, current_head, 1);\n-\tcommitable = run_status(stdout, index_file, prefix, 0, s);\n+\tcommittable = run_status(stdout, index_file, prefix, 0, s);\n \trollback_index_files();\n \n-\treturn commitable ? 0 : 1;\n+\treturn committable ? 0 : 1;\n }\n \n define_list_config_array_extra(color_status_slots, {\"added\"});\ndiff --git a/wt-status.c b/wt-status.c\nindex 8827a256d..18ea333a5 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -773,7 +773,7 @@ static void wt_longstatus_print_updated(struct wt_status *s)\n \t\t\tcontinue;\n \t\tif (!shown_header) {\n \t\t\twt_longstatus_print_cached_header(s);\n-\t\t\ts->commitable = 1;\n+\t\t\ts->committable = 1;\n \t\t\tshown_header = 1;\n \t\t}\n \t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n@@ -1008,7 +1008,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \t\trev.diffopt.use_color = 0;\n \t\twt_status_add_cut_line(s->fp);\n \t}\n-\tif (s->verbose > 1 && s->commitable) {\n+\tif (s->verbose > 1 && s->committable) {\n \t\t/* print_updated() printed a header, so do we */\n \t\tif (s->fp != stdout)\n \t\t\twt_longstatus_print_trailer(s);\n@@ -1089,7 +1089,7 @@ static void show_merge_in_progress(struct wt_status *s,\n \t\t\t\t\t _(\" (use \\\"git merge --abort\\\" to abort the merge)\"));\n \t\t}\n \t} else {\n-\t\ts-> commitable = 1;\n+\t\ts-> committable = 1;\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"All conflicts fixed but you are still merging.\"));\n \t\tif (s->hints)\n@@ -1665,14 +1665,14 @@ static void wt_longstatus_print(struct wt_status *s)\n \t\t\t\t\t \"new files yourself (see 'git help status').\"),\n \t\t\t\t\t s->untracked_in_ms / 1000.0);\n \t\t}\n-\t} else if (s->commitable)\n+\t} else if (s->committable)\n \t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, _(\"Untracked files not listed%s\"),\n \t\t\ts->hints\n \t\t\t? _(\" (use -u option to show untracked files)\") : \"\");\n \n \tif (s->verbose)\n \t\twt_longstatus_print_verbose(s);\n-\tif (!s->commitable) {\n+\tif (!s->committable) {\n \t\tif (s->amend)\n \t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, _(\"No changes\"));\n \t\telse if (s->nowarn)\ndiff --git a/wt-status.h b/wt-status.h\nindex 1673d146f..937b2c352 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -96,7 +96,7 @@ struct wt_status {\n \tunsigned char sha1_commit[GIT_MAX_RAWSZ]; /* when not Initial */\n \n \t/* These are computed during processing of the individual sections */\n-\tint commitable;\n+\tint committable;\n \tint workdir_dirty;\n \tconst char *index_file;\n \tFILE *fp;\n-- \n2.18.0\n\n"},{"id":"353809","messageId":"20180723020903.22435-4-sxlijin@gmail.com","threadId":"48321","inReplyTo":"20180715110807.25544-1-sxlijin@gmail.com","subject":"[PATCH v4 3/4] wt-status: teach wt_status_collect about merges in progress","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2018-07-23T02:09:02Z","receivedAt":"2018-07-29T00:43:10Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"To fix the breakages documented by t7501, the next patch in this series\nwill teach wt_status_collect() how to set the committable bit, instead\nof having wt_longstatus_print_updated() and show_merge_in_progress() set\nit (which is what currently happens). To set the committable bit\ncorrectly, however, wt_status_collect() needs to know whether or not\nthere is a merge in progress (if a merge is in progress, the bit (1)\nshould not be set if there are unresolved merge conflicts and (2) should\nbe set even if the index is the same as HEAD), so teach its (two)\ncallers to create, initialize, and pass in\ninstances of wt_status_state, which records this metadata.\n\nSince wt_longstatus_print() and show_merge_in_progress() are in the same\ncallpaths and currently create and init copies of wt_status_state,\nremove that logic and instead pass wt_status_state through.\n\nMake wt_status_get_state() easier to use, add a helper method to clean up\nwt_status_state, const-ify as many struct pointers in method signatures\nas possible, and add a FIXME for a struct pointer which should be const\nbut isn't (that this patch series will not address).\n\nSigned-off-by: Samuel Lijin \n---\ngitster: I kept the FIXME around because it wasn't clear whether or not\nyou were opposed to it. For what it's worth, there are only two\ncallsites that can't be const-ified because of this one item.\n\n builtin/commit.c | 14 ++--\n ref-filter.c | 3 +-\n wt-status.c | 178 +++++++++++++++++++++++------------------------\n wt-status.h | 11 +--\n 4 files changed, 105 insertions(+), 101 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 32f9db33b..dd3e83053 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -485,6 +485,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn,\n \t\t struct wt_status *s)\n {\n+\tstruct wt_status_state state;\n \tstruct object_id oid;\n \n \tif (s->relative_paths)\n@@ -504,8 +505,10 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \ts->status_format = status_format;\n \ts->ignore_submodule_arg = ignore_submodule_arg;\n \n-\twt_status_collect(s);\n-\twt_status_print(s);\n+\twt_status_get_state(s, &state);\n+\twt_status_collect(s, &state);\n+\twt_status_print(s, &state);\n+\twt_status_clear_state(&state);\n \n \treturn s->committable;\n }\n@@ -1295,6 +1298,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \tstatic int no_renames = -1;\n \tstatic const char *rename_score_arg = (const char *)-1;\n \tstatic struct wt_status s;\n+\tstruct wt_status_state state;\n \tint fd;\n \tstruct object_id oid;\n \tstatic struct option builtin_status_options[] = {\n@@ -1379,7 +1383,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \t\t\ts.rename_score = parse_rename_score(&rename_score_arg);\n \t}\n \n-\twt_status_collect(&s);\n+\twt_status_get_state(&s, &state);\n+\twt_status_collect(&s, &state);\n \n \tif (0 <= fd)\n \t\tupdate_index_if_able(&the_index, &index_lock);\n@@ -1387,7 +1392,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \tif (s.relative_paths)\n \t\ts.prefix = prefix;\n \n-\twt_status_print(&s);\n+\twt_status_print(&s, &state);\n+\twt_status_clear_state(&state);\n \treturn 0;\n }\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex 492f2b770..bc9b6b274 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1381,8 +1381,7 @@ char *get_head_description(void)\n {\n \tstruct strbuf desc = STRBUF_INIT;\n \tstruct wt_status_state state;\n-\tmemset(&state, 0, sizeof(state));\n-\twt_status_get_state(&state, 1);\n+\twt_status_get_state(NULL, &state);\n \tif (state.rebase_in_progress ||\n \t state.rebase_interactive_in_progress) {\n \t\tif (state.branch)\ndiff --git a/wt-status.c b/wt-status.c\nindex 18ea333a5..af83fae68 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -33,7 +33,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_NIL, /* WT_STATUS_ONBRANCH */\n };\n \n-static const char *color(int slot, struct wt_status *s)\n+static const char *color(int slot, const struct wt_status *s)\n {\n \tconst char *c = \"\";\n \tif (want_color(s->use_color))\n@@ -43,7 +43,7 @@ static const char *color(int slot, struct wt_status *s)\n \treturn c;\n }\n \n-static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n+static void status_vprintf(const struct wt_status *s, int at_bol, const char *color,\n \t\tconst char *fmt, va_list ap, const char *trail)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -89,7 +89,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n \tstrbuf_release(&sb);\n }\n \n-void status_printf_ln(struct wt_status *s, const char *color,\n+void status_printf_ln(const struct wt_status *s, const char *color,\n \t\t\tconst char *fmt, ...)\n {\n \tva_list ap;\n@@ -99,7 +99,7 @@ void status_printf_ln(struct wt_status *s, const char *color,\n \tva_end(ap);\n }\n \n-void status_printf(struct wt_status *s, const char *color,\n+void status_printf(const struct wt_status *s, const char *color,\n \t\t\tconst char *fmt, ...)\n {\n \tva_list ap;\n@@ -109,7 +109,7 @@ void status_printf(struct wt_status *s, const char *color,\n \tva_end(ap);\n }\n \n-static void status_printf_more(struct wt_status *s, const char *color,\n+static void status_printf_more(const struct wt_status *s, const char *color,\n \t\t\t const char *fmt, ...)\n {\n \tva_list ap;\n@@ -143,7 +143,7 @@ void wt_status_prepare(struct wt_status *s)\n \ts->rename_limit = -1;\n }\n \n-static void wt_longstatus_print_unmerged_header(struct wt_status *s)\n+static void wt_longstatus_print_unmerged_header(const struct wt_status *s)\n {\n \tint i;\n \tint del_mod_conflict = 0;\n@@ -195,7 +195,7 @@ static void wt_longstatus_print_unmerged_header(struct wt_status *s)\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_cached_header(struct wt_status *s)\n+static void wt_longstatus_print_cached_header(const struct wt_status *s)\n {\n \tconst char *c = color(WT_STATUS_HEADER, s);\n \n@@ -211,7 +211,7 @@ static void wt_longstatus_print_cached_header(struct wt_status *s)\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_dirty_header(struct wt_status *s,\n+static void wt_longstatus_print_dirty_header(const struct wt_status *s,\n \t\t\t\t\t int has_deleted,\n \t\t\t\t\t int has_dirty_submodules)\n {\n@@ -230,7 +230,7 @@ static void wt_longstatus_print_dirty_header(struct wt_status *s,\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_other_header(struct wt_status *s,\n+static void wt_longstatus_print_other_header(const struct wt_status *s,\n \t\t\t\t\t const char *what,\n \t\t\t\t\t const char *how)\n {\n@@ -242,7 +242,7 @@ static void wt_longstatus_print_other_header(struct wt_status *s,\n \tstatus_printf_ln(s, c, \"%s\", \"\");\n }\n \n-static void wt_longstatus_print_trailer(struct wt_status *s)\n+static void wt_longstatus_print_trailer(const struct wt_status *s)\n {\n \tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n }\n@@ -308,7 +308,7 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval)\n \treturn result;\n }\n \n-static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n+static void wt_longstatus_print_unmerged_data(const struct wt_status *s,\n \t\t\t\t\t struct string_list_item *it)\n {\n \tconst char *c = color(WT_STATUS_UNMERGED, s);\n@@ -335,7 +335,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n \tstrbuf_release(&onebuf);\n }\n \n-static void wt_longstatus_print_change_data(struct wt_status *s,\n+static void wt_longstatus_print_change_data(const struct wt_status *s,\n \t\t\t\t\t int change_type,\n \t\t\t\t\t struct string_list_item *it)\n {\n@@ -724,7 +724,7 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \t\ts->untracked_in_ms = (getnanotime() - t_begin) / 1000000;\n }\n \n-void wt_status_collect(struct wt_status *s)\n+void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)\n {\n \twt_status_collect_changes_worktree(s);\n \n@@ -732,10 +732,11 @@ void wt_status_collect(struct wt_status *s)\n \t\twt_status_collect_changes_initial(s);\n \telse\n \t\twt_status_collect_changes_index(s);\n+\n \twt_status_collect_untracked(s);\n }\n \n-static void wt_longstatus_print_unmerged(struct wt_status *s)\n+static void wt_longstatus_print_unmerged(const struct wt_status *s)\n {\n \tint shown_header = 0;\n \tint i;\n@@ -787,7 +788,7 @@ static void wt_longstatus_print_updated(struct wt_status *s)\n * 0 : no change\n * 1 : some change but no delete\n */\n-static int wt_status_check_worktree_changes(struct wt_status *s,\n+static int wt_status_check_worktree_changes(const struct wt_status *s,\n \t\t\t\t\t int *dirty_submodules)\n {\n \tint i;\n@@ -811,7 +812,7 @@ static int wt_status_check_worktree_changes(struct wt_status *s,\n \treturn changes;\n }\n \n-static void wt_longstatus_print_changed(struct wt_status *s)\n+static void wt_longstatus_print_changed(const struct wt_status *s)\n {\n \tint i, dirty_submodules;\n \tint worktree_changes = wt_status_check_worktree_changes(s, &dirty_submodules);\n@@ -843,7 +844,7 @@ static int stash_count_refs(struct object_id *ooid, struct object_id *noid,\n \treturn 0;\n }\n \n-static void wt_longstatus_print_stash_summary(struct wt_status *s)\n+static void wt_longstatus_print_stash_summary(const struct wt_status *s)\n {\n \tint stash_count = 0;\n \n@@ -855,7 +856,7 @@ static void wt_longstatus_print_stash_summary(struct wt_status *s)\n \t\t\t\t stash_count);\n }\n \n-static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncommitted)\n+static void wt_longstatus_print_submodule_summary(const struct wt_status *s, int uncommitted)\n {\n \tstruct child_process sm_summary = CHILD_PROCESS_INIT;\n \tstruct strbuf cmd_stdout = STRBUF_INIT;\n@@ -901,8 +902,8 @@ static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncom\n \tstrbuf_release(&summary);\n }\n \n-static void wt_longstatus_print_other(struct wt_status *s,\n-\t\t\t\t struct string_list *l,\n+static void wt_longstatus_print_other(const struct wt_status *s,\n+\t\t\t\t const struct string_list *l,\n \t\t\t\t const char *what,\n \t\t\t\t const char *how)\n {\n@@ -975,7 +976,7 @@ void wt_status_add_cut_line(FILE *fp)\n \tstrbuf_release(&buf);\n }\n \n-static void wt_longstatus_print_verbose(struct wt_status *s)\n+static void wt_longstatus_print_verbose(const struct wt_status *s)\n {\n \tstruct rev_info rev;\n \tstruct setup_revision_opt opt;\n@@ -1029,7 +1030,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \t}\n }\n \n-static void wt_longstatus_print_tracking(struct wt_status *s)\n+static void wt_longstatus_print_tracking(const struct wt_status *s)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *cp, *ep, *branch_name;\n@@ -1063,7 +1064,7 @@ static void wt_longstatus_print_tracking(struct wt_status *s)\n \tstrbuf_release(&sb);\n }\n \n-static int has_unmerged(struct wt_status *s)\n+static int has_unmerged(const struct wt_status *s)\n {\n \tint i;\n \n@@ -1077,7 +1078,7 @@ static int has_unmerged(struct wt_status *s)\n }\n \n static void show_merge_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n+\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tif (has_unmerged(s)) {\n@@ -1099,8 +1100,8 @@ static void show_merge_in_progress(struct wt_status *s,\n \twt_longstatus_print_trailer(s);\n }\n \n-static void show_am_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n+static void show_am_in_progress(const struct wt_status *s,\n+\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tstatus_printf_ln(s, color,\n@@ -1138,7 +1139,7 @@ static char *read_line_from_git_path(const char *filename)\n \t}\n }\n \n-static int split_commit_in_progress(struct wt_status *s)\n+static int split_commit_in_progress(const struct wt_status *s)\n {\n \tint split_in_progress = 0;\n \tchar *head, *orig_head, *rebase_amend, *rebase_orig_head;\n@@ -1232,8 +1233,8 @@ static int read_rebase_todolist(const char *fname, struct string_list *lines)\n \treturn 0;\n }\n \n-static void show_rebase_information(struct wt_status *s,\n-\t\t\t\t\tstruct wt_status_state *state,\n+static void show_rebase_information(const struct wt_status *s,\n+\t\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\t\tconst char *color)\n {\n \tif (state->rebase_interactive_in_progress) {\n@@ -1286,8 +1287,8 @@ static void show_rebase_information(struct wt_status *s,\n \t}\n }\n \n-static void print_rebase_state(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n+static void print_rebase_state(const struct wt_status *s,\n+\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tif (state->branch)\n@@ -1300,8 +1301,8 @@ static void print_rebase_state(struct wt_status *s,\n \t\t\t\t _(\"You are currently rebasing.\"));\n }\n \n-static void show_rebase_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n+static void show_rebase_in_progress(const struct wt_status *s,\n+\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tstruct stat st;\n@@ -1353,8 +1354,8 @@ static void show_rebase_in_progress(struct wt_status *s,\n \twt_longstatus_print_trailer(s);\n }\n \n-static void show_cherry_pick_in_progress(struct wt_status *s,\n-\t\t\t\t\tstruct wt_status_state *state,\n+static void show_cherry_pick_in_progress(const struct wt_status *s,\n+\t\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\t\tconst char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently cherry-picking commit %s.\"),\n@@ -1372,8 +1373,8 @@ static void show_cherry_pick_in_progress(struct wt_status *s,\n \twt_longstatus_print_trailer(s);\n }\n \n-static void show_revert_in_progress(struct wt_status *s,\n-\t\t\t\t\tstruct wt_status_state *state,\n+static void show_revert_in_progress(const struct wt_status *s,\n+\t\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\t\tconst char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently reverting commit %s.\"),\n@@ -1391,8 +1392,8 @@ static void show_revert_in_progress(struct wt_status *s,\n \twt_longstatus_print_trailer(s);\n }\n \n-static void show_bisect_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n+static void show_bisect_in_progress(const struct wt_status *s,\n+\t\t\t\tconst struct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tif (state->branch)\n@@ -1546,12 +1547,16 @@ int wt_status_check_bisect(const struct worktree *wt,\n \treturn 0;\n }\n \n-void wt_status_get_state(struct wt_status_state *state,\n-\t\t\t int get_detached_from)\n+void wt_status_get_state(\n+\t\tconst struct wt_status *s, struct wt_status_state *state)\n {\n+\tint get_detached_from =\n+\t\t(s == NULL) || (s->branch && !strcmp(s->branch, \"HEAD\"));\n \tstruct stat st;\n \tstruct object_id oid;\n \n+\tmemset(state, 0, sizeof(*state));\n+\n \tif (!stat(git_path_merge_head(the_repository), &st)) {\n \t\tstate->merge_in_progress = 1;\n \t} else if (wt_status_check_rebase(NULL, state)) {\n@@ -1572,8 +1577,15 @@ void wt_status_get_state(struct wt_status_state *state,\n \t\twt_status_get_detached_from(state);\n }\n \n+void wt_status_clear_state(struct wt_status_state *state)\n+{\n+\tfree(state->branch);\n+\tfree(state->onto);\n+\tfree(state->detached_from);\n+}\n+\n static void wt_longstatus_print_state(struct wt_status *s,\n-\t\t\t\t struct wt_status_state *state)\n+\t\t\t\t const struct wt_status_state *state)\n {\n \tconst char *state_color = color(WT_STATUS_HEADER, s);\n \tif (state->merge_in_progress)\n@@ -1590,30 +1602,25 @@ static void wt_longstatus_print_state(struct wt_status *s,\n \t\tshow_bisect_in_progress(s, state, state_color);\n }\n \n-static void wt_longstatus_print(struct wt_status *s)\n+static void wt_longstatus_print(struct wt_status *s, const struct wt_status_state *state)\n {\n \tconst char *branch_color = color(WT_STATUS_ONBRANCH, s);\n \tconst char *branch_status_color = color(WT_STATUS_HEADER, s);\n-\tstruct wt_status_state state;\n-\n-\tmemset(&state, 0, sizeof(state));\n-\twt_status_get_state(&state,\n-\t\t\t s->branch && !strcmp(s->branch, \"HEAD\"));\n \n \tif (s->branch) {\n \t\tconst char *on_what = _(\"On branch \");\n \t\tconst char *branch_name = s->branch;\n \t\tif (!strcmp(branch_name, \"HEAD\")) {\n \t\t\tbranch_status_color = color(WT_STATUS_NOBRANCH, s);\n-\t\t\tif (state.rebase_in_progress || state.rebase_interactive_in_progress) {\n-\t\t\t\tif (state.rebase_interactive_in_progress)\n+\t\t\tif (state->rebase_in_progress || state->rebase_interactive_in_progress) {\n+\t\t\t\tif (state->rebase_interactive_in_progress)\n \t\t\t\t\ton_what = _(\"interactive rebase in progress; onto \");\n \t\t\t\telse\n \t\t\t\t\ton_what = _(\"rebase in progress; onto \");\n-\t\t\t\tbranch_name = state.onto;\n-\t\t\t} else if (state.detached_from) {\n-\t\t\t\tbranch_name = state.detached_from;\n-\t\t\t\tif (state.detached_at)\n+\t\t\t\tbranch_name = state->onto;\n+\t\t\t} else if (state->detached_from) {\n+\t\t\t\tbranch_name = state->detached_from;\n+\t\t\t\tif (state->detached_at)\n \t\t\t\t\ton_what = _(\"HEAD detached at \");\n \t\t\t\telse\n \t\t\t\t\ton_what = _(\"HEAD detached from \");\n@@ -1630,10 +1637,7 @@ static void wt_longstatus_print(struct wt_status *s)\n \t\t\twt_longstatus_print_tracking(s);\n \t}\n \n-\twt_longstatus_print_state(s, &state);\n-\tfree(state.branch);\n-\tfree(state.onto);\n-\tfree(state.detached_from);\n+\twt_longstatus_print_state(s, state);\n \n \tif (s->is_initial) {\n \t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n@@ -1708,7 +1712,7 @@ static void wt_longstatus_print(struct wt_status *s)\n }\n \n static void wt_shortstatus_unmerged(struct string_list_item *it,\n-\t\t\t struct wt_status *s)\n+\t\t\t const struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n \tconst char *how = \"??\";\n@@ -1735,7 +1739,7 @@ static void wt_shortstatus_unmerged(struct string_list_item *it,\n }\n \n static void wt_shortstatus_status(struct string_list_item *it,\n-\t\t\t struct wt_status *s)\n+\t\t\t const struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n \n@@ -1778,7 +1782,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n }\n \n static void wt_shortstatus_other(struct string_list_item *it,\n-\t\t\t\t struct wt_status *s, const char *sign)\n+\t\t\t\t const struct wt_status *s, const char *sign)\n {\n \tif (s->null_termination) {\n \t\tfprintf(stdout, \"%s %s%c\", sign, it->string, 0);\n@@ -1792,7 +1796,7 @@ static void wt_shortstatus_other(struct string_list_item *it,\n \t}\n }\n \n-static void wt_shortstatus_print_tracking(struct wt_status *s)\n+static void wt_shortstatus_print_tracking(const struct wt_status *s)\n {\n \tstruct branch *branch;\n \tconst char *header_color = color(WT_STATUS_HEADER, s);\n@@ -1868,7 +1872,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n \tfputc(s->null_termination ? '\\0' : '\\n', s->fp);\n }\n \n-static void wt_shortstatus_print(struct wt_status *s)\n+static void wt_shortstatus_print(const struct wt_status *s)\n {\n \tstruct string_list_item *it;\n \n@@ -1932,18 +1936,14 @@ static void wt_porcelain_print(struct wt_status *s)\n * upstream. When AHEAD_BEHIND_QUICK is requested and the branches\n * are different, '?' will be substituted for the actual count.\n */\n-static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n+static void wt_porcelain_v2_print_tracking(const struct wt_status *s, const struct wt_status_state *state)\n {\n \tstruct branch *branch;\n \tconst char *base;\n \tconst char *branch_name;\n-\tstruct wt_status_state state;\n \tint ab_info, nr_ahead, nr_behind;\n \tchar eol = s->null_termination ? '\\0' : '\\n';\n \n-\tmemset(&state, 0, sizeof(state));\n-\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n-\n \tfprintf(s->fp, \"# branch.oid %s%c\",\n \t\t\t(s->is_initial ? \"(initial)\" : sha1_to_hex(s->sha1_commit)),\n \t\t\teol);\n@@ -1954,10 +1954,10 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n \t\tif (!strcmp(s->branch, \"HEAD\")) {\n \t\t\tfprintf(s->fp, \"# branch.head %s%c\", \"(detached)\", eol);\n \n-\t\t\tif (state.rebase_in_progress || state.rebase_interactive_in_progress)\n-\t\t\t\tbranch_name = state.onto;\n-\t\t\telse if (state.detached_from)\n-\t\t\t\tbranch_name = state.detached_from;\n+\t\t\tif (state->rebase_in_progress || state->rebase_interactive_in_progress)\n+\t\t\t\tbranch_name = state->onto;\n+\t\t\telse if (state->detached_from)\n+\t\t\t\tbranch_name = state->detached_from;\n \t\t\telse\n \t\t\t\tbranch_name = \"\";\n \t\t} else {\n@@ -1991,10 +1991,6 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n \t\t\t}\n \t\t}\n \t}\n-\n-\tfree(state.branch);\n-\tfree(state.onto);\n-\tfree(state.detached_from);\n }\n \n /*\n@@ -2002,7 +1998,7 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n * fixed-length string of characters in the buffer provided.\n */\n static void wt_porcelain_v2_submodule_state(\n-\tstruct wt_status_change_data *d,\n+\tconst struct wt_status_change_data *d,\n \tchar sub[5])\n {\n \tif (S_ISGITLINK(d->mode_head) ||\n@@ -2025,8 +2021,8 @@ static void wt_porcelain_v2_submodule_state(\n * Fix-up changed entries before we print them.\n */\n static void wt_porcelain_v2_fix_up_changed(\n-\tstruct string_list_item *it,\n-\tstruct wt_status *s)\n+\tconst struct string_list_item *it,\n+\tconst struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n \n@@ -2074,8 +2070,8 @@ static void wt_porcelain_v2_fix_up_changed(\n * Print porcelain v2 info for tracked entries with changes.\n */\n static void wt_porcelain_v2_print_changed_entry(\n-\tstruct string_list_item *it,\n-\tstruct wt_status *s)\n+\tconst struct string_list_item *it,\n+\tconst struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -2138,8 +2134,8 @@ static void wt_porcelain_v2_print_changed_entry(\n * Print porcelain v2 status info for unmerged entries.\n */\n static void wt_porcelain_v2_print_unmerged_entry(\n-\tstruct string_list_item *it,\n-\tstruct wt_status *s)\n+\tconst struct string_list_item *it,\n+\tconst struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n \tconst struct cache_entry *ce;\n@@ -2219,8 +2215,8 @@ static void wt_porcelain_v2_print_unmerged_entry(\n * Print porcelain V2 status info for untracked and ignored entries.\n */\n static void wt_porcelain_v2_print_other(\n-\tstruct string_list_item *it,\n-\tstruct wt_status *s,\n+\tconst struct string_list_item *it,\n+\tconst struct wt_status *s,\n \tchar prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -2250,14 +2246,14 @@ static void wt_porcelain_v2_print_other(\n * []*\n *\n */\n-static void wt_porcelain_v2_print(struct wt_status *s)\n+static void wt_porcelain_v2_print(const struct wt_status *s, const struct wt_status_state *state)\n {\n \tstruct wt_status_change_data *d;\n \tstruct string_list_item *it;\n \tint i;\n \n \tif (s->show_branch)\n-\t\twt_porcelain_v2_print_tracking(s);\n+\t\twt_porcelain_v2_print_tracking(s, state);\n \n \tfor (i = 0; i < s->change.nr; i++) {\n \t\tit = &(s->change.items[i]);\n@@ -2284,7 +2280,9 @@ static void wt_porcelain_v2_print(struct wt_status *s)\n \t}\n }\n \n-void wt_status_print(struct wt_status *s)\n+// FIXME: `struct wt_status *` should be `const struct wt_status` but because\n+// `wt_porcelain_print()` modifies it, that has to first be fixed\n+void wt_status_print(struct wt_status *s, const struct wt_status_state *state)\n {\n \tswitch (s->status_format) {\n \tcase STATUS_FORMAT_SHORT:\n@@ -2294,14 +2292,14 @@ void wt_status_print(struct wt_status *s)\n \t\twt_porcelain_print(s);\n \t\tbreak;\n \tcase STATUS_FORMAT_PORCELAIN_V2:\n-\t\twt_porcelain_v2_print(s);\n+\t\twt_porcelain_v2_print(s, state);\n \t\tbreak;\n \tcase STATUS_FORMAT_UNSPECIFIED:\n \t\tBUG(\"finalize_deferred_config() should have been called\");\n \t\tbreak;\n \tcase STATUS_FORMAT_NONE:\n \tcase STATUS_FORMAT_LONG:\n-\t\twt_longstatus_print(s);\n+\t\twt_longstatus_print(s, state);\n \t\tbreak;\n \t}\n }\ndiff --git a/wt-status.h b/wt-status.h\nindex 937b2c352..341bda9dc 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -128,18 +128,19 @@ struct wt_status_state {\n size_t wt_status_locate_end(const char *s, size_t len);\n void wt_status_add_cut_line(FILE *fp);\n void wt_status_prepare(struct wt_status *s);\n-void wt_status_print(struct wt_status *s);\n-void wt_status_collect(struct wt_status *s);\n-void wt_status_get_state(struct wt_status_state *state, int get_detached_from);\n+void wt_status_print(struct wt_status *s, const struct wt_status_state *state);\n+void wt_status_collect(struct wt_status *s, const struct wt_status_state *state);\n+void wt_status_get_state(const struct wt_status *s, struct wt_status_state *state);\n+void wt_status_clear_state(struct wt_status_state *state);\n int wt_status_check_rebase(const struct worktree *wt,\n \t\t\t struct wt_status_state *state);\n int wt_status_check_bisect(const struct worktree *wt,\n \t\t\t struct wt_status_state *state);\n \n __attribute__((format (printf, 3, 4)))\n-void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...);\n+void status_printf_ln(const struct wt_status *s, const char *color, const char *fmt, ...);\n __attribute__((format (printf, 3, 4)))\n-void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);\n+void status_printf(const struct wt_status *s, const char *color, const char *fmt, ...);\n \n /* The following functions expect that the caller took care of reading the index. */\n int has_unstaged_changes(int ignore_submodules);\n-- \n2.18.0\n\n"},{"id":"353970","messageId":"xmqqo9eoe5f3.fsf@gitster-ct.c.googlers.com","threadId":"48321","inReplyTo":"20180723020903.22435-1-sxlijin@gmail.com","subject":"Re: [PATCH v4 0/4] Rerolling patch series to fix t7501","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-30T22:15:44Z","receivedAt":"2018-07-30T22:15:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n> Following up on Junio's review from last time.\n>\n> Samuel Lijin (4):\n> t7501: add coverage for flags which imply dry runs\n> wt-status: rename commitable to committable\n> wt-status: teach wt_status_collect about merges in progress\n> commit: fix exit code when doing a dry run\n>\n> builtin/commit.c | 32 +++---\n> ref-filter.c | 3 +-\n> t/t7501-commit.sh | 150 ++++++++++++++++++++++++---\n> wt-status.c | 258 ++++++++++++++++++++++++----------------------\n> wt-status.h | 13 +--\n> 5 files changed, 298 insertions(+), 158 deletions(-)\n\n\nIt seems that t7512 & t7060 break with this topic queued.\n"}]}