{"thread":{"id":"49289","subject":"[PATCH v3 0/4] wt-status.c: commitable flag","startedAt":"2018-09-06T00:53:32Z","lastAt":"2018-09-30T14:12:53Z","messageCount":25,"participants":["Stephen P. Smith","Ævar Arnfjörð Bjarmason","Stephen & Linda Smith","Junio C Hamano","Stephen Smith","Taylor Blau","Eric Sunshine"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"357499","messageId":"20180906005329.11277-1-ischis2@cox.net","threadId":"49289","inReplyTo":null,"subject":"[PATCH v3 0/4] wt-status.c: commitable flag","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-06T00:53:25Z","receivedAt":"2018-09-06T00:53:32Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"A couple of years ago, during a patch review Junio found that the\ncommitable bit as implemented in wt-status.c was broken.\n\nStephen P. Smith (4):\n Move has_unmerged earlier in the file.\n wt-status: rename commitable to committable\n t7501: add test of \"commit --dry-run --short\"\n wt-status.c: Set the committable flag in the collect phase.\n\n builtin/commit.c | 18 +++++++++---------\n t/t7501-commit.sh | 10 ++++++++--\n wt-status.c | 45 +++++++++++++++++++++++++++------------------\n wt-status.h | 2 +-\n 4 files changed, 45 insertions(+), 30 deletions(-)\n\n-- \n2.18.0\n\n"},{"id":"357500","messageId":"20180906005329.11277-2-ischis2@cox.net","threadId":"49289","inReplyTo":"20180906005329.11277-1-ischis2@cox.net","subject":"[PATCH v3 1/4] Move has_unmerged earlier in the file.","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-06T00:53:26Z","receivedAt":"2018-09-06T00:53:33Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"Move has_unmerged up in the file so that has_unmerged can be called in\nwt_status_collect where we need to place a merge check.\n\nSigned-off-by: Stephen P. Smith \n---\n wt-status.c | 26 +++++++++++++-------------\n 1 file changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 5ffab6101..180faf6ba 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -724,6 +724,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 int has_unmerged(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 void wt_status_collect(struct wt_status *s)\n {\n \twt_status_collect_changes_worktree(s);\n@@ -1063,19 +1076,6 @@ 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-{\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\tstruct wt_status_state *state,\n \t\t\t\tconst char *color)\n-- \n2.18.0\n\n"},{"id":"357501","messageId":"20180906005329.11277-4-ischis2@cox.net","threadId":"49289","inReplyTo":"20180906005329.11277-1-ischis2@cox.net","subject":"[PATCH v3 3/4] t7501: add test of \"commit --dry-run --short\"","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-06T00:53:28Z","receivedAt":"2018-09-06T00:53:33Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"Add test for commit with --dry-run --short for a new file of zero\nlength.\n\nThe test demonstrates that the setting of the committable flag is\nbroken.\n\nSigned-off-by: Stephen P. Smith \n---\n t/t7501-commit.sh | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex 4cae92804..cf2a4c539 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -682,4 +682,10 @@ test_expect_success '--dry-run with conflicts fixed from a merge' '\n \tgit commit -m \"conflicts fixed from merge.\"\n '\n \n+test_expect_failure '--dry-run --short' '\n+\t>test-file &&\n+\tgit add test-file &&\n+\tgit commit --dry-run --short\n+'\n+\n test_done\n-- \n2.18.0\n\n"},{"id":"357502","messageId":"20180906005329.11277-3-ischis2@cox.net","threadId":"49289","inReplyTo":"20180906005329.11277-1-ischis2@cox.net","subject":"[PATCH v3 2/4] wt-status: rename commitable to committable","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-06T00:53:27Z","receivedAt":"2018-09-06T00:53:34Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"Fix variable spelling error.\n\nSigned-off-by: Stephen P. Smith \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 0d9828e29..51ecebbec 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 180faf6ba..4962b5bc8 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -786,7 +786,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@@ -1021,7 +1021,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":"357503","messageId":"20180906005329.11277-5-ischis2@cox.net","threadId":"49289","inReplyTo":"20180906005329.11277-1-ischis2@cox.net","subject":"[PATCH v3 4/4] wt-status.c: Set the committable flag in the collect phase.","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-06T00:53:29Z","receivedAt":"2018-09-06T00:53:40Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"In an update to fix a bug with \"commit --dry-run\" it was found that\nthe committable flag was broken. The update was, at the time, accepted\nas it was better than the previous version. [1]\n\nSince the setting of the committable flag had been done in\nwt_longstatus_print_updated, move it to wt_status_collect_updated_cb.\n\nSet the committable flag in wt_status_collect_changes_initial to keep\nfrom introducing a rebase regression.\n\nInstead of setting the committable flag in show_merge_in_progress, in\nwt_status_cllect check for a merge that has not been committed. If\npresent then set the committable flag.\n\nChange the tests to expect success since updates to the wt-status\nbroken code section is being fixed.\n\n[1] https://public-inbox.org/git/xmqqr3gcj9i5.fsf@gitster.mtv.corp.google.com/\n\nSigned-off-by: Stephen P. Smith \n---\n t/t7501-commit.sh | 6 +++---\n wt-status.c | 13 +++++++++++--\n 2 files changed, 14 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex cf2a4c539..e18c0b4a6 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@@ -682,7 +682,7 @@ test_expect_success '--dry-run with conflicts fixed from a merge' '\n \tgit commit -m \"conflicts fixed from merge.\"\n '\n \n-test_expect_failure '--dry-run --short' '\n+test_expect_success '--dry-run --short' '\n \t>test-file &&\n \tgit add test-file &&\n \tgit commit --dry-run --short\ndiff --git a/wt-status.c b/wt-status.c\nindex 4962b5bc8..c7f76d475 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -540,10 +540,12 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n \t\t\t/* Leave {mode,oid}_head zero for an add. */\n \t\t\td->mode_index = p->two->mode;\n \t\t\toidcpy(&d->oid_index, &p->two->oid);\n+\t\t\ts->committable = 1;\n \t\t\tbreak;\n \t\tcase DIFF_STATUS_DELETED:\n \t\t\td->mode_head = p->one->mode;\n \t\t\toidcpy(&d->oid_head, &p->one->oid);\n+\t\t\ts->committable = 1;\n \t\t\t/* Leave {mode,oid}_index zero for a delete. */\n \t\t\tbreak;\n \n@@ -561,6 +563,7 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n \t\t\td->mode_index = p->two->mode;\n \t\t\toidcpy(&d->oid_head, &p->one->oid);\n \t\t\toidcpy(&d->oid_index, &p->two->oid);\n+\t\t\ts->committable = 1;\n \t\t\tbreak;\n \t\tcase DIFF_STATUS_UNMERGED:\n \t\t\td->stagemask = unmerged_mask(p->two->path);\n@@ -665,11 +668,13 @@ static void wt_status_collect_changes_initial(struct wt_status *s)\n \t\t\t * code will output the stage values directly and not use the\n \t\t\t * values in these fields.\n \t\t\t */\n+\t\t\ts->committable = 1;\n \t\t} else {\n \t\t\td->index_status = DIFF_STATUS_ADDED;\n \t\t\t/* Leave {mode,oid}_head zero for adds. */\n \t\t\td->mode_index = ce->ce_mode;\n \t\t\toidcpy(&d->oid_index, &ce->oid);\n+\t\t\ts->committable = 1;\n \t\t}\n \t}\n }\n@@ -739,6 +744,7 @@ static int has_unmerged(struct wt_status *s)\n \n void wt_status_collect(struct wt_status *s)\n {\n+\tstruct wt_status_state state;\n \twt_status_collect_changes_worktree(s);\n \n \tif (s->is_initial)\n@@ -746,6 +752,11 @@ void wt_status_collect(struct wt_status *s)\n \telse\n \t\twt_status_collect_changes_index(s);\n \twt_status_collect_untracked(s);\n+\n+\tmemset(&state, 0, sizeof(state));\n+\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n+\tif (state.merge_in_progress && !has_unmerged(s))\n+\t\ts->committable = 1;\n }\n \n static void wt_longstatus_print_unmerged(struct wt_status *s)\n@@ -786,7 +797,6 @@ 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->committable = 1;\n \t\t\tshown_header = 1;\n \t\t}\n \t\twt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);\n@@ -1089,7 +1099,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-- \n2.18.0\n\n"},{"id":"357517","messageId":"8736unrs6o.fsf@evledraar.gmail.com","threadId":"49289","inReplyTo":"20180906005329.11277-1-ischis2@cox.net","subject":"Re: [PATCH v3 0/4] wt-status.c: commitable flag","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-09-06T07:38:55Z","receivedAt":"2018-09-06T07:39:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Sep 06 2018, Stephen P. Smith wrote:\n\nThis all looks good to me this time around.\n\n> Stephen P. Smith (4):\n> Move has_unmerged earlier in the file.\n> wt-status: rename commitable to committable\n> t7501: add test of \"commit --dry-run --short\"\n> wt-status.c: Set the committable flag in the collect phase.\n\nSometimes you send mail from this address as \"Stephen & Linda Smith\n\", do we also need Linda Smith's Signed-Off-By? :)\n"},{"id":"357544","messageId":"28999379.B3KWMFSn5l@thunderbird","threadId":"49289","inReplyTo":"20180906005329.11277-1-ischis2@cox.net","subject":"Re: [PATCH v3 0/4] wt-status.c: commitable flag","fromName":"Stephen & Linda Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-06T16:06:55Z","receivedAt":"2018-09-06T16:06:58Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"On Thursday, September 6, 2018 12:38:55 AM MST Ævar Arnfjörð Bjarmason wrote:\n> On Thu, Sep 06 2018, Stephen P. Smith wrote:\n> Sometimes you send mail from this address as \"Stephen & Linda Smith\n> \", do we also need Linda Smith's Signed-Off-By? :)\n\nMy wife and I share one email account. If I use the GUI email client to \nrespond to emails then her name is on the From line (I could edit future \nemails).\n\nWhen I am submitting patches/updates, I do so under my name since it is \n\"Stephen P. Smith\" that is creating the patch. \n\nI've never had anyone ask before.\n\n\n\n\n"},{"id":"357657","messageId":"xmqq7ejxvvhv.fsf@gitster-ct.c.googlers.com","threadId":"49289","inReplyTo":"20180906005329.11277-3-ischis2@cox.net","subject":"Re: [PATCH v3 2/4] wt-status: rename commitable to committable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-07T21:38:20Z","receivedAt":"2018-09-07T21:38:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Stephen P. Smith\" writes:\n\n> Fix variable spelling error.\n>\n> Signed-off-by: Stephen P. Smith \n> ---\n\nThanks ;-)\n"},{"id":"357658","messageId":"xmqq36ulvve3.fsf@gitster-ct.c.googlers.com","threadId":"49289","inReplyTo":"20180906005329.11277-1-ischis2@cox.net","subject":"Re: [PATCH v3 0/4] wt-status.c: commitable flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-07T21:40:36Z","receivedAt":"2018-09-07T21:40:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Stephen P. Smith\" writes:\n\n> A couple of years ago, during a patch review Junio found that the\n> commitable bit as implemented in wt-status.c was broken.\n>\n> Stephen P. Smith (4):\n> Move has_unmerged earlier in the file.\n> wt-status: rename commitable to committable\n> t7501: add test of \"commit --dry-run --short\"\n> wt-status.c: Set the committable flag in the collect phase.\n>\n> builtin/commit.c | 18 +++++++++---------\n> t/t7501-commit.sh | 10 ++++++++--\n> wt-status.c | 45 +++++++++++++++++++++++++++------------------\n> wt-status.h | 2 +-\n> 4 files changed, 45 insertions(+), 30 deletions(-)\n\nThanks.\n"},{"id":"357659","messageId":"xmqqworxufuv.fsf@gitster-ct.c.googlers.com","threadId":"49289","inReplyTo":"20180906005329.11277-5-ischis2@cox.net","subject":"Re: [PATCH v3 4/4] wt-status.c: Set the committable flag in the collect phase.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-07T22:01:28Z","receivedAt":"2018-09-07T22:01:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Stephen P. Smith\" writes:\n\n> void wt_status_collect(struct wt_status *s)\n> {\n> +\tstruct wt_status_state state;\n> \twt_status_collect_changes_worktree(s);\n> \n> \tif (s->is_initial)\n> @@ -746,6 +752,11 @@ void wt_status_collect(struct wt_status *s)\n> \telse\n> \t\twt_status_collect_changes_index(s);\n> \twt_status_collect_untracked(s);\n> +\n> +\tmemset(&state, 0, sizeof(state));\n> +\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n> +\tif (state.merge_in_progress && !has_unmerged(s))\n> +\t\ts->committable = 1;\n> }\n\nI do not think this is wrong per-se, but now we have three calls to\nwt_status_get_state() in wt-status.c, and it smells (at least to me)\nthat each of these callsites does so only because their callers\ndo not give them enough information, forcing them to find the state\nout for themselves.\n\nGiven that the ideal paradigm to come up with the \"work tree status\"\nis to do the collection followed by printing, and among three\ncallers of get_state(), two appear in the \"printing\" side of the\ncallchain [*1*], I wonder if it makes a better organization to\n\n - embed struct wt_status_state in struct wt_status\n\n - make the new call to wt_status_get_state() added above in this\n patch to populate the wt_status_state embedded in 's'\n\n - change the other two callers of wt_status_get_state() in\n wt_longstatus_print() and wt_porcelain_v2_print_tracking(), both\n of which will receive 's' that has been populated by a previous\n call to wt_status_collect(), so that they do *not* call\n get_state() themselves, but instead use the result recorded in\n wt_status_state embedded in 's', which was populated by\n wt_status_collect() before they got called.\n\nThat would bring the resulting code even closer to the ideal,\ni.e. the \"collect\" phase learns _everything_ we need about the\ncurrent state that is necessary in order to later show to the user,\nand the \"print\" phase does not do its own separate discovery.\n\nWhat do you think?\n\nThanks.\n\n\n[Reference]\n\n*1* Here are the selected functions and what other functions they\n call.\n\n wt_status_collect()\n\n -> wt_status_collect_changes_initial()\n -> wt_status_collect_changes_index()\n -> wt_status_collect_untracked()\n -> wt_status_get_state()\n\n wt_longstatus_print()\n\n -> wt_status_get_state()\n\n wt_porcelain_v2_print_tracking()\n\n -> wt_status_get_state()\n\n\n wt_status_print()\n\n -> wt_porcelain_v2_print()\n -> wt_porcelain_v2_print_tracking()\n -> wt_longstatus_print()\n\n\n run_status()\n\n -> wt_status_collect()\n -> wt_status_print()\n\n cmd_status()\n\n -> wt_status_collect()\n -> wt_status_print()\n\n\n prepare_to_commit(), dry_run_commit()\n\n -> run_status()\n\n\n Most notably, wt_status_collect() always happens before\n wt_status_print(), which is natural because the former is to\n collect information in 's' that is used by the latter to print.\n\n And in various functions wt_status_print() calls indirectly, the\n two other callers of wt_status_get_state() appear.\n"},{"id":"357660","messageId":"xmqqr2i5ueg4.fsf@gitster-ct.c.googlers.com","threadId":"49289","inReplyTo":"xmqqworxufuv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 4/4] wt-status.c: Set the committable flag in the collect phase.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-07T22:31:55Z","receivedAt":"2018-09-07T22:32:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"Stephen P. Smith\" writes:\n>\n>> void wt_status_collect(struct wt_status *s)\n>> {\n>> +\tstruct wt_status_state state;\n>> \twt_status_collect_changes_worktree(s);\n>> \n>> \tif (s->is_initial)\n>> @@ -746,6 +752,11 @@ void wt_status_collect(struct wt_status *s)\n>> \telse\n>> \t\twt_status_collect_changes_index(s);\n>> \twt_status_collect_untracked(s);\n>> +\n>> +\tmemset(&state, 0, sizeof(state));\n>> +\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n>> +\tif (state.merge_in_progress && !has_unmerged(s))\n>> +\t\ts->committable = 1;\n>> }\n>\n> I do not think this is wrong per-se, but now we have three calls to\n> wt_status_get_state() in wt-status.c, and it smells (at least to me)\n> that each of these callsites does so only because their callers\n> do not give them enough information, forcing them to find the state\n> out for themselves.\n>\n> Given that the ideal paradigm to come up with the \"work tree status\"\n> is to do the collection followed by printing, and among three\n> callers of get_state(), two appear in the \"printing\" side of the\n> callchain [*1*], I wonder if it makes a better organization to\n>\n> - embed struct wt_status_state in struct wt_status\n>\n> - make the new call to wt_status_get_state() added above in this\n> patch to populate the wt_status_state embedded in 's'\n>\n> - change the other two callers of wt_status_get_state() in\n> wt_longstatus_print() and wt_porcelain_v2_print_tracking(), both\n> of which will receive 's' that has been populated by a previous\n> call to wt_status_collect(), so that they do *not* call\n> get_state() themselves, but instead use the result recorded in\n> wt_status_state embedded in 's', which was populated by\n> wt_status_collect() before they got called.\n>\n> That would bring the resulting code even closer to the ideal,\n> i.e. the \"collect\" phase learns _everything_ we need about the\n> current state that is necessary in order to later show to the user,\n> and the \"print\" phase does not do its own separate discovery.\n>\n> What do you think?\n>\n> Thanks.\n\nSuch a \"clean-up\" may look like this patch:\n\n - add .state field to wt_status to embed a wt_status_state instance\n\n - remove a parameter of type struct wt_status_state from all\n functions where wt_status is already passed; we'd use .state\n field of the wt_status instead\n\nThe patch is mostly for illustration of the idea.\n\nThe result seems to compile and pass the test suite, but I haven't\ncarefully thought about what else I may be breaking with this\nmechanical change. For example, I noticed that both of the old\ncallsites of wt_status_get_state() have free() of a few fiedls in\nthe structure, and I kept the code as close to the original, but I\nsuspect they should not be freed there in the functions in the\n\"print\" phase, but rather the caller of the \"collect\" and \"print\"\nshould be made responsible for deciding when to dispose the entire\nwt_status (and wt_status_state as part of it). This illustration\npatch does not address that kind of details (yet).\n\n\n wt-status.c | 132 ++++++++++++++++++++++++++----------------------------------\n wt-status.h | 37 ++++++++---------\n 2 files changed, 77 insertions(+), 92 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex c7f76d4758..69f2cbdca9 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -744,18 +744,15 @@ static int has_unmerged(struct wt_status *s)\n \n void wt_status_collect(struct wt_status *s)\n {\n-\tstruct wt_status_state state;\n \twt_status_collect_changes_worktree(s);\n-\n \tif (s->is_initial)\n \t\twt_status_collect_changes_initial(s);\n \telse\n \t\twt_status_collect_changes_index(s);\n \twt_status_collect_untracked(s);\n \n-\tmemset(&state, 0, sizeof(state));\n-\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n-\tif (state.merge_in_progress && !has_unmerged(s))\n+\twt_status_get_state(&s->state, s->branch && !strcmp(s->branch, \"HEAD\"));\n+\tif (s->state.merge_in_progress && !has_unmerged(s))\n \t\ts->committable = 1;\n }\n \n@@ -1087,8 +1084,7 @@ static void wt_longstatus_print_tracking(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 char *color)\n+\t\t\t\t const char *color)\n {\n \tif (has_unmerged(s)) {\n \t\tstatus_printf_ln(s, color, _(\"You have unmerged paths.\"));\n@@ -1109,16 +1105,15 @@ static void show_merge_in_progress(struct wt_status *s,\n }\n \n static void show_am_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tstatus_printf_ln(s, color,\n \t\t_(\"You are in the middle of an am session.\"));\n-\tif (state->am_empty_patch)\n+\tif (s->state.am_empty_patch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"The current patch is empty.\"));\n \tif (s->hints) {\n-\t\tif (!state->am_empty_patch)\n+\t\tif (!s->state.am_empty_patch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (fix conflicts and then run \\\"git am --continue\\\")\"));\n \t\tstatus_printf_ln(s, color,\n@@ -1242,10 +1237,9 @@ static int read_rebase_todolist(const char *fname, struct string_list *lines)\n }\n \n static void show_rebase_information(struct wt_status *s,\n-\t\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n-\tif (state->rebase_interactive_in_progress) {\n+\tif (s->state.rebase_interactive_in_progress) {\n \t\tint i;\n \t\tint nr_lines_to_show = 2;\n \n@@ -1296,28 +1290,26 @@ static void show_rebase_information(struct wt_status *s,\n }\n \n static void print_rebase_state(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\tconst char *color)\n+\t\t\t const char *color)\n {\n-\tif (state->branch)\n+\tif (s->state.branch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t state->branch,\n-\t\t\t\t state->onto);\n+\t\t\t\t s->state.branch,\n+\t\t\t\t s->state.onto);\n \telse\n \t\tstatus_printf_ln(s, color,\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-\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n \tstruct stat st;\n \n-\tshow_rebase_information(s, state, color);\n+\tshow_rebase_information(s, color);\n \tif (has_unmerged(s)) {\n-\t\tprint_rebase_state(s, state, color);\n+\t\tprint_rebase_state(s, color);\n \t\tif (s->hints) {\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (fix conflicts and then run \\\"git rebase --continue\\\")\"));\n@@ -1326,17 +1318,18 @@ static void show_rebase_in_progress(struct wt_status *s,\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (use \\\"git rebase --abort\\\" to check out the original branch)\"));\n \t\t}\n-\t} else if (state->rebase_in_progress || !stat(git_path_merge_msg(the_repository), &st)) {\n-\t\tprint_rebase_state(s, state, color);\n+\t} else if (s->state.rebase_in_progress ||\n+\t\t !stat(git_path_merge_msg(the_repository), &st)) {\n+\t\tprint_rebase_state(s, color);\n \t\tif (s->hints)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (all conflicts fixed: run \\\"git rebase --continue\\\")\"));\n \t} else if (split_commit_in_progress(s)) {\n-\t\tif (state->branch)\n+\t\tif (s->state.branch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently splitting a commit while rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t\t state->branch,\n-\t\t\t\t\t state->onto);\n+\t\t\t\t\t s->state.branch,\n+\t\t\t\t\t s->state.onto);\n \t\telse\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently splitting a commit during a rebase.\"));\n@@ -1344,11 +1337,11 @@ static void show_rebase_in_progress(struct wt_status *s,\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (Once your working directory is clean, run \\\"git rebase --continue\\\")\"));\n \t} else {\n-\t\tif (state->branch)\n+\t\tif (s->state.branch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently editing a commit while rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t\t state->branch,\n-\t\t\t\t\t state->onto);\n+\t\t\t\t\t s->state.branch,\n+\t\t\t\t\t s->state.onto);\n \t\telse\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently editing a commit during a rebase.\"));\n@@ -1363,11 +1356,10 @@ static void show_rebase_in_progress(struct wt_status *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-\t\t\t\t\tconst char *color)\n+\t\t\t\t\t const char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently cherry-picking commit %s.\"),\n-\t\t\tfind_unique_abbrev(&state->cherry_pick_head_oid, DEFAULT_ABBREV));\n+\t\t\tfind_unique_abbrev(&s->state.cherry_pick_head_oid, DEFAULT_ABBREV));\n \tif (s->hints) {\n \t\tif (has_unmerged(s))\n \t\t\tstatus_printf_ln(s, color,\n@@ -1382,11 +1374,10 @@ static void show_cherry_pick_in_progress(struct wt_status *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-\t\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently reverting commit %s.\"),\n-\t\t\t find_unique_abbrev(&state->revert_head_oid, DEFAULT_ABBREV));\n+\t\t\t find_unique_abbrev(&s->state.revert_head_oid, DEFAULT_ABBREV));\n \tif (s->hints) {\n \t\tif (has_unmerged(s))\n \t\t\tstatus_printf_ln(s, color,\n@@ -1401,13 +1392,12 @@ static void show_revert_in_progress(struct wt_status *s,\n }\n \n static void show_bisect_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n-\tif (state->branch)\n+\tif (s->state.branch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently bisecting, started from branch '%s'.\"),\n-\t\t\t\t state->branch);\n+\t\t\t\t s->state.branch);\n \telse\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently bisecting.\"));\n@@ -1581,48 +1571,45 @@ void wt_status_get_state(struct wt_status_state *state,\n \t\twt_status_get_detached_from(state);\n }\n \n-static void wt_longstatus_print_state(struct wt_status *s,\n-\t\t\t\t struct wt_status_state *state)\n+static void wt_longstatus_print_state(struct wt_status *s)\n {\n \tconst char *state_color = color(WT_STATUS_HEADER, s);\n+\tstruct wt_status_state *state = &s->state;\n+\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+\t\tshow_am_in_progress(s, state_color);\n \telse if (state->rebase_in_progress || state->rebase_interactive_in_progress)\n-\t\tshow_rebase_in_progress(s, state, state_color);\n+\t\tshow_rebase_in_progress(s, state_color);\n \telse if (state->cherry_pick_in_progress)\n-\t\tshow_cherry_pick_in_progress(s, state, state_color);\n+\t\tshow_cherry_pick_in_progress(s, state_color);\n \telse if (state->revert_in_progress)\n-\t\tshow_revert_in_progress(s, state, state_color);\n+\t\tshow_revert_in_progress(s, state_color);\n \tif (state->bisect_in_progress)\n-\t\tshow_bisect_in_progress(s, state, state_color);\n+\t\tshow_bisect_in_progress(s, state_color);\n }\n \n static void wt_longstatus_print(struct wt_status *s)\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 (s->state.rebase_in_progress ||\n+\t\t\t s->state.rebase_interactive_in_progress) {\n+\t\t\t\tif (s->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 = s->state.onto;\n+\t\t\t} else if (s->state.detached_from) {\n+\t\t\t\tbranch_name = s->state.detached_from;\n+\t\t\t\tif (s->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@@ -1639,10 +1626,10 @@ 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);\n+\tfree(s->state.branch);\n+\tfree(s->state.onto);\n+\tfree(s->state.detached_from);\n \n \tif (s->is_initial) {\n \t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n@@ -1946,13 +1933,9 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\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@@ -1963,10 +1946,11 @@ 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 (s->state.rebase_in_progress ||\n+\t\t\t s->state.rebase_interactive_in_progress)\n+\t\t\t\tbranch_name = s->state.onto;\n+\t\t\telse if (s->state.detached_from)\n+\t\t\t\tbranch_name = s->state.detached_from;\n \t\t\telse\n \t\t\t\tbranch_name = \"\";\n \t\t} else {\n@@ -2001,9 +1985,9 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n \t\t}\n \t}\n \n-\tfree(state.branch);\n-\tfree(state.onto);\n-\tfree(state.detached_from);\n+\tfree(s->state.branch);\n+\tfree(s->state.onto);\n+\tfree(s->state.detached_from);\n }\n \n /*\ndiff --git a/wt-status.h b/wt-status.h\nindex 937b2c3521..f9115c59ae 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -64,6 +64,24 @@ enum wt_status_format {\n \tSTATUS_FORMAT_UNSPECIFIED\n };\n \n+struct wt_status_state {\n+\tint merge_in_progress;\n+\tint am_in_progress;\n+\tint am_empty_patch;\n+\tint rebase_in_progress;\n+\tint rebase_interactive_in_progress;\n+\tint cherry_pick_in_progress;\n+\tint bisect_in_progress;\n+\tint revert_in_progress;\n+\tint detached_at;\n+\tchar *branch;\n+\tchar *onto;\n+\tchar *detached_from;\n+\tstruct object_id detached_oid;\n+\tstruct object_id revert_head_oid;\n+\tstruct object_id cherry_pick_head_oid;\n+};\n+\n struct wt_status {\n \tint is_initial;\n \tchar *branch;\n@@ -93,6 +111,7 @@ struct wt_status {\n \tint rename_score;\n \tint rename_limit;\n \tenum wt_status_format status_format;\n+\tstruct wt_status_state state;\n \tunsigned char sha1_commit[GIT_MAX_RAWSZ]; /* when not Initial */\n \n \t/* These are computed during processing of the individual sections */\n@@ -107,24 +126,6 @@ struct wt_status {\n \tuint32_t untracked_in_ms;\n };\n \n-struct wt_status_state {\n-\tint merge_in_progress;\n-\tint am_in_progress;\n-\tint am_empty_patch;\n-\tint rebase_in_progress;\n-\tint rebase_interactive_in_progress;\n-\tint cherry_pick_in_progress;\n-\tint bisect_in_progress;\n-\tint revert_in_progress;\n-\tint detached_at;\n-\tchar *branch;\n-\tchar *onto;\n-\tchar *detached_from;\n-\tstruct object_id detached_oid;\n-\tstruct object_id revert_head_oid;\n-\tstruct object_id cherry_pick_head_oid;\n-};\n-\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\n"},{"id":"357670","messageId":"1827990.xjSgEIESZI@thunderbird","threadId":"49289","inReplyTo":"xmqqworxufuv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 4/4] wt-status.c: Set the committable flag in the collect phase.","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-07T23:55:41Z","receivedAt":"2018-09-07T23:55:45Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"On Friday, September 7, 2018 3:31:55 PM MST you wrote:\n> Junio C Hamano writes:\n\n> The patch is mostly for illustration of the idea.\n> \n> The result seems to compile and pass the test suite, but I haven't\n> carefully thought about what else I may be breaking with this\n> mechanical change. For example, I noticed that both of the old\n> callsites of wt_status_get_state() have free() of a few fiedls in\n> the structure, and I kept the code as close to the original, but I\n> suspect they should not be freed there in the functions in the\n> \"print\" phase, but rather the caller of the \"collect\" and \"print\"\n> should be made responsible for deciding when to dispose the entire\n> wt_status (and wt_status_state as part of it). This illustration\n> patch does not address that kind of details (yet).\n\nIf we use this as a basis of a follow on patch, how do I handle credit. You \nobviously wrote this patch and I did not.\n\nSo how is the mechanics of that normally done? Thanks for the patch I will \nwork with it.\n\nsps\n\n\n"},{"id":"357880","messageId":"xmqqworsosod.fsf@gitster-ct.c.googlers.com","threadId":"49289","inReplyTo":"1827990.xjSgEIESZI@thunderbird","subject":"Re: [PATCH v3 4/4] wt-status.c: Set the committable flag in the collect phase.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-11T17:22:26Z","receivedAt":"2018-09-11T17:22:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Stephen P. Smith\" writes:\n\n> On Friday, September 7, 2018 3:31:55 PM MST you wrote:\n>> Junio C Hamano writes:\n>\n>> The patch is mostly for illustration of the idea.\n>> \n>> The result seems to compile and pass the test suite, but I haven't\n>> carefully thought about what else I may be breaking with this\n>> mechanical change. For example, I noticed that both of the old\n>> callsites of wt_status_get_state() have free() of a few fiedls in\n>> the structure, and I kept the code as close to the original, but I\n>> suspect they should not be freed there in the functions in the\n>> \"print\" phase, but rather the caller of the \"collect\" and \"print\"\n>> should be made responsible for deciding when to dispose the entire\n>> wt_status (and wt_status_state as part of it). This illustration\n>> patch does not address that kind of details (yet).\n>\n> If we use this as a basis of a follow on patch, how do I handle credit. You \n> obviously wrote this patch and I did not.\n\nOften people just mention \"This was based on an earlier work by ...\"\nat/near the end of the log message. When the result ends up to be\nvery different from the earlier work, just adding \"Helped-by: ...\"\nbefore your sign-off is often sufficient.\n\n"},{"id":"358744","messageId":"2295579.U4Xb9QnJqG@thunderbird","threadId":"49289","inReplyTo":"xmqqworxufuv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 4/4] wt-status.c: Set the committable flag in the collect phase.","fromName":"Stephen Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-24T03:15:16Z","receivedAt":"2018-09-24T03:15:23Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"On Friday, September 7, 2018 3:31:55 PM MST Junio C Hamano wrote:\n> For example, I noticed that both of the old\n> callsites of wt_status_get_state() have free() of a few fiedls in\n> the structure, and I kept the code as close to the original, but I\n> suspect they should not be freed there in the functions in the\n> \"print\" phase, but rather the caller of the \"collect\" and \"print\"\n> should be made responsible for deciding when to dispose the entire\n> wt_status (and wt_status_state as part of it). This illustration\n> patch does not address that kind of details (yet).\n\nI followed the call tree back to original callers run_status() and \ncmd_status() in commit.c\n\nThis leads to a philosophical question. We want to move the state information \nout of the print functions because it doesn't seem correct. For the case in \nquestion this includes the calls to free() . By doing this we seem go have \ntraded one location that shouldn't be touching the state variables for \nanother. \n\nI can see three solutions and could support any of the three:\n1) Move the free calls to run_status() and cmd_status().\n2) Move the calls calls to wt_status_print since that is the last function \nfrom wt_status.c that is called befor the structure goes out of scope in \nrun_status() and cmd_status().\n3) Add a new wt_collect*() function to free the variables. This would have an \nadvantage that the free calls could be grouped in on place and not done in to \nfunctions. A second advantage is that the free calls would be located where \nthe pointers are initialized. \n\nPersonally I like solutions 1 and 3 over 2.\nWhat do others think?\n\nsps\n\n\n\n\n\n\n\n"},{"id":"358787","messageId":"xmqqzhw6r4m1.fsf@gitster-ct.c.googlers.com","threadId":"49289","inReplyTo":"2295579.U4Xb9QnJqG@thunderbird","subject":"Re: [PATCH v3 4/4] wt-status.c: Set the committable flag in the collect phase.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-24T21:02:30Z","receivedAt":"2018-09-24T21:02:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephen Smith writes:\n\n> I can see three solutions and could support any of the three:\n> 1) Move the free calls to run_status() and cmd_status().\n> 2) Move the calls calls to wt_status_print since that is the last function \n> from wt_status.c that is called befor the structure goes out of scope in \n> run_status() and cmd_status().\n> 3) Add a new wt_collect*() function to free the variables. This would have an \n> advantage that the free calls could be grouped in on place and not done in to \n> functions. A second advantage is that the free calls would be located where \n> the pointers are initialized. \n>\n> Personally I like solutions 1 and 3 over 2.\n> What do others think?\n\nI think freeing at the top level caller (i.e. #1) once it finished\nusing the information collected would make the most sense---it\ninitiated the collection, then fed the collected info to shower, and\nnow it knows it is done with the pieces of memory it used to make\nthese two parts communicate with each other.\n\nAnd for keeping multiple \"pieces of memory\" as a unit, introducing a\nhelper is a good technique (i.e. #3); but I view that mostly as an\nimplementation detail of #1.\n\nThanks.\n\n"},{"id":"359184","messageId":"20180928044936.2919-1-ischis2@cox.net","threadId":"49289","inReplyTo":"xmqqr2i5ueg4.fsf@gitster-ct.c.googlers.com","subject":"[PATCH 0/1] wt-status-state-cleanup","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-28T04:49:35Z","receivedAt":"2018-09-28T04:49:39Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"Junio suggested a cleanup patch, jc/wt-status-state-cleanup, which is\nthe basis for this patch.\n\nThis patch uses ss/wt-status-committable.\n\nThe main update from the patch suggestion was cleanup of the free\ncalls for three strings in the status structure.\n\nStephen P. Smith (1):\n roll wt_status_state into wt_status and populate in the collect phase\n\n builtin/commit.c | 3 ++\n wt-status.c | 135 +++++++++++++++++++++--------------------------\n wt-status.h | 38 ++++++-------\n 3 files changed, 83 insertions(+), 93 deletions(-)\n\n-- \n2.19.0\n\n"},{"id":"359185","messageId":"20180928044936.2919-2-ischis2@cox.net","threadId":"49289","inReplyTo":"20180928044936.2919-1-ischis2@cox.net","subject":"[PATCH 1/1] roll wt_status_state into wt_status and populate in the collect phase","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-28T04:49:36Z","receivedAt":"2018-09-28T04:49:40Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"When updating the collect and print functions, it was found that\nstatus variables were initialized in the collect phase and some\nvariables were later freed in the print functions.\n\nMove the status state structure variables into the status state\nstructure and populate them in the collect functions.\n\nCreate a new funciton to free the buffers that were being freed in the\nprint function. Call this new function in commit.c where both the\ncollect and print functions were being called.\n\nBased on a patch suggestion by Junio C Hamano. [1]\n\n[1] https://public-inbox.org/git/xmqqr2i5ueg4.fsf@gitster-ct.c.googlers.com/\n\nSigned-off-by: Stephen P. Smith \n---\n builtin/commit.c | 3 ++\n wt-status.c | 135 +++++++++++++++++++++--------------------------\n wt-status.h | 38 ++++++-------\n 3 files changed, 83 insertions(+), 93 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 51ecebbec1..e168321e49 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -506,6 +506,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \n \twt_status_collect(s);\n \twt_status_print(s);\n+\twt_status_collect_free_buffers(s);\n \n \treturn s->committable;\n }\n@@ -1388,6 +1389,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \t\ts.prefix = prefix;\n \n \twt_status_print(&s);\n+\twt_status_collect_free_buffers(&s);\n+\n \treturn 0;\n }\n \ndiff --git a/wt-status.c b/wt-status.c\nindex c7f76d4758..9977f0cdf2 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -744,21 +744,26 @@ static int has_unmerged(struct wt_status *s)\n \n void wt_status_collect(struct wt_status *s)\n {\n-\tstruct wt_status_state state;\n \twt_status_collect_changes_worktree(s);\n-\n \tif (s->is_initial)\n \t\twt_status_collect_changes_initial(s);\n \telse\n \t\twt_status_collect_changes_index(s);\n \twt_status_collect_untracked(s);\n \n-\tmemset(&state, 0, sizeof(state));\n-\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n-\tif (state.merge_in_progress && !has_unmerged(s))\n+\twt_status_get_state(&s->state, s->branch && !strcmp(s->branch, \"HEAD\"));\n+\tif (s->state.merge_in_progress && !has_unmerged(s))\n \t\ts->committable = 1;\n }\n \n+void wt_status_collect_free_buffers(struct wt_status *s)\n+{\n+\tfree(s->state.branch);\n+\tfree(s->state.onto);\n+\tfree(s->state.detached_from);\n+}\n+\n+\n static void wt_longstatus_print_unmerged(struct wt_status *s)\n {\n \tint shown_header = 0;\n@@ -1087,8 +1092,7 @@ static void wt_longstatus_print_tracking(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 char *color)\n+\t\t\t\t const char *color)\n {\n \tif (has_unmerged(s)) {\n \t\tstatus_printf_ln(s, color, _(\"You have unmerged paths.\"));\n@@ -1109,16 +1113,15 @@ static void show_merge_in_progress(struct wt_status *s,\n }\n \n static void show_am_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tstatus_printf_ln(s, color,\n \t\t_(\"You are in the middle of an am session.\"));\n-\tif (state->am_empty_patch)\n+\tif (s->state.am_empty_patch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"The current patch is empty.\"));\n \tif (s->hints) {\n-\t\tif (!state->am_empty_patch)\n+\t\tif (!s->state.am_empty_patch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (fix conflicts and then run \\\"git am --continue\\\")\"));\n \t\tstatus_printf_ln(s, color,\n@@ -1242,10 +1245,9 @@ static int read_rebase_todolist(const char *fname, struct string_list *lines)\n }\n \n static void show_rebase_information(struct wt_status *s,\n-\t\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n-\tif (state->rebase_interactive_in_progress) {\n+\tif (s->state.rebase_interactive_in_progress) {\n \t\tint i;\n \t\tint nr_lines_to_show = 2;\n \n@@ -1296,28 +1298,26 @@ static void show_rebase_information(struct wt_status *s,\n }\n \n static void print_rebase_state(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\tconst char *color)\n+\t\t\t const char *color)\n {\n-\tif (state->branch)\n+\tif (s->state.branch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t state->branch,\n-\t\t\t\t state->onto);\n+\t\t\t\t s->state.branch,\n+\t\t\t\t s->state.onto);\n \telse\n \t\tstatus_printf_ln(s, color,\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-\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n \tstruct stat st;\n \n-\tshow_rebase_information(s, state, color);\n+\tshow_rebase_information(s, color);\n \tif (has_unmerged(s)) {\n-\t\tprint_rebase_state(s, state, color);\n+\t\tprint_rebase_state(s, color);\n \t\tif (s->hints) {\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (fix conflicts and then run \\\"git rebase --continue\\\")\"));\n@@ -1326,17 +1326,18 @@ static void show_rebase_in_progress(struct wt_status *s,\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (use \\\"git rebase --abort\\\" to check out the original branch)\"));\n \t\t}\n-\t} else if (state->rebase_in_progress || !stat(git_path_merge_msg(the_repository), &st)) {\n-\t\tprint_rebase_state(s, state, color);\n+\t} else if (s->state.rebase_in_progress ||\n+\t\t !stat(git_path_merge_msg(the_repository), &st)) {\n+\t\tprint_rebase_state(s, color);\n \t\tif (s->hints)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (all conflicts fixed: run \\\"git rebase --continue\\\")\"));\n \t} else if (split_commit_in_progress(s)) {\n-\t\tif (state->branch)\n+\t\tif (s->state.branch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently splitting a commit while rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t\t state->branch,\n-\t\t\t\t\t state->onto);\n+\t\t\t\t\t s->state.branch,\n+\t\t\t\t\t s->state.onto);\n \t\telse\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently splitting a commit during a rebase.\"));\n@@ -1344,11 +1345,11 @@ static void show_rebase_in_progress(struct wt_status *s,\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (Once your working directory is clean, run \\\"git rebase --continue\\\")\"));\n \t} else {\n-\t\tif (state->branch)\n+\t\tif (s->state.branch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently editing a commit while rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t\t state->branch,\n-\t\t\t\t\t state->onto);\n+\t\t\t\t\t s->state.branch,\n+\t\t\t\t\t s->state.onto);\n \t\telse\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently editing a commit during a rebase.\"));\n@@ -1363,11 +1364,10 @@ static void show_rebase_in_progress(struct wt_status *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-\t\t\t\t\tconst char *color)\n+\t\t\t\t\t const char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently cherry-picking commit %s.\"),\n-\t\t\tfind_unique_abbrev(&state->cherry_pick_head_oid, DEFAULT_ABBREV));\n+\t\t\tfind_unique_abbrev(&s->state.cherry_pick_head_oid, DEFAULT_ABBREV));\n \tif (s->hints) {\n \t\tif (has_unmerged(s))\n \t\t\tstatus_printf_ln(s, color,\n@@ -1382,11 +1382,10 @@ static void show_cherry_pick_in_progress(struct wt_status *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-\t\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently reverting commit %s.\"),\n-\t\t\t find_unique_abbrev(&state->revert_head_oid, DEFAULT_ABBREV));\n+\t\t\t find_unique_abbrev(&s->state.revert_head_oid, DEFAULT_ABBREV));\n \tif (s->hints) {\n \t\tif (has_unmerged(s))\n \t\t\tstatus_printf_ln(s, color,\n@@ -1401,13 +1400,12 @@ static void show_revert_in_progress(struct wt_status *s,\n }\n \n static void show_bisect_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n-\tif (state->branch)\n+\tif (s->state.branch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently bisecting, started from branch '%s'.\"),\n-\t\t\t\t state->branch);\n+\t\t\t\t s->state.branch);\n \telse\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently bisecting.\"));\n@@ -1581,48 +1579,45 @@ void wt_status_get_state(struct wt_status_state *state,\n \t\twt_status_get_detached_from(state);\n }\n \n-static void wt_longstatus_print_state(struct wt_status *s,\n-\t\t\t\t struct wt_status_state *state)\n+static void wt_longstatus_print_state(struct wt_status *s)\n {\n \tconst char *state_color = color(WT_STATUS_HEADER, s);\n+\tstruct wt_status_state *state = &s->state;\n+\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+\t\tshow_am_in_progress(s, state_color);\n \telse if (state->rebase_in_progress || state->rebase_interactive_in_progress)\n-\t\tshow_rebase_in_progress(s, state, state_color);\n+\t\tshow_rebase_in_progress(s, state_color);\n \telse if (state->cherry_pick_in_progress)\n-\t\tshow_cherry_pick_in_progress(s, state, state_color);\n+\t\tshow_cherry_pick_in_progress(s, state_color);\n \telse if (state->revert_in_progress)\n-\t\tshow_revert_in_progress(s, state, state_color);\n+\t\tshow_revert_in_progress(s, state_color);\n \tif (state->bisect_in_progress)\n-\t\tshow_bisect_in_progress(s, state, state_color);\n+\t\tshow_bisect_in_progress(s, state_color);\n }\n \n static void wt_longstatus_print(struct wt_status *s)\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 (s->state.rebase_in_progress ||\n+\t\t\t s->state.rebase_interactive_in_progress) {\n+\t\t\t\tif (s->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 = s->state.onto;\n+\t\t\t} else if (s->state.detached_from) {\n+\t\t\t\tbranch_name = s->state.detached_from;\n+\t\t\t\tif (s->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@@ -1639,10 +1634,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);\n \n \tif (s->is_initial) {\n \t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n@@ -1946,13 +1938,9 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\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@@ -1963,10 +1951,11 @@ 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 (s->state.rebase_in_progress ||\n+\t\t\t s->state.rebase_interactive_in_progress)\n+\t\t\t\tbranch_name = s->state.onto;\n+\t\t\telse if (s->state.detached_from)\n+\t\t\t\tbranch_name = s->state.detached_from;\n \t\t\telse\n \t\t\t\tbranch_name = \"\";\n \t\t} else {\n@@ -2000,10 +1989,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 /*\ndiff --git a/wt-status.h b/wt-status.h\nindex 937b2c3521..1fcf93afbf 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -64,6 +64,24 @@ enum wt_status_format {\n \tSTATUS_FORMAT_UNSPECIFIED\n };\n \n+struct wt_status_state {\n+\tint merge_in_progress;\n+\tint am_in_progress;\n+\tint am_empty_patch;\n+\tint rebase_in_progress;\n+\tint rebase_interactive_in_progress;\n+\tint cherry_pick_in_progress;\n+\tint bisect_in_progress;\n+\tint revert_in_progress;\n+\tint detached_at;\n+\tchar *branch;\n+\tchar *onto;\n+\tchar *detached_from;\n+\tstruct object_id detached_oid;\n+\tstruct object_id revert_head_oid;\n+\tstruct object_id cherry_pick_head_oid;\n+};\n+\n struct wt_status {\n \tint is_initial;\n \tchar *branch;\n@@ -93,6 +111,7 @@ struct wt_status {\n \tint rename_score;\n \tint rename_limit;\n \tenum wt_status_format status_format;\n+\tstruct wt_status_state state;\n \tunsigned char sha1_commit[GIT_MAX_RAWSZ]; /* when not Initial */\n \n \t/* These are computed during processing of the individual sections */\n@@ -107,29 +126,12 @@ struct wt_status {\n \tuint32_t untracked_in_ms;\n };\n \n-struct wt_status_state {\n-\tint merge_in_progress;\n-\tint am_in_progress;\n-\tint am_empty_patch;\n-\tint rebase_in_progress;\n-\tint rebase_interactive_in_progress;\n-\tint cherry_pick_in_progress;\n-\tint bisect_in_progress;\n-\tint revert_in_progress;\n-\tint detached_at;\n-\tchar *branch;\n-\tchar *onto;\n-\tchar *detached_from;\n-\tstruct object_id detached_oid;\n-\tstruct object_id revert_head_oid;\n-\tstruct object_id cherry_pick_head_oid;\n-};\n-\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_collect_free_buffers(struct wt_status *s);\n void wt_status_get_state(struct wt_status_state *state, int get_detached_from);\n int wt_status_check_rebase(const struct worktree *wt,\n \t\t\t struct wt_status_state *state);\n-- \n2.19.0\n\n"},{"id":"359201","messageId":"20180928135549.GA23652@syl","threadId":"49289","inReplyTo":"20180928044936.2919-2-ischis2@cox.net","subject":"Re: [PATCH 1/1] roll wt_status_state into wt_status and populate in the collect phase","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-09-28T13:55:49Z","receivedAt":"2018-09-28T13:55:54Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Sep 27, 2018 at 09:49:36PM -0700, Stephen P. Smith wrote:\n> When updating the collect and print functions, it was found that\n> status variables were initialized in the collect phase and some\n> variables were later freed in the print functions.\n\nNit: I think that in the past Eric Sunshine has recommended that I use\nactive voice in patches, but \"it was found\" is passive.\n\nI tried to find the message that I was thinking of, but couldn't, so\nperhaps I'm inventing it myself ;-).\n\nI'm CC-ing Eric to check my judgement.\n\n> Move the status state structure variables into the status state\n> structure and populate them in the collect functions.\n>\n> Create a new funciton to free the buffers that were being freed in the\n> print function. Call this new function in commit.c where both the\n> collect and print functions were being called.\n>\n> Based on a patch suggestion by Junio C Hamano. [1]\n>\n> [1] https://public-inbox.org/git/xmqqr2i5ueg4.fsf@gitster-ct.c.googlers.com/\n>\n> Signed-off-by: Stephen P. Smith \n> ---\n> builtin/commit.c | 3 ++\n> wt-status.c | 135 +++++++++++++++++++++--------------------------\n> wt-status.h | 38 ++++++-------\n> 3 files changed, 83 insertions(+), 93 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 51ecebbec1..e168321e49 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -506,6 +506,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n>\n> \twt_status_collect(s);\n> \twt_status_print(s);\n> +\twt_status_collect_free_buffers(s);\n>\n> \treturn s->committable;\n> }\n> @@ -1388,6 +1389,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n> \t\ts.prefix = prefix;\n>\n> \twt_status_print(&s);\n> +\twt_status_collect_free_buffers(&s);\n> +\n> \treturn 0;\n> }\n>\n> diff --git a/wt-status.c b/wt-status.c\n> index c7f76d4758..9977f0cdf2 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -744,21 +744,26 @@ static int has_unmerged(struct wt_status *s)\n>\n> void wt_status_collect(struct wt_status *s)\n> {\n> -\tstruct wt_status_state state;\n> \twt_status_collect_changes_worktree(s);\n> -\n\nNit: unnecessary diff, but I certainly don't think that this is worth a\nre-roll on its own.\n\n> \tif (s->is_initial)\n> \t\twt_status_collect_changes_initial(s);\n> \telse\n> \t\twt_status_collect_changes_index(s);\n> \twt_status_collect_untracked(s);\n>\n> -\tmemset(&state, 0, sizeof(state));\n> -\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n> -\tif (state.merge_in_progress && !has_unmerged(s))\n> +\twt_status_get_state(&s->state, s->branch && !strcmp(s->branch, \"HEAD\"));\n> +\tif (s->state.merge_in_progress && !has_unmerged(s))\n> \t\ts->committable = 1;\n\nShould this line be de-dented to match the above?\n\n> }\n>\n> +void wt_status_collect_free_buffers(struct wt_status *s)\n> +{\n> +\tfree(s->state.branch);\n> +\tfree(s->state.onto);\n> +\tfree(s->state.detached_from);\n> +}\n> +\n> +\n\nNit: too much whitespace between 'wt_status_collect_free_buffers()' and\n'wt_longstatus_print_unmerged()' below. I see that there are two\nnewlines above, but I think that there should just be one.\n\n> static void wt_longstatus_print_unmerged(struct wt_status *s)\n> {\n> \tint shown_header = 0;\n> @@ -1087,8 +1092,7 @@ static void wt_longstatus_print_tracking(struct wt_status *s)\n> }\n\nThe rest of this patch looks sensible to me, but I didn't follow the\noriginal discussion in [1], so take my review with a grain of salt :-).\n\nThanks,\nTaylor\n"},{"id":"359230","messageId":"xmqq4le9cvy6.fsf@gitster-ct.c.googlers.com","threadId":"49289","inReplyTo":"20180928135549.GA23652@syl","subject":"Re: [PATCH 1/1] roll wt_status_state into wt_status and populate in the collect phase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-28T18:34:41Z","receivedAt":"2018-09-28T18:34:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> On Thu, Sep 27, 2018 at 09:49:36PM -0700, Stephen P. Smith wrote:\n>> When updating the collect and print functions, it was found that\n>> status variables were initialized in the collect phase and some\n>> variables were later freed in the print functions.\n>\n> Nit: I think that in the past Eric Sunshine has recommended that I use\n> active voice in patches, but \"it was found\" is passive.\n\nYeah, and when/how it was found is much less interesting backstory\nthan _why_ we are doing this follow-thru. I think the first line\ncan just simply go without losing clarity.\n\n>> Move the status state structure variables into the status state\n>> structure and populate them in the collect functions.\n\nOn the other hand this one may deserve a bit more backstory. If I\nunderstand correctly, what happened over time was\n\n - A \"struct wt_status\" used to be sufficient for the output phase\n to work. It was designed to be filled in the collect phase and\n consumed in the output phase, but over time some fields are added\n and output phase started filling it; we recently corrected it so\n that .committable field is filled in the collect phase.\n\n A \"struct wt_status_state\" that was used in other codepaths\n turned out to be useful in showing the \"git status\" output, so\n some output phase functions started taking it. This is not tied\n to \"struct wt_status\", so the discipline of filling in the\n collect phase to be consumed in the output phase was never\n followed.\n\nI am not suggesting to write that much in the log message, but and\nwith a backstory like that, embedding a wt_status_state inside\nwt_status and fill it in the collect phase, which this patch does,\nstarts to make sense, I would think.\n\n>> diff --git a/wt-status.c b/wt-status.c\n>> index c7f76d4758..9977f0cdf2 100644\n>> --- a/wt-status.c\n>> +++ b/wt-status.c\n>> @@ -744,21 +744,26 @@ static int has_unmerged(struct wt_status *s)\n>>\n>> void wt_status_collect(struct wt_status *s)\n>> {\n>> -\tstruct wt_status_state state;\n>> \twt_status_collect_changes_worktree(s);\n>> -\n>\n> Nit: unnecessary diff, but I certainly don't think that this is worth a\n> re-roll on its own.\n\nI do not think it is unnecessary to remove the blank between three\nthings this function does (i.e. (1) inspect working tree, (2)\ninspect index and (3) inspect untrackeed; if there is no blank line\nbetween (2) and (3), we shouldn't have a blank between (1) and (2)).\n\nI do agree with you it is an unrelated change. Its correctness (not\nto the compiler, but to the humans due to the above) is so trivial\nthat it probably is a good taste to include it in this patch.\n\n>> \tif (s->is_initial)\n>> \t\twt_status_collect_changes_initial(s);\n>> \telse\n>> \t\twt_status_collect_changes_index(s);\n>> \twt_status_collect_untracked(s);\n>>\n>> -\tmemset(&state, 0, sizeof(state));\n>> -\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n>> -\tif (state.merge_in_progress && !has_unmerged(s))\n>> +\twt_status_get_state(&s->state, s->branch && !strcmp(s->branch, \"HEAD\"));\n>> +\tif (s->state.merge_in_progress && !has_unmerged(s))\n>> \t\ts->committable = 1;\n>\n> Should this line be de-dented to match the above?\n\nI am not sure if I follow.\n\nThanks.\n"},{"id":"359307","messageId":"20180929185539.4144-1-ischis2@cox.net","threadId":"49289","inReplyTo":"xmqq4le9cvy6.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 0/1] wt-status-state-cleanup","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-29T18:55:38Z","receivedAt":"2018-09-29T18:55:43Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"Junio suggested a cleanup patch, jc/wt-status-state-cleanup, which is\nthe basis for this patch.\n\nThis patch uses ss/wt-status-committable.\n\nUpdated commit comment and removed one extra blank line insertion.\nStephen P. Smith (1):\n roll wt_status_state into wt_status and populate in the collect phase\n\n builtin/commit.c | 3 ++\n wt-status.c | 134 +++++++++++++++++++++--------------------------\n wt-status.h | 38 +++++++-------\n 3 files changed, 82 insertions(+), 93 deletions(-)\n\n-- \n2.19.0\n\n"},{"id":"359308","messageId":"20180929185539.4144-2-ischis2@cox.net","threadId":"49289","inReplyTo":"20180929185539.4144-1-ischis2@cox.net","subject":"[PATCH v2 1/1] roll wt_status_state into wt_status and populate in the collect phase","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-29T18:55:39Z","receivedAt":"2018-09-29T18:55:45Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"Status variables were initialized in the collect phase and some\nvariables were later freed in the print functions.\n\nA \"struct wt_status\" used to be sufficient for the output phase to\nwork. It was designed to be filled in the collect phase and consumed\nin the output phase, but over time some fields were added and output\nphase started filling the fields.\n\nA \"struct wt_status_state\" that was used in other codepaths turned out\nto be useful in the \"git status\" output. This is not tied to \"struct\nwt_status\", so filling in the collect phase was not consistently\nfollowed.\n\nMove the status state structure variables into the status state\nstructure and populate them in the collect functions.\n\nCreate a new funciton to free the buffers that were being freed in the\nprint function. Call this new function in commit.c where both the\ncollect and print functions were being called.\n\nBased on a patch suggestion by Junio C Hamano. [1]\n\n[1] https://public-inbox.org/git/xmqqr2i5ueg4.fsf@gitster-ct.c.googlers.com/\n\nSigned-off-by: Stephen P. Smith \n---\n builtin/commit.c | 3 ++\n wt-status.c | 134 +++++++++++++++++++++--------------------------\n wt-status.h | 38 +++++++-------\n 3 files changed, 82 insertions(+), 93 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d6efd996f8..c91af2fe38 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -508,6 +508,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \n \twt_status_collect(s);\n \twt_status_print(s);\n+\twt_status_collect_free_buffers(s);\n \n \treturn s->committable;\n }\n@@ -1390,6 +1391,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \t\ts.prefix = prefix;\n \n \twt_status_print(&s);\n+\twt_status_collect_free_buffers(&s);\n+\n \treturn 0;\n }\n \ndiff --git a/wt-status.c b/wt-status.c\nindex 1822e95f65..cc3e84b997 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -744,21 +744,25 @@ static int has_unmerged(struct wt_status *s)\n \n void wt_status_collect(struct wt_status *s)\n {\n-\tstruct wt_status_state state;\n \twt_status_collect_changes_worktree(s);\n-\n \tif (s->is_initial)\n \t\twt_status_collect_changes_initial(s);\n \telse\n \t\twt_status_collect_changes_index(s);\n \twt_status_collect_untracked(s);\n \n-\tmemset(&state, 0, sizeof(state));\n-\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n-\tif (state.merge_in_progress && !has_unmerged(s))\n+\twt_status_get_state(&s->state, s->branch && !strcmp(s->branch, \"HEAD\"));\n+\tif (s->state.merge_in_progress && !has_unmerged(s))\n \t\ts->committable = 1;\n }\n \n+void wt_status_collect_free_buffers(struct wt_status *s)\n+{\n+\tfree(s->state.branch);\n+\tfree(s->state.onto);\n+\tfree(s->state.detached_from);\n+}\n+\n static void wt_longstatus_print_unmerged(struct wt_status *s)\n {\n \tint shown_header = 0;\n@@ -1087,8 +1091,7 @@ static void wt_longstatus_print_tracking(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 char *color)\n+\t\t\t\t const char *color)\n {\n \tif (has_unmerged(s)) {\n \t\tstatus_printf_ln(s, color, _(\"You have unmerged paths.\"));\n@@ -1109,16 +1112,15 @@ static void show_merge_in_progress(struct wt_status *s,\n }\n \n static void show_am_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tstatus_printf_ln(s, color,\n \t\t_(\"You are in the middle of an am session.\"));\n-\tif (state->am_empty_patch)\n+\tif (s->state.am_empty_patch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"The current patch is empty.\"));\n \tif (s->hints) {\n-\t\tif (!state->am_empty_patch)\n+\t\tif (!s->state.am_empty_patch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (fix conflicts and then run \\\"git am --continue\\\")\"));\n \t\tstatus_printf_ln(s, color,\n@@ -1242,10 +1244,9 @@ static int read_rebase_todolist(const char *fname, struct string_list *lines)\n }\n \n static void show_rebase_information(struct wt_status *s,\n-\t\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n-\tif (state->rebase_interactive_in_progress) {\n+\tif (s->state.rebase_interactive_in_progress) {\n \t\tint i;\n \t\tint nr_lines_to_show = 2;\n \n@@ -1296,28 +1297,26 @@ static void show_rebase_information(struct wt_status *s,\n }\n \n static void print_rebase_state(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\tconst char *color)\n+\t\t\t const char *color)\n {\n-\tif (state->branch)\n+\tif (s->state.branch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t state->branch,\n-\t\t\t\t state->onto);\n+\t\t\t\t s->state.branch,\n+\t\t\t\t s->state.onto);\n \telse\n \t\tstatus_printf_ln(s, color,\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-\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n \tstruct stat st;\n \n-\tshow_rebase_information(s, state, color);\n+\tshow_rebase_information(s, color);\n \tif (has_unmerged(s)) {\n-\t\tprint_rebase_state(s, state, color);\n+\t\tprint_rebase_state(s, color);\n \t\tif (s->hints) {\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (fix conflicts and then run \\\"git rebase --continue\\\")\"));\n@@ -1326,17 +1325,18 @@ static void show_rebase_in_progress(struct wt_status *s,\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (use \\\"git rebase --abort\\\" to check out the original branch)\"));\n \t\t}\n-\t} else if (state->rebase_in_progress || !stat(git_path_merge_msg(the_repository), &st)) {\n-\t\tprint_rebase_state(s, state, color);\n+\t} else if (s->state.rebase_in_progress ||\n+\t\t !stat(git_path_merge_msg(the_repository), &st)) {\n+\t\tprint_rebase_state(s, color);\n \t\tif (s->hints)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (all conflicts fixed: run \\\"git rebase --continue\\\")\"));\n \t} else if (split_commit_in_progress(s)) {\n-\t\tif (state->branch)\n+\t\tif (s->state.branch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently splitting a commit while rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t\t state->branch,\n-\t\t\t\t\t state->onto);\n+\t\t\t\t\t s->state.branch,\n+\t\t\t\t\t s->state.onto);\n \t\telse\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently splitting a commit during a rebase.\"));\n@@ -1344,11 +1344,11 @@ static void show_rebase_in_progress(struct wt_status *s,\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (Once your working directory is clean, run \\\"git rebase --continue\\\")\"));\n \t} else {\n-\t\tif (state->branch)\n+\t\tif (s->state.branch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently editing a commit while rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t\t state->branch,\n-\t\t\t\t\t state->onto);\n+\t\t\t\t\t s->state.branch,\n+\t\t\t\t\t s->state.onto);\n \t\telse\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently editing a commit during a rebase.\"));\n@@ -1363,11 +1363,10 @@ static void show_rebase_in_progress(struct wt_status *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-\t\t\t\t\tconst char *color)\n+\t\t\t\t\t const char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently cherry-picking commit %s.\"),\n-\t\t\tfind_unique_abbrev(&state->cherry_pick_head_oid, DEFAULT_ABBREV));\n+\t\t\tfind_unique_abbrev(&s->state.cherry_pick_head_oid, DEFAULT_ABBREV));\n \tif (s->hints) {\n \t\tif (has_unmerged(s))\n \t\t\tstatus_printf_ln(s, color,\n@@ -1382,11 +1381,10 @@ static void show_cherry_pick_in_progress(struct wt_status *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-\t\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently reverting commit %s.\"),\n-\t\t\t find_unique_abbrev(&state->revert_head_oid, DEFAULT_ABBREV));\n+\t\t\t find_unique_abbrev(&s->state.revert_head_oid, DEFAULT_ABBREV));\n \tif (s->hints) {\n \t\tif (has_unmerged(s))\n \t\t\tstatus_printf_ln(s, color,\n@@ -1401,13 +1399,12 @@ static void show_revert_in_progress(struct wt_status *s,\n }\n \n static void show_bisect_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n-\tif (state->branch)\n+\tif (s->state.branch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently bisecting, started from branch '%s'.\"),\n-\t\t\t\t state->branch);\n+\t\t\t\t s->state.branch);\n \telse\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently bisecting.\"));\n@@ -1581,48 +1578,45 @@ void wt_status_get_state(struct wt_status_state *state,\n \t\twt_status_get_detached_from(state);\n }\n \n-static void wt_longstatus_print_state(struct wt_status *s,\n-\t\t\t\t struct wt_status_state *state)\n+static void wt_longstatus_print_state(struct wt_status *s)\n {\n \tconst char *state_color = color(WT_STATUS_HEADER, s);\n+\tstruct wt_status_state *state = &s->state;\n+\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+\t\tshow_am_in_progress(s, state_color);\n \telse if (state->rebase_in_progress || state->rebase_interactive_in_progress)\n-\t\tshow_rebase_in_progress(s, state, state_color);\n+\t\tshow_rebase_in_progress(s, state_color);\n \telse if (state->cherry_pick_in_progress)\n-\t\tshow_cherry_pick_in_progress(s, state, state_color);\n+\t\tshow_cherry_pick_in_progress(s, state_color);\n \telse if (state->revert_in_progress)\n-\t\tshow_revert_in_progress(s, state, state_color);\n+\t\tshow_revert_in_progress(s, state_color);\n \tif (state->bisect_in_progress)\n-\t\tshow_bisect_in_progress(s, state, state_color);\n+\t\tshow_bisect_in_progress(s, state_color);\n }\n \n static void wt_longstatus_print(struct wt_status *s)\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 (s->state.rebase_in_progress ||\n+\t\t\t s->state.rebase_interactive_in_progress) {\n+\t\t\t\tif (s->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 = s->state.onto;\n+\t\t\t} else if (s->state.detached_from) {\n+\t\t\t\tbranch_name = s->state.detached_from;\n+\t\t\t\tif (s->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@@ -1639,10 +1633,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);\n \n \tif (s->is_initial) {\n \t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n@@ -1946,13 +1937,9 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\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@@ -1963,10 +1950,11 @@ 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 (s->state.rebase_in_progress ||\n+\t\t\t s->state.rebase_interactive_in_progress)\n+\t\t\t\tbranch_name = s->state.onto;\n+\t\t\telse if (s->state.detached_from)\n+\t\t\t\tbranch_name = s->state.detached_from;\n \t\t\telse\n \t\t\t\tbranch_name = \"\";\n \t\t} else {\n@@ -2000,10 +1988,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 /*\ndiff --git a/wt-status.h b/wt-status.h\nindex 937b2c3521..1fcf93afbf 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -64,6 +64,24 @@ enum wt_status_format {\n \tSTATUS_FORMAT_UNSPECIFIED\n };\n \n+struct wt_status_state {\n+\tint merge_in_progress;\n+\tint am_in_progress;\n+\tint am_empty_patch;\n+\tint rebase_in_progress;\n+\tint rebase_interactive_in_progress;\n+\tint cherry_pick_in_progress;\n+\tint bisect_in_progress;\n+\tint revert_in_progress;\n+\tint detached_at;\n+\tchar *branch;\n+\tchar *onto;\n+\tchar *detached_from;\n+\tstruct object_id detached_oid;\n+\tstruct object_id revert_head_oid;\n+\tstruct object_id cherry_pick_head_oid;\n+};\n+\n struct wt_status {\n \tint is_initial;\n \tchar *branch;\n@@ -93,6 +111,7 @@ struct wt_status {\n \tint rename_score;\n \tint rename_limit;\n \tenum wt_status_format status_format;\n+\tstruct wt_status_state state;\n \tunsigned char sha1_commit[GIT_MAX_RAWSZ]; /* when not Initial */\n \n \t/* These are computed during processing of the individual sections */\n@@ -107,29 +126,12 @@ struct wt_status {\n \tuint32_t untracked_in_ms;\n };\n \n-struct wt_status_state {\n-\tint merge_in_progress;\n-\tint am_in_progress;\n-\tint am_empty_patch;\n-\tint rebase_in_progress;\n-\tint rebase_interactive_in_progress;\n-\tint cherry_pick_in_progress;\n-\tint bisect_in_progress;\n-\tint revert_in_progress;\n-\tint detached_at;\n-\tchar *branch;\n-\tchar *onto;\n-\tchar *detached_from;\n-\tstruct object_id detached_oid;\n-\tstruct object_id revert_head_oid;\n-\tstruct object_id cherry_pick_head_oid;\n-};\n-\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_collect_free_buffers(struct wt_status *s);\n void wt_status_get_state(struct wt_status_state *state, int get_detached_from);\n int wt_status_check_rebase(const struct worktree *wt,\n \t\t\t struct wt_status_state *state);\n-- \n2.19.0\n\n"},{"id":"359327","messageId":"CAPig+cSW1D9hEc8CX-QmqSA8jJw+2E1hSYXk7osZJVQ9_JNThQ@mail.gmail.com","threadId":"49289","inReplyTo":"20180928135549.GA23652@syl","subject":"Re: [PATCH 1/1] roll wt_status_state into wt_status and populate in the collect phase","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-09-30T04:40:20Z","receivedAt":"2018-09-30T04:40:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 28, 2018 at 9:55 AM Taylor Blau wrote:\n> On Thu, Sep 27, 2018 at 09:49:36PM -0700, Stephen P. Smith wrote:\n> > When updating the collect and print functions, it was found that\n> > status variables were initialized in the collect phase and some\n> > variables were later freed in the print functions.\n>\n> Nit: I think that in the past Eric Sunshine has recommended that I use\n> active voice in patches, but \"it was found\" is passive.\n>\n> I tried to find the message that I was thinking of, but couldn't, so\n> perhaps I'm inventing it myself ;-).\n>\n> I'm CC-ing Eric to check my judgement.\n\nYou're probably thinking of \"imperative mood\" (and perhaps [1]), which\nthis commit message already uses when it says \"Move the...\" and\n\"Create a new function...\" (in the couple paragraphs following the\npart you quoted).\n\n> > Move the status state structure variables into the status state\n> > structure and populate them in the collect functions.\n> >\n> > Create a new funciton to free the buffers that were being freed in the\n\ns/funciton/function/\n\n> > print function. Call this new function in commit.c where both the\n> > collect and print functions were being called.\n\n[1]: https://public-inbox.org/git/CAPig+cTozduqSAxh+w4H85m7en72Yo09asdx+1KSTswqbnBr4w@mail.gmail.com/\n"},{"id":"359328","messageId":"CAPig+cTm_Fb9xFend5UF2TAx+5M6+siseRDVEAVGYUodjBAMRg@mail.gmail.com","threadId":"49289","inReplyTo":"20180929185539.4144-2-ischis2@cox.net","subject":"Re: [PATCH v2 1/1] roll wt_status_state into wt_status and populate in the collect phase","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-09-30T04:41:14Z","receivedAt":"2018-09-30T04:41:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Sep 29, 2018 at 2:55 PM Stephen P. Smith wrote:\n> Status variables were initialized in the collect phase and some\n> variables were later freed in the print functions.\n>\n> A \"struct wt_status\" used to be sufficient for the output phase to\n> work. It was designed to be filled in the collect phase and consumed\n> in the output phase, but over time some fields were added and output\n> phase started filling the fields.\n>\n> A \"struct wt_status_state\" that was used in other codepaths turned out\n> to be useful in the \"git status\" output. This is not tied to \"struct\n> wt_status\", so filling in the collect phase was not consistently\n> followed.\n>\n> Move the status state structure variables into the status state\n> structure and populate them in the collect functions.\n>\n> Create a new funciton to free the buffers that were being freed in the\n\ns/funciton/function/\n\n> print function. Call this new function in commit.c where both the\n> collect and print functions were being called.\n>\n> Signed-off-by: Stephen P. Smith \n"},{"id":"359340","messageId":"20180930141246.19651-1-ischis2@cox.net","threadId":"49289","inReplyTo":"CAPig+cTm_Fb9xFend5UF2TAx+5M6+siseRDVEAVGYUodjBAMRg@mail.gmail.com","subject":"[PATCH v3 0/1] wt-status-state-cleanup","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-30T14:12:44Z","receivedAt":"2018-09-30T14:12:51Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"Junio suggested a cleanup patch, jc/wt-status-state-cleanup, which is\nthe basis for this patch.\n\nThis patch uses ss/wt-status-committable.\n\nUpdate to fix a spelling error in the commet message\n\nStephen P. Smith (1):\n roll wt_status_state into wt_status and populate in the collect phase\n\n builtin/commit.c | 3 ++\n wt-status.c | 134 +++++++++++++++++++++--------------------------\n wt-status.h | 38 +++++++-------\n 3 files changed, 82 insertions(+), 93 deletions(-)\n\n-- \n2.19.0\n\n"},{"id":"359341","messageId":"20180930141246.19651-2-ischis2@cox.net","threadId":"49289","inReplyTo":"20180930141246.19651-1-ischis2@cox.net","subject":"[PATCH v3 1/1] roll wt_status_state into wt_status and populate in the collect phase","fromName":"Stephen P. Smith","fromEmail":"ischis2@cox.net","sentAt":"2018-09-30T14:12:45Z","receivedAt":"2018-09-30T14:12:53Z","isPatch":true,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"Status variables were initialized in the collect phase and some\nvariables were later freed in the print functions.\n\nA \"struct wt_status\" used to be sufficient for the output phase to\nwork. It was designed to be filled in the collect phase and consumed\nin the output phase, but over time some fields were added and output\nphase started filling the fields.\n\nA \"struct wt_status_state\" that was used in other codepaths turned out\nto be useful in the \"git status\" output. This is not tied to \"struct\nwt_status\", so filling in the collect phase was not consistently\nfollowed.\n\nMove the status state structure variables into the status state\nstructure and populate them in the collect functions.\n\nCreate a new function to free the buffers that were being freed in the\nprint function. Call this new function in commit.c where both the\ncollect and print functions were being called.\n\nBased on a patch suggestion by Junio C Hamano. [1]\n\n[1] https://public-inbox.org/git/xmqqr2i5ueg4.fsf@gitster-ct.c.googlers.com/\n\nSigned-off-by: Stephen P. Smith \n---\n builtin/commit.c | 3 ++\n wt-status.c | 134 +++++++++++++++++++++--------------------------\n wt-status.h | 38 +++++++-------\n 3 files changed, 82 insertions(+), 93 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d6efd996f8..c91af2fe38 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -508,6 +508,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \n \twt_status_collect(s);\n \twt_status_print(s);\n+\twt_status_collect_free_buffers(s);\n \n \treturn s->committable;\n }\n@@ -1390,6 +1391,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \t\ts.prefix = prefix;\n \n \twt_status_print(&s);\n+\twt_status_collect_free_buffers(&s);\n+\n \treturn 0;\n }\n \ndiff --git a/wt-status.c b/wt-status.c\nindex 1822e95f65..cc3e84b997 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -744,21 +744,25 @@ static int has_unmerged(struct wt_status *s)\n \n void wt_status_collect(struct wt_status *s)\n {\n-\tstruct wt_status_state state;\n \twt_status_collect_changes_worktree(s);\n-\n \tif (s->is_initial)\n \t\twt_status_collect_changes_initial(s);\n \telse\n \t\twt_status_collect_changes_index(s);\n \twt_status_collect_untracked(s);\n \n-\tmemset(&state, 0, sizeof(state));\n-\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n-\tif (state.merge_in_progress && !has_unmerged(s))\n+\twt_status_get_state(&s->state, s->branch && !strcmp(s->branch, \"HEAD\"));\n+\tif (s->state.merge_in_progress && !has_unmerged(s))\n \t\ts->committable = 1;\n }\n \n+void wt_status_collect_free_buffers(struct wt_status *s)\n+{\n+\tfree(s->state.branch);\n+\tfree(s->state.onto);\n+\tfree(s->state.detached_from);\n+}\n+\n static void wt_longstatus_print_unmerged(struct wt_status *s)\n {\n \tint shown_header = 0;\n@@ -1087,8 +1091,7 @@ static void wt_longstatus_print_tracking(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 char *color)\n+\t\t\t\t const char *color)\n {\n \tif (has_unmerged(s)) {\n \t\tstatus_printf_ln(s, color, _(\"You have unmerged paths.\"));\n@@ -1109,16 +1112,15 @@ static void show_merge_in_progress(struct wt_status *s,\n }\n \n static void show_am_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n \t\t\t\tconst char *color)\n {\n \tstatus_printf_ln(s, color,\n \t\t_(\"You are in the middle of an am session.\"));\n-\tif (state->am_empty_patch)\n+\tif (s->state.am_empty_patch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"The current patch is empty.\"));\n \tif (s->hints) {\n-\t\tif (!state->am_empty_patch)\n+\t\tif (!s->state.am_empty_patch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (fix conflicts and then run \\\"git am --continue\\\")\"));\n \t\tstatus_printf_ln(s, color,\n@@ -1242,10 +1244,9 @@ static int read_rebase_todolist(const char *fname, struct string_list *lines)\n }\n \n static void show_rebase_information(struct wt_status *s,\n-\t\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n-\tif (state->rebase_interactive_in_progress) {\n+\tif (s->state.rebase_interactive_in_progress) {\n \t\tint i;\n \t\tint nr_lines_to_show = 2;\n \n@@ -1296,28 +1297,26 @@ static void show_rebase_information(struct wt_status *s,\n }\n \n static void print_rebase_state(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\tconst char *color)\n+\t\t\t const char *color)\n {\n-\tif (state->branch)\n+\tif (s->state.branch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t state->branch,\n-\t\t\t\t state->onto);\n+\t\t\t\t s->state.branch,\n+\t\t\t\t s->state.onto);\n \telse\n \t\tstatus_printf_ln(s, color,\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-\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n \tstruct stat st;\n \n-\tshow_rebase_information(s, state, color);\n+\tshow_rebase_information(s, color);\n \tif (has_unmerged(s)) {\n-\t\tprint_rebase_state(s, state, color);\n+\t\tprint_rebase_state(s, color);\n \t\tif (s->hints) {\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (fix conflicts and then run \\\"git rebase --continue\\\")\"));\n@@ -1326,17 +1325,18 @@ static void show_rebase_in_progress(struct wt_status *s,\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (use \\\"git rebase --abort\\\" to check out the original branch)\"));\n \t\t}\n-\t} else if (state->rebase_in_progress || !stat(git_path_merge_msg(the_repository), &st)) {\n-\t\tprint_rebase_state(s, state, color);\n+\t} else if (s->state.rebase_in_progress ||\n+\t\t !stat(git_path_merge_msg(the_repository), &st)) {\n+\t\tprint_rebase_state(s, color);\n \t\tif (s->hints)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (all conflicts fixed: run \\\"git rebase --continue\\\")\"));\n \t} else if (split_commit_in_progress(s)) {\n-\t\tif (state->branch)\n+\t\tif (s->state.branch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently splitting a commit while rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t\t state->branch,\n-\t\t\t\t\t state->onto);\n+\t\t\t\t\t s->state.branch,\n+\t\t\t\t\t s->state.onto);\n \t\telse\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently splitting a commit during a rebase.\"));\n@@ -1344,11 +1344,11 @@ static void show_rebase_in_progress(struct wt_status *s,\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\" (Once your working directory is clean, run \\\"git rebase --continue\\\")\"));\n \t} else {\n-\t\tif (state->branch)\n+\t\tif (s->state.branch)\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently editing a commit while rebasing branch '%s' on '%s'.\"),\n-\t\t\t\t\t state->branch,\n-\t\t\t\t\t state->onto);\n+\t\t\t\t\t s->state.branch,\n+\t\t\t\t\t s->state.onto);\n \t\telse\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t\t _(\"You are currently editing a commit during a rebase.\"));\n@@ -1363,11 +1363,10 @@ static void show_rebase_in_progress(struct wt_status *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-\t\t\t\t\tconst char *color)\n+\t\t\t\t\t const char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently cherry-picking commit %s.\"),\n-\t\t\tfind_unique_abbrev(&state->cherry_pick_head_oid, DEFAULT_ABBREV));\n+\t\t\tfind_unique_abbrev(&s->state.cherry_pick_head_oid, DEFAULT_ABBREV));\n \tif (s->hints) {\n \t\tif (has_unmerged(s))\n \t\t\tstatus_printf_ln(s, color,\n@@ -1382,11 +1381,10 @@ static void show_cherry_pick_in_progress(struct wt_status *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-\t\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n \tstatus_printf_ln(s, color, _(\"You are currently reverting commit %s.\"),\n-\t\t\t find_unique_abbrev(&state->revert_head_oid, DEFAULT_ABBREV));\n+\t\t\t find_unique_abbrev(&s->state.revert_head_oid, DEFAULT_ABBREV));\n \tif (s->hints) {\n \t\tif (has_unmerged(s))\n \t\t\tstatus_printf_ln(s, color,\n@@ -1401,13 +1399,12 @@ static void show_revert_in_progress(struct wt_status *s,\n }\n \n static void show_bisect_in_progress(struct wt_status *s,\n-\t\t\t\tstruct wt_status_state *state,\n-\t\t\t\tconst char *color)\n+\t\t\t\t const char *color)\n {\n-\tif (state->branch)\n+\tif (s->state.branch)\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently bisecting, started from branch '%s'.\"),\n-\t\t\t\t state->branch);\n+\t\t\t\t s->state.branch);\n \telse\n \t\tstatus_printf_ln(s, color,\n \t\t\t\t _(\"You are currently bisecting.\"));\n@@ -1581,48 +1578,45 @@ void wt_status_get_state(struct wt_status_state *state,\n \t\twt_status_get_detached_from(state);\n }\n \n-static void wt_longstatus_print_state(struct wt_status *s,\n-\t\t\t\t struct wt_status_state *state)\n+static void wt_longstatus_print_state(struct wt_status *s)\n {\n \tconst char *state_color = color(WT_STATUS_HEADER, s);\n+\tstruct wt_status_state *state = &s->state;\n+\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+\t\tshow_am_in_progress(s, state_color);\n \telse if (state->rebase_in_progress || state->rebase_interactive_in_progress)\n-\t\tshow_rebase_in_progress(s, state, state_color);\n+\t\tshow_rebase_in_progress(s, state_color);\n \telse if (state->cherry_pick_in_progress)\n-\t\tshow_cherry_pick_in_progress(s, state, state_color);\n+\t\tshow_cherry_pick_in_progress(s, state_color);\n \telse if (state->revert_in_progress)\n-\t\tshow_revert_in_progress(s, state, state_color);\n+\t\tshow_revert_in_progress(s, state_color);\n \tif (state->bisect_in_progress)\n-\t\tshow_bisect_in_progress(s, state, state_color);\n+\t\tshow_bisect_in_progress(s, state_color);\n }\n \n static void wt_longstatus_print(struct wt_status *s)\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 (s->state.rebase_in_progress ||\n+\t\t\t s->state.rebase_interactive_in_progress) {\n+\t\t\t\tif (s->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 = s->state.onto;\n+\t\t\t} else if (s->state.detached_from) {\n+\t\t\t\tbranch_name = s->state.detached_from;\n+\t\t\t\tif (s->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@@ -1639,10 +1633,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);\n \n \tif (s->is_initial) {\n \t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n@@ -1946,13 +1937,9 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\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@@ -1963,10 +1950,11 @@ 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 (s->state.rebase_in_progress ||\n+\t\t\t s->state.rebase_interactive_in_progress)\n+\t\t\t\tbranch_name = s->state.onto;\n+\t\t\telse if (s->state.detached_from)\n+\t\t\t\tbranch_name = s->state.detached_from;\n \t\t\telse\n \t\t\t\tbranch_name = \"\";\n \t\t} else {\n@@ -2000,10 +1988,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 /*\ndiff --git a/wt-status.h b/wt-status.h\nindex 937b2c3521..1fcf93afbf 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -64,6 +64,24 @@ enum wt_status_format {\n \tSTATUS_FORMAT_UNSPECIFIED\n };\n \n+struct wt_status_state {\n+\tint merge_in_progress;\n+\tint am_in_progress;\n+\tint am_empty_patch;\n+\tint rebase_in_progress;\n+\tint rebase_interactive_in_progress;\n+\tint cherry_pick_in_progress;\n+\tint bisect_in_progress;\n+\tint revert_in_progress;\n+\tint detached_at;\n+\tchar *branch;\n+\tchar *onto;\n+\tchar *detached_from;\n+\tstruct object_id detached_oid;\n+\tstruct object_id revert_head_oid;\n+\tstruct object_id cherry_pick_head_oid;\n+};\n+\n struct wt_status {\n \tint is_initial;\n \tchar *branch;\n@@ -93,6 +111,7 @@ struct wt_status {\n \tint rename_score;\n \tint rename_limit;\n \tenum wt_status_format status_format;\n+\tstruct wt_status_state state;\n \tunsigned char sha1_commit[GIT_MAX_RAWSZ]; /* when not Initial */\n \n \t/* These are computed during processing of the individual sections */\n@@ -107,29 +126,12 @@ struct wt_status {\n \tuint32_t untracked_in_ms;\n };\n \n-struct wt_status_state {\n-\tint merge_in_progress;\n-\tint am_in_progress;\n-\tint am_empty_patch;\n-\tint rebase_in_progress;\n-\tint rebase_interactive_in_progress;\n-\tint cherry_pick_in_progress;\n-\tint bisect_in_progress;\n-\tint revert_in_progress;\n-\tint detached_at;\n-\tchar *branch;\n-\tchar *onto;\n-\tchar *detached_from;\n-\tstruct object_id detached_oid;\n-\tstruct object_id revert_head_oid;\n-\tstruct object_id cherry_pick_head_oid;\n-};\n-\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_collect_free_buffers(struct wt_status *s);\n void wt_status_get_state(struct wt_status_state *state, int get_detached_from);\n int wt_status_check_rebase(const struct worktree *wt,\n \t\t\t struct wt_status_state *state);\n-- \n2.19.0\n\n"}]}