{"thread":{"id":"16250","subject":"git commit -v does not removes the patch","startedAt":"2008-11-10T15:20:51Z","lastAt":"2008-11-22T22:43:30Z","messageCount":22,"participants":["Santi Béjar","Jeff King","Junio C Hamano","SZEDER Gábor"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"95338","messageId":"adf1fd3d0811100720n52ac1d47id9b7f402412aa0d3@mail.gmail.com","threadId":"16250","inReplyTo":null,"subject":"git commit -v does not removes the patch","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2008-11-10T15:20:51Z","receivedAt":"2008-11-10T15:20:51Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"Hi *,\n\n $subject since:\n\n4f672ad (wt-status: load diff ui config, 2008-10-26)\n\nI tried to make a test case, but failed. I think because it is a bit\ntricky the fake_editor/stdin/stdout stuff, so at the end I bisected it\nby hand\n\nRegards,\nSanti\n"},{"id":"95352","messageId":"20081110181023.GA22753@coredump.intra.peff.net","threadId":"16250","inReplyTo":"adf1fd3d0811100720n52ac1d47id9b7f402412aa0d3@mail.gmail.com","subject":"Re: git commit -v does not removes the patch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-10T18:10:24Z","receivedAt":"2008-11-10T18:10:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 10, 2008 at 04:20:51PM +0100, Santi Béjar wrote:\n\n> Hi *,\n> \n> $subject since:\n> \n> 4f672ad (wt-status: load diff ui config, 2008-10-26)\n> \n> I tried to make a test case, but failed. I think because it is a bit\n> tricky the fake_editor/stdin/stdout stuff, so at the end I bisected it\n> by hand\n\nSorry, I don't quite understand what the problem is. From reading your\nsubject line, I expected that \"git commit -v\" would show the diff in\nyour editor, but then accidentally also include it in the final commit\nmessage. But I can't seem to reproduce that.\n\nCan you describe the problem in more detail?\n\n-Peff\n"},{"id":"95395","messageId":"adf1fd3d0811101434j658b2e8aj83d8cbe2293f5021@mail.gmail.com","threadId":"16250","inReplyTo":"20081110181023.GA22753@coredump.intra.peff.net","subject":"Re: git commit -v does not removes the patch","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2008-11-10T22:34:51Z","receivedAt":"2008-11-10T22:34:51Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Mon, Nov 10, 2008 at 7:10 PM, Jeff King wrote:\n> On Mon, Nov 10, 2008 at 04:20:51PM +0100, Santi Béjar wrote:\n>\n>> Hi *,\n>>\n>> $subject since:\n>>\n>> 4f672ad (wt-status: load diff ui config, 2008-10-26)\n>>\n>> I tried to make a test case, but failed. I think because it is a bit\n>> tricky the fake_editor/stdin/stdout stuff, so at the end I bisected it\n>> by hand\n>\n> Sorry, I don't quite understand what the problem is. From reading your\n> subject line, I expected that \"git commit -v\" would show the diff in\n> your editor, but then accidentally also include it in the final commit\n> message. But I can't seem to reproduce that.\n>\n> Can you describe the problem in more detail?\n\nIt is exactly as you described. I'll try in other systems.\n\nSanti\n"},{"id":"95401","messageId":"7vej1j40x5.fsf@gitster.siamese.dyndns.org","threadId":"16250","inReplyTo":"adf1fd3d0811101434j658b2e8aj83d8cbe2293f5021@mail.gmail.com","subject":"Re: git commit -v does not removes the patch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-10T23:27:18Z","receivedAt":"2008-11-10T23:27:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Santi Béjar\" writes:\n\n> On Mon, Nov 10, 2008 at 7:10 PM, Jeff King wrote:\n>> On Mon, Nov 10, 2008 at 04:20:51PM +0100, Santi Béjar wrote:\n>>\n>>> Hi *,\n>>>\n>>> $subject since:\n>>>\n>>> 4f672ad (wt-status: load diff ui config, 2008-10-26)\n>>>\n>>> I tried to make a test case, but failed. I think because it is a bit\n>>> tricky the fake_editor/stdin/stdout stuff, so at the end I bisected it\n>>> by hand\n>>\n>> Sorry, I don't quite understand what the problem is. From reading your\n>> subject line, I expected that \"git commit -v\" would show the diff in\n>> your editor, but then accidentally also include it in the final commit\n>> message. But I can't seem to reproduce that.\n>>\n>> Can you describe the problem in more detail?\n>\n> It is exactly as you described. I'll try in other systems.\n\nGuess in the dark... by any chance are you enabling color unconditionally?\n"},{"id":"95416","messageId":"20081111000706.GA26223@coredump.intra.peff.net","threadId":"16250","inReplyTo":"7vej1j40x5.fsf@gitster.siamese.dyndns.org","subject":"Re: git commit -v does not removes the patch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-11T00:07:06Z","receivedAt":"2008-11-11T00:07:06Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 10, 2008 at 03:27:18PM -0800, Junio C Hamano wrote:\n\n> > It is exactly as you described. I'll try in other systems.\n> \n> Guess in the dark... by any chance are you enabling color unconditionally?\n\nI thought I covered that case explicitly...\n\n...yes, I can't reproduce with color set to \"always\". And indeed,\nlooking at the diff for 4f672ad shows that we always turn off color for\nthe message that goes to the editor (since we can't trust our isatty\nsetting _anyway_, since we're not going to the tty).\n\nSo that's not it.\n\n-Peff\n"},{"id":"95441","messageId":"adf1fd3d0811102356u6e671dcfj6491f81cf462ec2e@mail.gmail.com","threadId":"16250","inReplyTo":"20081111000706.GA26223@coredump.intra.peff.net","subject":"Re: git commit -v does not removes the patch","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2008-11-11T07:56:34Z","receivedAt":"2008-11-11T07:56:34Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Tue, Nov 11, 2008 at 1:07 AM, Jeff King wrote:\n> On Mon, Nov 10, 2008 at 03:27:18PM -0800, Junio C Hamano wrote:\n>\n>> > It is exactly as you described. I'll try in other systems.\n>>\n>> Guess in the dark... by any chance are you enabling color unconditionally?\n>\n> I thought I covered that case explicitly...\n>\n> ...yes, I can't reproduce with color set to \"always\". And indeed,\n> looking at the diff for 4f672ad shows that we always turn off color for\n> the message that goes to the editor (since we can't trust our isatty\n> setting _anyway_, since we're not going to the tty).\n>\n> So that's not it.\n\nAlmost! I have diff.mnemonicprefix=true, if I unset it everything works.\n\nSanti\n"},{"id":"95448","messageId":"20081111102914.GA30330@coredump.intra.peff.net","threadId":"16250","inReplyTo":"adf1fd3d0811102356u6e671dcfj6491f81cf462ec2e@mail.gmail.com","subject":"Re: git commit -v does not removes the patch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-11T10:29:15Z","receivedAt":"2008-11-11T10:29:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 11, 2008 at 08:56:34AM +0100, Santi Béjar wrote:\n\n> Almost! I have diff.mnemonicprefix=true, if I unset it everything works.\n\nAh, indeed. The obvious fix is just loosening our match a little bit:\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 93ca496..a721990 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -1015,7 +1015,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* Truncate the message just before the diff, if any. */\n-\tp = strstr(sb.buf, \"\\ndiff --git a/\");\n+\tp = strstr(sb.buf, \"\\ndiff --git \");\n \tif (p != NULL)\n \t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n \n\nBut I have to wonder if there is some more robust solution. It seems\nlike this can have false positives if you include diff output in your\ncommit message, and a potential false negative if you delete the newline\n(e.g., delete everything up to \"diff --git\", making it the first line).\n\nBut I guess we haven't seen a lot of complaints, so maybe those\nconditions aren't worth worrying about.\n\n-Peff\n"},{"id":"95453","messageId":"adf1fd3d0811110320w53f9745bs68a6bd1b2e437f42@mail.gmail.com","threadId":"16250","inReplyTo":"20081111102914.GA30330@coredump.intra.peff.net","subject":"Re: git commit -v does not removes the patch","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2008-11-11T11:20:52Z","receivedAt":"2008-11-11T11:20:52Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Tue, Nov 11, 2008 at 11:29 AM, Jeff King wrote:\n> On Tue, Nov 11, 2008 at 08:56:34AM +0100, Santi Béjar wrote:\n>\n>> Almost! I have diff.mnemonicprefix=true, if I unset it everything works.\n>\n> Ah, indeed. The obvious fix is just loosening our match a little bit:\n>\n> diff --git a/builtin-commit.c b/builtin-commit.c\n> index 93ca496..a721990 100644\n> --- a/builtin-commit.c\n> +++ b/builtin-commit.c\n> @@ -1015,7 +1015,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n> }\n>\n> /* Truncate the message just before the diff, if any. */\n> - p = strstr(sb.buf, \"\\ndiff --git a/\");\n> + p = strstr(sb.buf, \"\\ndiff --git \");\n> if (p != NULL)\n> strbuf_setlen(&sb, p - sb.buf + 1);\n>\n\nIt fixes it, thanks.\n\n>\n> But I have to wonder if there is some more robust solution. It seems\n> like this can have false positives if you include diff output in your\n> commit message, and a potential false negative if you delete the newline\n> (e.g., delete everything up to \"diff --git\", making it the first line).\n>\n> But I guess we haven't seen a lot of complaints, so maybe those\n> conditions aren't worth worrying about.\n\nThe false positive/negative were already possible, it was just a\nlittle more narrow. So not worth worrying about.\nAlso, if you really want a diff in the commit message you can use the\n--cleanup option.\n\nA more robust solution could be to have a:\n\n# Everything after this line will be removed from the commit message\n\nBut, again, not worth.\n\nSanti\n\n\n>\n> -Peff\n>\n"},{"id":"95468","messageId":"7v4p2e2nkg.fsf@gitster.siamese.dyndns.org","threadId":"16250","inReplyTo":"20081111102914.GA30330@coredump.intra.peff.net","subject":"Re: git commit -v does not removes the patch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-11T17:13:18Z","receivedAt":"2008-11-11T17:13:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> But I have to wonder if there is some more robust solution. It seems\n> like this can have false positives if you include diff output in your\n> commit message, and a potential false negative if you delete the newline\n> (e.g., delete everything up to \"diff --git\", making it the first line).\n\nActually, I recall (from my googling quite some time ago) people noticing\nthis, but they learnt to live with it.\n\nWe may want to change this. We can say \"# Everything under this line is\ndeleted.\" at the beginning of the \"#\" block we produce in the commit log\nmessage editor, replacing the \"Lines starting with '#' will be ignored, \"\nwe currently have. When reading back the editor result, make \"git commit\n-v\" scan for the \"# Everything ...\" line. We remove it and everything\nthat follows, but we do not touch anything above that line (including the\nones that begin with \"diff\" or \"#\") except the usual trailing whitespace\nremoval. That way, people can leave a sample shell session with root\nprompt, and sample diff, in their message.\n\nIf we do not see \"# Everything ...\" when we read it back, we can do what\nwe currently do as a fallback.\n"},{"id":"95533","messageId":"20081112081609.GA3720@coredump.intra.peff.net","threadId":"16250","inReplyTo":"7v4p2e2nkg.fsf@gitster.siamese.dyndns.org","subject":"Re: git commit -v does not removes the patch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-12T08:16:09Z","receivedAt":"2008-11-12T08:16:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 11, 2008 at 09:13:18AM -0800, Junio C Hamano wrote:\n\n> We may want to change this. We can say \"# Everything under this line is\n> deleted.\" at the beginning of the \"#\" block we produce in the commit log\n> message editor, replacing the \"Lines starting with '#' will be ignored, \"\n> we currently have. When reading back the editor result, make \"git commit\n> -v\" scan for the \"# Everything ...\" line. We remove it and everything\n> that follows, but we do not touch anything above that line (including the\n> ones that begin with \"diff\" or \"#\") except the usual trailing whitespace\n> removal. That way, people can leave a sample shell session with root\n> prompt, and sample diff, in their message.\n>\n> If we do not see \"# Everything ...\" when we read it back, we can do what\n> we currently do as a fallback.\n\nI am little hesitant to do this, because I think people have scripted\nminorly around the template format (at the very least, for syntax\nhighlighting). I think some people may have pre-written templates, as\nwell, though I guess your \"If we do not see...\" paragraph is meant to\naddress that. Though that brings some confusion itself, because now the\nparsing rules for what is kept change if I delete that line.\n\nHere's a patch series that at least improves the situation by turning\noff the diff-stripping if we never put in a diff in the first place.\nThat way only people who actually _use_ \"-v\" will have to pay for it.\nIt has the fix I sent to Santi earlier, as well as some related\ncleanups.\n\n 1/5: define empty tree sha1 as a macro\n 2/5: wt-status: refactor initial commit printing\n 3/5: status: show \"-v\" diff even for initial commit\n 4/5: commit: loosen pattern for matching \"-v\" diff\n 5/5: commit: only strip diff from message in verbose mode\n\n-Peff\n"},{"id":"95534","messageId":"20081112081752.GA3751@coredump.intra.peff.net","threadId":"16250","inReplyTo":"20081112081609.GA3720@coredump.intra.peff.net","subject":"[PATCH 1/5] define empty tree sha1 as a macro","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-12T08:17:52Z","receivedAt":"2008-11-12T08:17:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This can potentially be used in a few places, so let's make\nit available to all parts of the code.\n\nSigned-off-by: Jeff King \n---\nI use the _HEX version in further patches, but since it is such a magic\nnumber, I thought it made sense to keep all definitions in the same\nplace.\n\n cache.h | 6 ++++++\n sha1_file.c | 4 +---\n 2 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex bcc57ba..46eb2af 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -528,6 +528,12 @@ static inline void hashclr(unsigned char *hash)\n }\n extern int is_empty_blob_sha1(const unsigned char *sha1);\n \n+#define EMPTY_TREE_SHA1_HEX \\\n+\t\"4b825dc642cb6eb9a060e54bf8d69288fbee4904\"\n+#define EMPTY_TREE_SHA1_BIN \\\n+\t \"\\x4b\\x82\\x5d\\xc6\\x42\\xcb\\x6e\\xb9\\xa0\\x60\" \\\n+\t \"\\xe5\\x4b\\xf8\\xd6\\x92\\x88\\xfb\\xee\\x49\\x04\"\n+\n int git_mkstemp(char *path, size_t n, const char *template);\n \n /*\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 654d039..037e439 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2056,9 +2056,7 @@ static struct cached_object {\n static int cached_object_nr, cached_object_alloc;\n \n static struct cached_object empty_tree = {\n-\t/* empty tree sha1: 4b825dc642cb6eb9a060e54bf8d69288fbee4904 */\n-\t\"\\x4b\\x82\\x5d\\xc6\\x42\\xcb\\x6e\\xb9\\xa0\\x60\"\n-\t\"\\xe5\\x4b\\xf8\\xd6\\x92\\x88\\xfb\\xee\\x49\\x04\",\n+\tEMPTY_TREE_SHA1_BIN,\n \tOBJ_TREE,\n \t\"\",\n \t0\n-- \n1.6.0.4.883.g4593ee.dirty\n"},{"id":"95535","messageId":"20081112082138.GB3751@coredump.intra.peff.net","threadId":"16250","inReplyTo":"20081112081609.GA3720@coredump.intra.peff.net","subject":"[PATCH 2/5] wt-status: refactor initial commit printing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-12T08:21:39Z","receivedAt":"2008-11-12T08:21:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we showed the initial commit, we had no reference to\ndiff against, so we went through the cache manually.\nNowadays, however, we have a virtual empty tree commit, so\nwe can simply diff against that to get the same results.\n\nSigned-off-by: Jeff King \n---\nI ran across this when I realized I could do the same trick for the\nverbose diff (which I will do in the next patch). I think it is a\nworthwhile cleanup. Not only does it remove a lot of lines, but it gives\na single codepath for printing, which will be helpful if that codepath\never changes (e.g., with the alternate status formats we talked about a\nfew weeks ago).\n\nThis could be made even simpler by setting s->reference to the empty\ntree when we are on the initial commit, but I think that is a little\nmessy. The caller may have set s->reference to an arbitrary pointer,\nincluding one which needs free()d, and we would be overwriting that.\nAs it happens, with the current callers there is no problem, but it\nseems like a bad interface.\n\n wt-status.c | 28 +++-------------------------\n 1 files changed, 3 insertions(+), 25 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 6a7645e..c78588e 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -185,31 +185,12 @@ static void wt_status_print_changed_cb(struct diff_queue_struct *q,\n \t\twt_status_print_trailer(s);\n }\n \n-static void wt_status_print_initial(struct wt_status *s)\n-{\n-\tint i;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\n-\tif (active_nr) {\n-\t\ts->commitable = 1;\n-\t\twt_status_print_cached_header(s);\n-\t}\n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tcolor_fprintf(s->fp, color(WT_STATUS_HEADER), \"#\\t\");\n-\t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_UPDATED), \"new file: %s\",\n-\t\t\t\tquote_path(active_cache[i]->name, -1,\n-\t\t\t\t\t &buf, s->prefix));\n-\t}\n-\tif (active_nr)\n-\t\twt_status_print_trailer(s);\n-\tstrbuf_release(&buf);\n-}\n-\n static void wt_status_print_updated(struct wt_status *s)\n {\n \tstruct rev_info rev;\n \tinit_revisions(&rev, NULL);\n-\tsetup_revisions(0, NULL, &rev, s->reference);\n+\tsetup_revisions(0, NULL, &rev,\n+\t\ts->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference);\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = wt_status_print_updated_cb;\n \trev.diffopt.format_callback_data = s;\n@@ -360,12 +341,9 @@ void wt_status_print(struct wt_status *s)\n \t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER), \"#\");\n \t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER), \"# Initial commit\");\n \t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER), \"#\");\n-\t\twt_status_print_initial(s);\n-\t}\n-\telse {\n-\t\twt_status_print_updated(s);\n \t}\n \n+\twt_status_print_updated(s);\n \twt_status_print_changed(s);\n \tif (wt_status_submodule_summary)\n \t\twt_status_print_submodule_summary(s);\n-- \n1.6.0.4.883.g4593ee.dirty\n"},{"id":"95536","messageId":"20081112082337.GC3751@coredump.intra.peff.net","threadId":"16250","inReplyTo":"20081112081609.GA3720@coredump.intra.peff.net","subject":"[PATCH 3/5] status: show \"-v\" diff even for initial commit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-12T08:23:37Z","receivedAt":"2008-11-12T08:23:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since we can use the same \"diff against empty tree\" trick as\nwe do for the non-initial case, it is trivial to make this\nwork.\n\nSigned-off-by: Jeff King \n---\nI ran across this while writing my test cases for the later patches. I\ndon't see any reason why it shouldn't behave this way, and I suspect the\nreason was just that the original author saw how we did\n\"wt_status_print_initial\" and didn't want to have to do the equivalent\nfor generating diff output.\n\n t/t7507-commit-verbose.sh | 29 +++++++++++++++++++++++++++++\n wt-status.c | 5 +++--\n 2 files changed, 32 insertions(+), 2 deletions(-)\n create mode 100755 t/t7507-commit-verbose.sh\n\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nnew file mode 100755\nindex 0000000..94b12e9\n--- /dev/null\n+++ b/t/t7507-commit-verbose.sh\n@@ -0,0 +1,29 @@\n+#!/bin/sh\n+\n+test_description='verbose commit template'\n+. ./test-lib.sh\n+\n+cat >check-for-diff <message <<'EOF'\n+subject\n+\n+body\n+EOF\n+\n+test_expect_success 'setup' '\n+\techo content >file &&\n+\tgit add file &&\n+\tgit commit -F message\n+'\n+\n+test_expect_success 'initial commit shows verbose diff' '\n+\tgit commit --amend -v\n+'\n+\n+test_done\ndiff --git a/wt-status.c b/wt-status.c\nindex c78588e..3edae43 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -279,7 +279,8 @@ static void wt_status_print_verbose(struct wt_status *s)\n \tstruct rev_info rev;\n \n \tinit_revisions(&rev, NULL);\n-\tsetup_revisions(0, NULL, &rev, s->reference);\n+\tsetup_revisions(0, NULL, &rev,\n+\t\ts->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference);\n \trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n \trev.diffopt.detect_rename = 1;\n \tDIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);\n@@ -352,7 +353,7 @@ void wt_status_print(struct wt_status *s)\n \telse if (s->commitable)\n \t\t fprintf(s->fp, \"# Untracked files not listed (use -u option to show untracked files)\\n\");\n \n-\tif (s->verbose && !s->is_initial)\n+\tif (s->verbose)\n \t\twt_status_print_verbose(s);\n \tif (!s->commitable) {\n \t\tif (s->amend)\n-- \n1.6.0.4.883.g4593ee.dirty\n"},{"id":"95537","messageId":"20081112082410.GD3751@coredump.intra.peff.net","threadId":"16250","inReplyTo":"20081112081609.GA3720@coredump.intra.peff.net","subject":"[PATCH 4/5] commit: loosen pattern for matching \"-v\" diff","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-12T08:24:10Z","receivedAt":"2008-11-12T08:24:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When a user asks to see the diff to be applied via \"git\ncommit -v\", the diff ends up in the commit template, and we\nmust later remove it.\n\nTo detect the start of the included diff, we used to search\nfor a line beginning with \"diff --git a/\". However, in the\nface of diff.mnemonicprefix, that will actually be \"diff\n--git i/\".\n\nSo let's just loosen the pattern a bit to handle either\ncase.\n\nSigned-off-by: Jeff King \n---\nAnd this is the fix from before, with a test case.\n\n builtin-commit.c | 2 +-\n t/t7507-commit-verbose.sh | 16 ++++++++++++++++\n 2 files changed, 17 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 93ca496..a721990 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -1015,7 +1015,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* Truncate the message just before the diff, if any. */\n-\tp = strstr(sb.buf, \"\\ndiff --git a/\");\n+\tp = strstr(sb.buf, \"\\ndiff --git \");\n \tif (p != NULL)\n \t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n \ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex 94b12e9..be70166 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -26,4 +26,20 @@ test_expect_success 'initial commit shows verbose diff' '\n \tgit commit --amend -v\n '\n \n+check_message() {\n+\tgit log -1 --pretty=format:%s%n%n%b >actual &&\n+\ttest_cmp \"$1\" actual\n+}\n+\n+test_expect_success 'verbose diff is stripped out' '\n+\tgit commit --amend -v &&\n+\tcheck_message message\n+'\n+\n+test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '\n+\tgit config diff.mnemonicprefix true &&\n+\tgit commit --amend -v &&\n+\tcheck_message message\n+'\n+\n test_done\n-- \n1.6.0.4.883.g4593ee.dirty\n"},{"id":"95538","messageId":"20081112082552.GE3751@coredump.intra.peff.net","threadId":"16250","inReplyTo":"20081112081609.GA3720@coredump.intra.peff.net","subject":"[PATCH 5/5] commit: only strip diff from message in verbose mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-12T08:25:52Z","receivedAt":"2008-11-12T08:25:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When the \"-v\" option is given, we put diff of what is to be\ncommitted into the commit template, and then strip it back\nout again after the user has edited it.\n\nWe guess at the location of the diff by searching for the\n\"diff --git\" header. This means that if the user puts their\nown diff in the message (e.g., as a sample output), then we\nwill accidentally trigger the pattern, removing part of\ntheir output.\n\nWe can avoid doing this stripping altogether if the user\ndidn't use \"-v\" in the first place, so we know that any\nmatch we find will be a false positive.\n\nSigned-off-by: Jeff King \n---\nObviously this doesn't solve the problem entirely, but it at least makes\na reasonable rule: if we accidentally eat your sample diff output with\n\"-v\", then try without it. :)\n\nBut more importantly, I suspect \"-v\" is used infrequently, so in\npractice this should fix most cases.\n\n builtin-commit.c | 8 +++++---\n t/t7507-commit-verbose.sh | 21 +++++++++++++++++++++\n 2 files changed, 26 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex a721990..591d16b 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -1015,9 +1015,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* Truncate the message just before the diff, if any. */\n-\tp = strstr(sb.buf, \"\\ndiff --git \");\n-\tif (p != NULL)\n-\t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n+\tif (verbose) {\n+\t\tp = strstr(sb.buf, \"\\ndiff --git \");\n+\t\tif (p != NULL)\n+\t\t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n+\t}\n \n \tif (cleanup_mode != CLEANUP_NONE)\n \t\tstripspace(&sb, cleanup_mode == CLEANUP_ALL);\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex be70166..2a3ea7a 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -42,4 +42,25 @@ test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '\n \tcheck_message message\n '\n \n+cat >diff <<'EOF'\n+This is an example commit message that contains a diff.\n+\n+diff --git c/file i/file\n+new file mode 100644\n+index 0000000..f95c11d\n+--- /dev/null\n++++ i/file\n+@@ -0,0 +1 @@\n++this is some content\n+EOF\n+\n+test_expect_success 'diff in message is retained without -v' '\n+\tgit commit --amend -F diff &&\n+\tcheck_message diff\n+'\n+\n+test_expect_failure 'diff in message is retained with -v' '\n+\tgit commit --amend -F diff -v &&\n+\tcheck_message diff\n+'\n test_done\n-- \n1.6.0.4.883.g4593ee.dirty\n"},{"id":"95540","messageId":"20081112082947.GA3817@coredump.intra.peff.net","threadId":"16250","inReplyTo":"20081112082552.GE3751@coredump.intra.peff.net","subject":"Re: [PATCH 5/5] commit: only strip diff from message in verbose mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-12T08:29:47Z","receivedAt":"2008-11-12T08:29:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 12, 2008 at 03:25:52AM -0500, Jeff King wrote:\n\n> +test_expect_success 'diff in message is retained without -v' '\n> +\tgit commit --amend -F diff &&\n> +\tcheck_message diff\n> +'\n> +\n> +test_expect_failure 'diff in message is retained with -v' '\n> +\tgit commit --amend -F diff -v &&\n> +\tcheck_message diff\n> +'\n\nI put in this expect_failure to remind us that we could be doing better.\nThe solution you proposed would fix this test. Another idea I had was to\nuse a bogus diff header, and match on that. Something like:\n\n diff --git --verbose-commit a/file b/file\n\nand searching for \"diff --git --verbose-commit\", which would not trigger\non any diffs (unless, of course, you were trying to explain how the\nverbose mode worked in your commit message :) ).\n\nBut that doesn't cover any other special input (i.e., lines beginning\nwith '#'), and would probably require an ugly hack to the diff machinery\n(some diff option for \"add these bogus diff command line options\").\n\n-Peff\n"},{"id":"95655","messageId":"7vk5b8jrqm.fsf@gitster.siamese.dyndns.org","threadId":"16250","inReplyTo":"20081112081609.GA3720@coredump.intra.peff.net","subject":"Re: git commit -v does not removes the patch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-13T02:15:45Z","receivedAt":"2008-11-13T02:15:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Here's a patch series that at least improves the situation by turning\n> off the diff-stripping if we never put in a diff in the first place.\n> That way only people who actually _use_ \"-v\" will have to pay for it.\n> It has the fix I sent to Santi earlier, as well as some related\n> cleanups.\n>\n> 1/5: define empty tree sha1 as a macro\n> 2/5: wt-status: refactor initial commit printing\n> 3/5: status: show \"-v\" diff even for initial commit\n> 4/5: commit: loosen pattern for matching \"-v\" diff\n> 5/5: commit: only strip diff from message in verbose mode\n\nMakes sense. Thanks, will queue for 'master' with minor futzing to lift\nminimum fix to 'maint'.\n"},{"id":"96233","messageId":"20081120130851.GA17608@neumann","threadId":"16250","inReplyTo":"20081112081609.GA3720@coredump.intra.peff.net","subject":"Re: git commit -v does not removes the patch","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2008-11-20T13:09:28Z","receivedAt":"2008-11-20T13:09:28Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\nOn Wed, Nov 12, 2008 at 03:16:09AM -0500, Jeff King wrote:\n> Here's a patch series that at least improves the situation by turning\n> off the diff-stripping if we never put in a diff in the first place.\n> That way only people who actually _use_ \"-v\" will have to pay for it.\n\nI always want to see the diff while editing a commit message, even if\nit's the commit message of a merge, a revert, a squash in rebase.\nSince these commands do not have an option equivalent to 'git commit\n-v', I wrote a little prepare-commit-msg hook to always append the\nappropriate diff to the end of the appropriate commit message file.\nThis worked pretty well for the last couple of months, until 5/5 got\ninto maint.\n\nSo, what is the/is there a preferred way to always include the diff in\nthe commit message template and get it removed automatically? Are\nthere any workarounds other than revert that commit locally?\n\nThanks,\nGábor\n"},{"id":"96263","messageId":"20081120152015.GA6283@coredump.intra.peff.net","threadId":"16250","inReplyTo":"20081120130851.GA17608@neumann","subject":"Re: git commit -v does not removes the patch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-20T15:20:16Z","receivedAt":"2008-11-20T15:20:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 20, 2008 at 02:09:28PM +0100, SZEDER Gábor wrote:\n\n> I always want to see the diff while editing a commit message, even if\n> it's the commit message of a merge, a revert, a squash in rebase.\n> Since these commands do not have an option equivalent to 'git commit\n> -v', I wrote a little prepare-commit-msg hook to always append the\n> appropriate diff to the end of the appropriate commit message file.\n> This worked pretty well for the last couple of months, until 5/5 got\n> into maint.\n\nHmm. I am sad that this change has broken somebody's existing workflow.\nOTOH, I'm not convinced that workflow wasn't a little crazy to begin\nwith, depending on this undocumented munging.\n\n> So, what is the/is there a preferred way to always include the diff in\n> the commit message template and get it removed automatically? Are\n> there any workarounds other than revert that commit locally?\n\nHow about:\n\n $ cat .git/hooks/prepare-commit-msg\n #!/bin/sh\n git diff --cached >>\"$1\"\n\n $ cat .git/hooks/commit-msg\n #!/bin/sh\n sed -i '/^diff --git/Q' \"$1\"\n\nwhich is more or less the original behavior. The only downside I see is\nthat \"--no-verify\" will turn off the commit-msg hook, but not the\nprepare-commit-msg hook.\n\n-Peff\n"},{"id":"96365","messageId":"20081122155541.GC6885@neumann","threadId":"16250","inReplyTo":"20081120152015.GA6283@coredump.intra.peff.net","subject":"Re: git commit -v does not removes the patch","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2008-11-22T15:55:41Z","receivedAt":"2008-11-22T15:55:41Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi Jeff,\n\nOn Thu, Nov 20, 2008 at 10:20:16AM -0500, Jeff King wrote:\n> On Thu, Nov 20, 2008 at 02:09:28PM +0100, SZEDER Gábor wrote:\n\n> Hmm. I am sad that this change has broken somebody's existing workflow.\n> OTOH, I'm not convinced that workflow wasn't a little crazy to begin\n> with, depending on this undocumented munging.\n\nWell, the docs didn't say that the patch is only removed in verbose\nmode (; But I agree with you that this is (was) an undocumented\n\"feature\" that I should not have relied upon.\n\n> > So, what is the/is there a preferred way to always include the diff in\n> > the commit message template and get it removed automatically? Are\n> > there any workarounds other than revert that commit locally?\n> \n> How about:\n> \n> $ cat .git/hooks/prepare-commit-msg\n> #!/bin/sh\n> git diff --cached >>\"$1\"\n> \n> $ cat .git/hooks/commit-msg\n> #!/bin/sh\n> sed -i '/^diff --git/Q' \"$1\"\n> \n> which is more or less the original behavior. The only downside I see is\n> that \"--no-verify\" will turn off the commit-msg hook, but not the\n> prepare-commit-msg hook.\n\nThanks for the tip. Junio's suggestion about '# Everything under this\nline is deleted.' could be implemented this way fairly easily (just an\nadditional echo in prepare-commit-msg, and other pattern in\ncommit-msg). But 'git commit --no-verify' is indeed a problem.\nAlthough I never use it explicitly, 'git rebase -i' does so.\n\nHowever.\n\n'git rebase -i' uses 'git commit --no-verify' since c5b09feb (Avoid\nupdate hook during git-rebase --interactive, 2007-12-19) to avoid\nsquashing commits together when the pre-commit hook fails because of\nwhitespace errors. But note that at that time the 'git commit'\ncalls looked like this (after that patch):\n\n $USE_OUTPUT git commit --no-verify -F \"$MSG\" $EDIT_COMMIT\n git commit --no-verify -F \"$DOTEST\"/message -e\n\nWitness that it's not checked whether 'git commit' succeeded or\nfailed.\nHowever, nowadays those lines look like this:\n\n $USE_OUTPUT git commit --no-verify $MSG_OPT \"$MSG_FILE\" $EDIT_COMMIT || failed=t\n git commit --no-verify -F \"$DOTEST\"/message -e || { ...\n\nThere is a check for failed 'git commit', so if the pre-commit hook\nfinds whitespace errors, then 'git commit' will error out, and,\nconsequently, 'git rebase -i' will error out, too, and commits won't\nbe squashed accidentaly.\n\nSo, I wonder whether those '--no-verify' options are still required in\n'git rebase -i'.\n\nAny thoughts?\n\nThanks,\nGábor\n"},{"id":"96367","messageId":"20081122201000.GA5903@sigill.intra.peff.net","threadId":"16250","inReplyTo":"20081122155541.GC6885@neumann","subject":"Re: git commit -v does not removes the patch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-22T20:10:01Z","receivedAt":"2008-11-22T20:10:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 22, 2008 at 04:55:41PM +0100, SZEDER Gábor wrote:\n\n> Thanks for the tip. Junio's suggestion about '# Everything under this\n> line is deleted.' could be implemented this way fairly easily (just an\n> additional echo in prepare-commit-msg, and other pattern in\n> commit-msg). But 'git commit --no-verify' is indeed a problem.\n> Although I never use it explicitly, 'git rebase -i' does so.\n\nOne other option would be to munge the editor script instead of using\nhooks. I.e., something like this:\n\n $ cat ~/local/bin/git-editor\n #!/bin/sh\n echo '# Everything under this line is deleted' >>\"$1\"\n git diff --cached >>\"$1\"\n $EDITOR \"$1\" || exit $?\n sed -i '/^# Everything under this line is deleted$/Q' \"$1\"\n\n $ git config core.editor ~/local/bin/git-editor\n\nOf course, then you end up with the munging for things like \"git rebase\n-i\" picking commits, but it should be harmless (since we end up removing\nit anyway).\n\n-Peff\n"},{"id":"96370","messageId":"7v4p1zpejx.fsf@gitster.siamese.dyndns.org","threadId":"16250","inReplyTo":"20081122155541.GC6885@neumann","subject":"Re: git commit -v does not removes the patch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-22T22:43:30Z","receivedAt":"2008-11-22T22:43:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> So, I wonder whether those '--no-verify' options are still required in\n> 'git rebase -i'.\n\nThe use of --no-verify there does not have anything to do with \"whitespace\nerrors\". It is to override _any_ validation the users want to do when\nusing \"git commit\" in their interactive workflow. It so happens that the\nexample hook we ship demonstrates how you hunt for whitespace errors, but\nyou have to remember that it is just an example.\n\nWe may want to disable the checking of exit status from commit-msg hook\nwhile still calling the hook itself, though. The primary purpose of the\nhook is to allow users to reformat (say, passing \"fmt -64\") the message,\nbut it is allowed to interfere and that was meant to happen when using\n\"git commit\" but we probably do not want it when rebasing.\n\nFurther, we also may want to make the use of --no-verify overridable from\nthe command line when running rebase. The primary purpose of the rebase\ncommand is to transplant a sequence of commits to someplace else without\nmolesting its contents and message unnecessarily, and --no-verify is a\ngood thing to use in general for that reason, but people may sometimes\nwant to use it as a way to clean up the changes.\n"}]}