{"thread":{"id":"35928","subject":"[PATCH 0/3] make commit --verbose work with --no-status","startedAt":"2014-02-21T19:09:19Z","lastAt":"2014-02-24T08:33:12Z","messageCount":8,"participants":["Tay Ray Chuan","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"235150","messageId":"1393009762-31133-1-git-send-email-rctay89@gmail.com","threadId":"35928","inReplyTo":null,"subject":"[PATCH 0/3] make commit --verbose work with --no-status","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2014-02-21T19:09:19Z","receivedAt":"2014-02-21T19:09:19Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"One would expect 'git commit --verbose --no-status' to give a commit\nmessage with a diff of the commit, sans the output of git-status.\nHowever, this does not work currently; the commit message body is\nentirely empty (diff is absent as well). This patch series attempts to\nmake this work, as one would expect.\n\n[PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT\n\nWe first rename STATUS_FORMAT_NONE to *_DEFAULT, so that we can use it\nto mean \"no status output at all\".\n\n[PATCH 2/3] extract setting of wt_status.commitable flag out of\nwt_status_print_updated()\n\nCurrently, the wt_status.commitable flag is set only when we call\nwt_status_print(). * Extract this logic, so that we don't have to go\nthrough the git-status output code.\n\n* In fact, it is not set when --short or --porcelain are used. This\n  series does not attempt to fix the bug, though it should be trivial to\n  do so with this patch.\n\n[PATCH 3/3] make commit --verbose work with --no-status\n\nActual work here.\n\n-- \n1.9.0.291.g027825b\n"},{"id":"235151","messageId":"1393009762-31133-2-git-send-email-rctay89@gmail.com","threadId":"35928","inReplyTo":"1393009762-31133-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2014-02-21T19:09:20Z","receivedAt":"2014-02-21T19:09:20Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Cc: Jeff King <peff@peff.net>\n\nIn f3f47a1 (status: add --long output format option), STATUS_FORMAT_NONE\nwas introduced, meaning \"the user did not specify anything\". Rename this\nto *_DEFAULT to better indicate its meaning.\n\nThis paves the way for _NONE to really mean \"no status\".\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n builtin/commit.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 3783bca..2e86b76 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -125,7 +125,7 @@ static const char *only_include_assumed;\n static struct strbuf message = STRBUF_INIT;\n \n static enum status_format {\n-\tSTATUS_FORMAT_NONE = 0,\n+\tSTATUS_FORMAT_DEFAULT = 0,\n \tSTATUS_FORMAT_LONG,\n \tSTATUS_FORMAT_SHORT,\n \tSTATUS_FORMAT_PORCELAIN,\n@@ -487,7 +487,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \tcase STATUS_FORMAT_UNSPECIFIED:\n \t\tdie(\"BUG: finalize_deferred_config() should have been called\");\n \t\tbreak;\n-\tcase STATUS_FORMAT_NONE:\n+\tcase STATUS_FORMAT_DEFAULT:\n \tcase STATUS_FORMAT_LONG:\n \t\twt_status_print(s);\n \t\tbreak;\n@@ -1028,7 +1028,7 @@ static void finalize_deferred_config(struct wt_status *s)\n \t\t\t\t   !s->null_termination);\n \n \tif (s->null_termination) {\n-\t\tif (status_format == STATUS_FORMAT_NONE ||\n+\t\tif (status_format == STATUS_FORMAT_DEFAULT ||\n \t\t    status_format == STATUS_FORMAT_UNSPECIFIED)\n \t\t\tstatus_format = STATUS_FORMAT_PORCELAIN;\n \t\telse if (status_format == STATUS_FORMAT_LONG)\n@@ -1038,7 +1038,7 @@ static void finalize_deferred_config(struct wt_status *s)\n \tif (use_deferred_config && status_format == STATUS_FORMAT_UNSPECIFIED)\n \t\tstatus_format = status_deferred_config.status_format;\n \tif (status_format == STATUS_FORMAT_UNSPECIFIED)\n-\t\tstatus_format = STATUS_FORMAT_NONE;\n+\t\tstatus_format = STATUS_FORMAT_DEFAULT;\n \n \tif (use_deferred_config && s->show_branch < 0)\n \t\ts->show_branch = status_deferred_config.show_branch;\n@@ -1141,7 +1141,7 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \tif (all && argc > 0)\n \t\tdie(_(\"Paths with -a does not make sense.\"));\n \n-\tif (status_format != STATUS_FORMAT_NONE)\n+\tif (status_format != STATUS_FORMAT_DEFAULT)\n \t\tdry_run = 1;\n \n \treturn argc;\n@@ -1197,7 +1197,7 @@ static int git_status_config(const char *k, const char *v, void *cb)\n \t\tif (git_config_bool(k, v))\n \t\t\tstatus_deferred_config.status_format = STATUS_FORMAT_SHORT;\n \t\telse\n-\t\t\tstatus_deferred_config.status_format = STATUS_FORMAT_NONE;\n+\t\t\tstatus_deferred_config.status_format = STATUS_FORMAT_DEFAULT;\n \t\treturn 0;\n \t}\n \tif (!strcmp(k, \"status.branch\")) {\n@@ -1314,7 +1314,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \tcase STATUS_FORMAT_UNSPECIFIED:\n \t\tdie(\"BUG: finalize_deferred_config() should have been called\");\n \t\tbreak;\n-\tcase STATUS_FORMAT_NONE:\n+\tcase STATUS_FORMAT_DEFAULT:\n \tcase STATUS_FORMAT_LONG:\n \t\ts.verbose = verbose;\n \t\ts.ignore_submodule_arg = ignore_submodule_arg;\n@@ -1522,7 +1522,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(builtin_commit_usage, builtin_commit_options);\n \n \tstatus_init_config(&s, git_commit_config);\n-\tstatus_format = STATUS_FORMAT_NONE; /* Ignore status.short */\n+\tstatus_format = STATUS_FORMAT_DEFAULT; /* Ignore status.short */\n \ts.colopts = 0;\n \n \tif (get_sha1(\"HEAD\", sha1))\n-- \n1.9.0.291.g027825b\n"},{"id":"235152","messageId":"1393009762-31133-3-git-send-email-rctay89@gmail.com","threadId":"35928","inReplyTo":"1393009762-31133-2-git-send-email-rctay89@gmail.com","subject":"[PATCH 2/3] extract setting of wt_status.commitable flag out of wt_status_print_updated()","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2014-02-21T19:09:21Z","receivedAt":"2014-02-21T19:09:21Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"At first glance, wt_status_print_updated() appears to be doing a bunch\nof printing (as its name implies), and it may not be immediately obvious\nthat it also sets the vital wt_status.commitable flag. Extract this out\ninto a separate function; it is hoped that the improved clarity to\nfuture Git contributors would outweigh the performance penalty.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n wt-status.c | 17 ++++++++++++++++-\n 1 file changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex a452407..9b0189c 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -589,6 +589,21 @@ void wt_status_collect(struct wt_status *s)\n \twt_status_collect_untracked(s);\n }\n \n+void wt_status_mark_commitable(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->index_status ||\n+\t\t    d->index_status == DIFF_STATUS_UNMERGED)\n+\t\t\tcontinue;\n+\t\ts->commitable = 1;\n+\t\tbreak;\n+\t}\n+}\n+\n static void wt_status_print_unmerged(struct wt_status *s)\n {\n \tint shown_header = 0;\n@@ -627,7 +642,6 @@ static void wt_status_print_updated(struct wt_status *s)\n \t\t\tcontinue;\n \t\tif (!shown_header) {\n \t\t\twt_status_print_cached_header(s);\n-\t\t\ts->commitable = 1;\n \t\t\tshown_header = 1;\n \t\t}\n \t\twt_status_print_change_data(s, WT_STATUS_UPDATED, it);\n@@ -1309,6 +1323,7 @@ void wt_status_print(struct wt_status *s)\n \t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"\");\n \t}\n \n+\twt_status_mark_commitable(s);\n \twt_status_print_updated(s);\n \twt_status_print_unmerged(s);\n \twt_status_print_changed(s);\n-- \n1.9.0.291.g027825b\n"},{"id":"235153","messageId":"1393009762-31133-4-git-send-email-rctay89@gmail.com","threadId":"35928","inReplyTo":"1393009762-31133-3-git-send-email-rctay89@gmail.com","subject":"[PATCH 3/3] make commit --verbose work with --no-status","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2014-02-21T19:09:22Z","receivedAt":"2014-02-21T19:09:22Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"One would expect 'git commit --verbose --no-status' to give a commit\nmessage with a diff of the commit, sans the output of git-status.\nHowever, this does not work currently; the commit message body is\nentirely empty (diff is absent as well). This is because internally the\nstatus machinery is used to provide both the diff and status output, but\nit is skipped due to --no-status.\n\nWe introduce a new status_format, STATUS_FORMAT_NONE. Thus, we still\ncall run_status(), but produce nothing in place of the usual git-status\noutput. status_format is set only when git-commit is passed i)\n--verbose, and ii) --no-status.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n builtin/commit.c | 14 +++++++++++++-\n wt-status.c      |  2 +-\n wt-status.h      |  3 +++\n 3 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 2e86b76..fca6a6b 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -126,6 +126,7 @@ static struct strbuf message = STRBUF_INIT;\n \n static enum status_format {\n \tSTATUS_FORMAT_DEFAULT = 0,\n+\tSTATUS_FORMAT_NONE,\n \tSTATUS_FORMAT_LONG,\n \tSTATUS_FORMAT_SHORT,\n \tSTATUS_FORMAT_PORCELAIN,\n@@ -478,6 +479,10 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \twt_status_collect(s);\n \n \tswitch (status_format) {\n+\tcase STATUS_FORMAT_NONE:\n+\t\twt_status_mark_commitable(s);\n+\t\twt_status_print_verbose(s);\n+\t\tbreak;\n \tcase STATUS_FORMAT_SHORT:\n \t\twt_shortstatus_print(s);\n \t\tbreak;\n@@ -1141,7 +1146,12 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \tif (all && argc > 0)\n \t\tdie(_(\"Paths with -a does not make sense.\"));\n \n-\tif (status_format != STATUS_FORMAT_DEFAULT)\n+\tif (verbose && !include_status) {\n+\t\tinclude_status = 1;\n+\t\tstatus_format = STATUS_FORMAT_NONE;\n+\t}\n+\n+\tif (status_format != STATUS_FORMAT_DEFAULT && !verbose)\n \t\tdry_run = 1;\n \n \treturn argc;\n@@ -1305,6 +1315,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \t\ts.prefix = prefix;\n \n \tswitch (status_format) {\n+\tcase STATUS_FORMAT_NONE:\n+\t\tbreak;\n \tcase STATUS_FORMAT_SHORT:\n \t\twt_shortstatus_print(&s);\n \t\tbreak;\ndiff --git a/wt-status.c b/wt-status.c\nindex 9b0189c..e90fd0f 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -822,7 +822,7 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\n \tstrbuf_release(&pattern);\n }\n \n-static void wt_status_print_verbose(struct wt_status *s)\n+void wt_status_print_verbose(struct wt_status *s)\n {\n \tstruct rev_info rev;\n \tstruct setup_revision_opt opt;\ndiff --git a/wt-status.h b/wt-status.h\nindex 30a4812..5c89f23 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -95,8 +95,11 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *);\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_mark_commitable(struct wt_status *state);\n void wt_status_get_state(struct wt_status_state *state, int get_detached_from);\n \n+void wt_status_print_verbose(struct wt_status *s);\n+\n void wt_shortstatus_print(struct wt_status *s);\n void wt_porcelain_print(struct wt_status *s);\n \n-- \n1.9.0.291.g027825b\n"},{"id":"235169","messageId":"20140222082418.GD1576@sigill.intra.peff.net","threadId":"35928","inReplyTo":"1393009762-31133-2-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 1/3] rename STATUS_FORMAT_NONE to STATUS_FORMAT_DEFAULT","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-22T08:24:19Z","receivedAt":"2014-02-22T08:24:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 22, 2014 at 03:09:20AM +0800, Tay Ray Chuan wrote:\n\n> In f3f47a1 (status: add --long output format option), STATUS_FORMAT_NONE\n> was introduced, meaning \"the user did not specify anything\". Rename this\n> to *_DEFAULT to better indicate its meaning.\n\nHmm. We later introduced STATUS_FORMAT_UNSPECIFIED in 84b4202d. It seems\nlike that is the same thing as the _DEFAULT you are proposing here. Can\nwe collapse them into a single value?\n\n-Peff\n"},{"id":"235170","messageId":"20140222083110.GE1576@sigill.intra.peff.net","threadId":"35928","inReplyTo":"1393009762-31133-4-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 3/3] make commit --verbose work with --no-status","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-22T08:31:10Z","receivedAt":"2014-02-22T08:31:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 22, 2014 at 03:09:22AM +0800, Tay Ray Chuan wrote:\n\n> @@ -1141,7 +1146,12 @@ static int parse_and_validate_options(int argc, const char *argv[],\n>  \tif (all && argc > 0)\n>  \t\tdie(_(\"Paths with -a does not make sense.\"));\n>  \n> -\tif (status_format != STATUS_FORMAT_DEFAULT)\n> +\tif (verbose && !include_status) {\n> +\t\tinclude_status = 1;\n> +\t\tstatus_format = STATUS_FORMAT_NONE;\n> +\t}\n> +\n> +\tif (status_format != STATUS_FORMAT_DEFAULT && !verbose)\n>  \t\tdry_run = 1;\n\nWhat happens here when there is an alternate status format _and_\n--verbose is used? If I say \"git commit --porcelain\" it should imply\n--dry-run. But \"git commit --porcelain --verbose\" no longer does so\nafter your patch.\n\n-Peff\n"},{"id":"235218","messageId":"CALUzUxrO-=a5u-NpPVcQdnc2sp9dq3St0PZm=QOWr7oMWDz-Jw@mail.gmail.com","threadId":"35928","inReplyTo":"20140222083110.GE1576@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] make commit --verbose work with --no-status","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2014-02-24T04:24:42Z","receivedAt":"2014-02-24T04:24:42Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Sat, Feb 22, 2014 at 4:31 PM, Jeff King <peff@peff.net> wrote:\n> On Sat, Feb 22, 2014 at 03:09:22AM +0800, Tay Ray Chuan wrote:\n>\n>> @@ -1141,7 +1146,12 @@ static int parse_and_validate_options(int argc, const char *argv[],\n>>       if (all && argc > 0)\n>>               die(_(\"Paths with -a does not make sense.\"));\n>>\n>> -     if (status_format != STATUS_FORMAT_DEFAULT)\n>> +     if (verbose && !include_status) {\n>> +             include_status = 1;\n>> +             status_format = STATUS_FORMAT_NONE;\n>> +     }\n>> +\n>> +     if (status_format != STATUS_FORMAT_DEFAULT && !verbose)\n>>               dry_run = 1;\n>\n> What happens here when there is an alternate status format _and_\n> --verbose is used? If I say \"git commit --porcelain\" it should imply\n> --dry-run. But \"git commit --porcelain --verbose\" no longer does so\n> after your patch.\n\nI should have just left the --dry-run inference alone, like this.\n\n-->8--\n\n@@ -1141,7 +1146,12 @@ static int parse_and_validate_options\n        if (all && argc > 0)\n                die(_(\"Paths with -a does not make sense.\"));\n\n-       if (status_format != STATUS_FORMAT_DEFAULT)\n+       if (verbose && !include_status) {\n+               include_status = 1;\n+               status_format = STATUS_FORMAT_NONE;\n+       } else if (status_format != STATUS_FORMAT_DEFAULT)\n                dry_run = 1;\n\n        return argc;\n"},{"id":"235229","messageId":"20140224083312.GB32594@sigill.intra.peff.net","threadId":"35928","inReplyTo":"CALUzUxrO-=a5u-NpPVcQdnc2sp9dq3St0PZm=QOWr7oMWDz-Jw@mail.gmail.com","subject":"Re: [PATCH 3/3] make commit --verbose work with --no-status","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-24T08:33:12Z","receivedAt":"2014-02-24T08:33:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 24, 2014 at 12:24:42PM +0800, Tay Ray Chuan wrote:\n\n> > What happens here when there is an alternate status format _and_\n> > --verbose is used? If I say \"git commit --porcelain\" it should imply\n> > --dry-run. But \"git commit --porcelain --verbose\" no longer does so\n> > after your patch.\n> \n> I should have just left the --dry-run inference alone, like this.\n> \n> -->8--\n> \n> @@ -1141,7 +1146,12 @@ static int parse_and_validate_options\n>         if (all && argc > 0)\n>                 die(_(\"Paths with -a does not make sense.\"));\n> \n> -       if (status_format != STATUS_FORMAT_DEFAULT)\n> +       if (verbose && !include_status) {\n> +               include_status = 1;\n> +               status_format = STATUS_FORMAT_NONE;\n> +       } else if (status_format != STATUS_FORMAT_DEFAULT)\n>                 dry_run = 1;\n> \n>         return argc;\n\nHrm, not quite, because the way include_status works is weird. If I turn\nit off in the config, like this:\n\n  git config commit.status false\n\nthen asking explicitly for a status format should still dry-run and show\nit:\n\n  git commit --porcelain\n\nIOW, include_status is only respected when we are generating the actual\ncommit. So I think you need something more like:\n\n  if (status_format == STATUS_FORMAT_DEFAULT) {\n          if (verbose && !include_status) {\n                  include_status = 1;\n                  status_format = STATUS_FORMAT_NONE;\n          }\n  } else\n          dry_run = 1;\n\n-Peff\n"}]}