{"thread":{"id":"26663","subject":"frustrated forensics: hard to find diff that undid a fix","startedAt":"2011-03-05T06:20:46Z","lastAt":"2011-03-09T22:20:29Z","messageCount":26,"participants":["Adam Monsen","Jonathan del Strother","Jakub Narebski","Jonathan Nieder","Jeff King","Martin von Zweigbergk","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"162808","messageId":"4D71D63E.3030907@gmail.com","threadId":"26663","inReplyTo":null,"subject":"frustrated forensics: hard to find diff that undid a fix","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-05T06:20:46Z","receivedAt":"2011-03-05T06:20:46Z","isPatch":false,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"I made a fix a month ago on the master branch in a shared repo. A week\nlater, a colleague did a merge that undid the fix. I didn't figure out\nthe problem until just now because I'd been assuming the fix was still\non master. I mean, if it wasn't, I should see a reverse patch using \"git\nlog -p master\", right? Wrong. Turns out the fix was undone as part of\nmerge conflict resolution (I think).\n\nIs there some way to include merge conflict resolutions in \"git log -p\"\nor \"git show\"? Apparently some important information can be hidden in\nthe conflict resolution. Or, more likely, I just don't understand how\nthis bit of git works.\n\nI also tried bisect and pickaxe. Bisect wrongly identified the first bad\ncommit, and pickaxe just didn't see the change at all.\n\n ~ * ~\n\nHere's some details in case anyone wants to (a) point out where I messed\nup or (b) help me avoid this kind of blunder in the future.\n\n1. The repo is git://mifos.git.sourceforge.net/gitroot/mifos/head\n(mirror: https://github.com/mifos/head ). Branch master.\n\n2. My commit 2a1ed0436 introduced a fix that includes the text\n\"native2ascii\". Shows up in \"git log -p -1 2a1ed0436\" and \"git show\n2a1ed0436\". Life is good.\n\n3. It appears that the merge commit 0f8132386 tossed my \"native2ascii\"\nfix. The only way I could figure out to actually visualize this is \"git\ndiff 58320586..0f813238\".\n\nThis took a while to figure out. Am I missing something obvious?\n"},{"id":"162814","messageId":"AANLkTinKmgnVN+zyhu03yiH4z2ucxqd9yBn+6f7ptnp0@mail.gmail.com","threadId":"26663","inReplyTo":"4D71D63E.3030907@gmail.com","subject":"Re: frustrated forensics: hard to find diff that undid a fix","fromName":"Jonathan del Strother","fromEmail":"maillist@steelskies.com","sentAt":"2011-03-05T10:00:45Z","receivedAt":"2011-03-05T10:00:45Z","isPatch":false,"sender":{"key":"jon.delstrother@bestbefore.tv","avatar":"https://gravatar.com/avatar/754e21ab701c00e2d21fc261187254c34b2a1c0b959d9ee5be1a295990be3081?d=mp&s=160"},"body":"On 5 March 2011 06:20, Adam Monsen wrote:\n> I made a fix a month ago on the master branch in a shared repo. A week\n> later, a colleague did a merge that undid the fix. I didn't figure out\n> the problem until just now because I'd been assuming the fix was still\n> on master. I mean, if it wasn't, I should see a reverse patch using \"git\n> log -p master\", right? Wrong. Turns out the fix was undone as part of\n> merge conflict resolution (I think).\n>\n> Is there some way to include merge conflict resolutions in \"git log -p\"\n> or \"git show\"? Apparently some important information can be hidden in\n> the conflict resolution. Or, more likely, I just don't understand how\n> this bit of git works.\n>\n> I also tried bisect and pickaxe. Bisect wrongly identified the first bad\n> commit, and pickaxe just didn't see the change at all.\n>\n>    ~ * ~\n>\n> Here's some details in case anyone wants to (a) point out where I messed\n> up or (b) help me avoid this kind of blunder in the future.\n>\n> 1. The repo is git://mifos.git.sourceforge.net/gitroot/mifos/head\n> (mirror: https://github.com/mifos/head ). Branch master.\n>\n> 2. My commit 2a1ed0436 introduced a fix that includes the text\n> \"native2ascii\". Shows up in \"git log -p -1 2a1ed0436\" and \"git show\n> 2a1ed0436\". Life is good.\n>\n> 3. It appears that the merge commit 0f8132386 tossed my \"native2ascii\"\n> fix. The only way I could figure out to actually visualize this is \"git\n> diff 58320586..0f813238\".\n>\n> This took a while to figure out. Am I missing something obvious?\n\n\nSeems like a bunch of people (myself included) have been tripped up by\nthis behaviour in the past few months. Did something change to start\nthis, or has it always been there?\n\n-Jonathan\n"},{"id":"162815","messageId":"m37hcd7qfv.fsf@localhost.localdomain","threadId":"26663","inReplyTo":"4D71D63E.3030907@gmail.com","subject":"Re: frustrated forensics: hard to find diff that undid a fix","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-03-05T11:33:26Z","receivedAt":"2011-03-05T11:33:26Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Adam Monsen writes:\n\n> I made a fix a month ago on the master branch in a shared repo. A week\n> later, a colleague did a merge that undid the fix. I didn't figure out\n> the problem until just now because I'd been assuming the fix was still\n> on master. I mean, if it wasn't, I should see a reverse patch using \"git\n> log -p master\", right? Wrong. Turns out the fix was undone as part of\n> merge conflict resolution (I think).\n> \n> Is there some way to include merge conflict resolutions in \"git log -p\"\n> or \"git show\"? Apparently some important information can be hidden in\n> the conflict resolution. Or, more likely, I just don't understand how\n> this bit of git works.\n\nBy default \"git log -p\" and \"git show\" considers merges uninteresting.\nTry \"git log -p -c\" or \"git log -p -m\".\n \n> I also tried bisect and pickaxe. Bisect wrongly identified the first bad\n> commit, and pickaxe just didn't see the change at all.\n\nI guess that pickaxe also needs -c or -m.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"162818","messageId":"20110305125100.GA14547@elie","threadId":"26663","inReplyTo":"m37hcd7qfv.fsf@localhost.localdomain","subject":"Re: frustrated forensics: hard to find diff that undid a fix","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-05T12:51:00Z","receivedAt":"2011-03-05T12:51:00Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJakub Narebski wrote:\n\n> I guess that pickaxe also needs -c or -m.\n\nI am not so sure.\n\nPickaxe is used to ask, \"what commit introduced this string?\". Using\n\"git log --raw -c\", I can see that the current state of\ncontrib/fast-import/git-p4 came about in commit 6d74e5c9d (Merge\nbranch 'mh/p4', 2011-03-04):\n\n| $ git log --oneline --raw -c\n| 07873dc Merge branch 'maint'\n| \n| 6d74e5c Merge branch 'mh/p4'\n| \n| ::100755 100755 100755 a4f440d... 8b00fd8... 2df3bb2... MM\n| contrib/fast-import/git-p4\n[...]\n\nNow, working backwards, I ask git:\n\n| $ git log --oneline -S \"$(cat contrib/fast-import/git-p4)\" maint..master\n| $\n\nNo hits. Maybe it's from one of those mergey diffs?\n\n| $ git log --oneline -m -S \"$(cat contrib/fast-import/git-p4)\" maint..master\n| 07873dc (from 964498e) Merge branch 'maint'\n| 6d74e5c (from 08fd871) Merge branch 'mh/p4'\n| 6d74e5c (from c9dbab0) Merge branch 'mh/p4'\n| $\n\nToo many hits (it includes every merge in which one side contains\nthe string and the other does not). How about -c, which seemed to\nproduce such nice output with --raw?\n\n| $ git log --oneline -c -S \"$(cat contrib/fast-import/git-p4)\" maint..master\n| 07873dc Merge branch 'maint'\n| 6d74e5c Merge branch 'mh/p4'\n| 08fd871 Merge branch 'mg/maint-difftool-vim-readonly'\n| 5cb3c9b Merge branch 'jn/maint-commit-missing-template'\n| 1538f21 Merge branch 'jk/diffstat-binary'\n| 24161eb Merge branch 'lt/rename-no-extra-copy-detection'\n| [...]\n\nOh. diff_tree_combined_merge simply doesn't know about pickaxe,\nso -c and --cc with -S print _all_ merges.\n\nSo the only sensible way to use pickaxe with merges is\n\n| $ git log --oneline -m --first-parent \\\n|\t-S \"$(cat contrib/fast-import/git-p4)\" maint..master\n| 6d74e5c Merge branch 'mh/p4'\n\nfor now. I'd be happy to help anyone hoping to improve this.\n(Hopefully all that is needed is something like the\ndiff_queue_is_empty() check from v0.99~504 --- Diffcore updates,\n2005-05-22.)\n"},{"id":"162821","messageId":"20110305134846.GA2221@sigill.intra.peff.net","threadId":"26663","inReplyTo":"20110305125100.GA14547@elie","subject":"Re: frustrated forensics: hard to find diff that undid a fix","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-05T13:48:47Z","receivedAt":"2011-03-05T13:48:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 05, 2011 at 06:51:00AM -0600, Jonathan Nieder wrote:\n\n> | $ git log --oneline -m -S \"$(cat contrib/fast-import/git-p4)\" maint..master\n> | 07873dc (from 964498e) Merge branch 'maint'\n> | 6d74e5c (from 08fd871) Merge branch 'mh/p4'\n> | 6d74e5c (from c9dbab0) Merge branch 'mh/p4'\n> | $\n> \n> Too many hits (it includes every merge in which one side contains\n> the string and the other does not). How about -c, which seemed to\n> produce such nice output with --raw?\n\nYeah, I think the problem is that the diffcore chain doesn't know enough\nabout the merge. It just sees the filepairs between the commit and each\nparent separately. I looked into this recently as part of this thread:\n\n http://article.gmane.org/gmane.comp.version-control.git/165743\n\nI described some reasonable semantics there, but implementing them\nlooked non-trivial. I'd be very happy to be proven wrong. Patches\nwelcome. :)\n\n-Peff\n"},{"id":"162825","messageId":"alpine.DEB.2.00.1103050844130.26585@debian","threadId":"26663","inReplyTo":"4D71D63E.3030907@gmail.com","subject":"Re: frustrated forensics: hard to find diff that undid a fix","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-03-05T14:29:57Z","receivedAt":"2011-03-05T14:29:57Z","isPatch":false,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Fri, 4 Mar 2011, Adam Monsen wrote:\n\n> I made a fix a month ago on the master branch in a shared repo. A week\n> later, a colleague did a merge that undid the fix. I didn't figure out\n> the problem until just now because I'd been assuming the fix was still\n> on master. I mean, if it wasn't, I should see a reverse patch using \"git\n> log -p master\", right? Wrong. Turns out the fix was undone as part of\n> merge conflict resolution (I think).\n> \n> Is there some way to include merge conflict resolutions in \"git log -p\"\n> or \"git show\"? Apparently some important information can be hidden in\n> the conflict resolution. Or, more likely, I just don't understand how\n> this bit of git works.\n> \n> I also tried bisect and pickaxe. Bisect wrongly identified the first bad\n> commit, and pickaxe just didn't see the change at all.\n\nI was also bitten by this at work not too long ago. I started\nwondering if it would make sense to introduce a new option to git log\nand friends that would show the differences compared to a recreated\nmerge commit. In the simple case where there are no merge conflicts,\nthis would show only changes that someone explicitly dropped, like in\nyour case. If there were conflicts, I imagine it would show the same\noutput as -c or --cc. Does this make any sense?\n\nOne of the reasons that people sometimes drop the 'theirs' side while\nmerging is that they see the files show up when running 'git status'\nand they think \"Hmm... I didn't modify this file, let's reset\nit\". Would it be completely nonsensical to suggest that 'git status'\ncould learn to, during a merge, compare to a recreated merge commit\ninstead of comparing to HEAD?\n\nLet me know what you think. I haven't really thought this through, so\nI wouldn't be surprised if I'm just talking nonsense.\n\n\n/Martin\n"},{"id":"162826","messageId":"4D724A0F.7050904@gmail.com","threadId":"26663","inReplyTo":"m37hcd7qfv.fsf@localhost.localdomain","subject":"Re: frustrated forensics: hard to find diff that undid a fix","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-05T14:34:55Z","receivedAt":"2011-03-05T14:34:55Z","isPatch":false,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"Jakub Narebski wrote:\n> By default \"git log -p\" and \"git show\" considers merges uninteresting.\n> Try \"git log -p -c\" or \"git log -p -m\".\n\nHoly cow, that's a lifesaver! Thank you, Jakub!!\n\nI totally missed the whole \"Diff Formatting\" section in the git-log(1)\nmanpage.\n\n\"git log -p -c\" does exactly what I want. I'll make an alias or\nsomething for these options.\n\nI'm confused by this section of git-log(1):\n\n COMBINED DIFF FORMAT\n \"git-diff-tree\", \"git-diff-files\" and \"git-diff\" can take -c\n or --cc option to produce combined diff. For showing a merge\n commit with \"git log -p\", this is the default format; you can\n force showing full diff with the -m option.\n\nSounds like this is saying that -c is the default with \"git log -p\".\nI'll submit a patch to fix that.\n"},{"id":"162836","messageId":"1299355004-3532-1-git-send-email-haircut@gmail.com","threadId":"26663","inReplyTo":"4D724A0F.7050904@gmail.com","subject":"[PATCH 0/2] improve combined diff documentation","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-05T19:56:42Z","receivedAt":"2011-03-05T19:56:42Z","isPatch":true,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"The \"combined diff format\" documentation in diff-generate-patch.txt\nincorrectly implied that \"log -p\" shows conflict resolutions as\ncombined diffs.\n\nWith these small documentation fixes I'm hoping to save someone the\ntime it took me to figure out how a merge conflict was resolved.\n\nRelated thread:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/168481\n\nThe patches apply cleanly to maint.\n\nHope this helps,\n-Adam (\"meonkeys\" on IRC)\n\nAdam Monsen (2):\n documentation fix: git log -p does not imply -c.\n English grammar fixes for combined diff doc.\n\n Documentation/diff-generate-patch.txt | 8 +++-----\n 1 files changed, 3 insertions(+), 5 deletions(-)\n\n-- \n1.7.2.3\n"},{"id":"162838","messageId":"1299355004-3532-2-git-send-email-haircut@gmail.com","threadId":"26663","inReplyTo":"4D724A0F.7050904@gmail.com","subject":"[PATCH 1/2] documentation fix: git log -p does not imply -c.","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-05T19:56:43Z","receivedAt":"2011-03-05T19:56:43Z","isPatch":true,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"Relates to the thread with subject \"frustrated forensics: hard to find\ndiff that undid a fix\" on the git mailing list.\n\n\thttp://thread.gmane.org/gmane.comp.version-control.git/168481\n\nI don't wish for anyone to repeat my bungled forensics episode.\nHopefully this will help others git along happily.\n\nSigned-off-by: Adam Monsen \n---\n Documentation/diff-generate-patch.txt | 7 +++----\n 1 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\nindex 3ac2bea..6cd5270 100644\n--- a/Documentation/diff-generate-patch.txt\n+++ b/Documentation/diff-generate-patch.txt\n@@ -75,10 +75,9 @@ combined diff format\n --------------------\n \n \"git-diff-tree\", \"git-diff-files\" and \"git-diff\" can take '-c' or\n-'--cc' option to produce 'combined diff'. For showing a merge commit\n-with \"git log -p\", this is the default format; you can force showing\n-full diff with the '-m' option.\n-A 'combined diff' format looks like this:\n+'--cc' option to produce 'combined diff'. You can force showing\n+full diff with the '-m' option. A 'combined diff' format looks like\n+this:\n \n ------------\n diff --combined describe.c\n-- \n1.7.2.3\n"},{"id":"162837","messageId":"1299355004-3532-3-git-send-email-haircut@gmail.com","threadId":"26663","inReplyTo":"4D724A0F.7050904@gmail.com","subject":"[PATCH 2/2] English grammar fixes for combined diff doc.","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-05T19:56:44Z","receivedAt":"2011-03-05T19:56:44Z","isPatch":true,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"This is incredibly minor, I only separated it from the first patch so it\nwas easier to see how this is separate from the other change.\n\nSigned-off-by: Adam Monsen \n---\n Documentation/diff-generate-patch.txt | 7 +++----\n 1 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\nindex 6cd5270..607fa54 100644\n--- a/Documentation/diff-generate-patch.txt\n+++ b/Documentation/diff-generate-patch.txt\n@@ -74,10 +74,9 @@ separate lines indicate the old and the new mode.\n combined diff format\n --------------------\n \n-\"git-diff-tree\", \"git-diff-files\" and \"git-diff\" can take '-c' or\n-'--cc' option to produce 'combined diff'. You can force showing\n-full diff with the '-m' option. A 'combined diff' format looks like\n-this:\n+\"git-diff-tree\", \"git-diff-files\" and \"git-diff\" can take a '-c' or\n+'--cc' option to produce 'combined diff' output. You can force showing\n+full diffs with the '-m' option. A 'combined diff' looks like this:\n \n ------------\n diff --combined describe.c\n-- \n1.7.2.3\n"},{"id":"162888","messageId":"7vbp1n4vhv.fsf@alter.siamese.dyndns.org","threadId":"26663","inReplyTo":"1299355004-3532-2-git-send-email-haircut@gmail.com","subject":"Re: [PATCH 1/2] documentation fix: git log -p does not imply -c.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-07T00:36:28Z","receivedAt":"2011-03-07T00:36:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Monsen writes:\n\n> Relates to the thread with subject \"frustrated forensics: hard to find\n> diff that undid a fix\" on the git mailing list.\n>\n> \thttp://thread.gmane.org/gmane.comp.version-control.git/168481\n>\n> I don't wish for anyone to repeat my bungled forensics episode.\n\nBut this patch is wrong, isn't it?\n\nThe --cc format _is_ the default, not -c format.\n"},{"id":"162919","messageId":"20110307154712.GA11934@sigill.intra.peff.net","threadId":"26663","inReplyTo":"7vbp1n4vhv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] documentation fix: git log -p does not imply -c.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-07T15:47:12Z","receivedAt":"2011-03-07T15:47:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 06, 2011 at 04:36:28PM -0800, Junio C Hamano wrote:\n\n> Adam Monsen writes:\n> \n> > Relates to the thread with subject \"frustrated forensics: hard to find\n> > diff that undid a fix\" on the git mailing list.\n> >\n> > \thttp://thread.gmane.org/gmane.comp.version-control.git/168481\n> >\n> > I don't wish for anyone to repeat my bungled forensics episode.\n> \n> But this patch is wrong, isn't it?\n> \n> The --cc format _is_ the default, not -c format.\n\nHmm. \"git show\" seems to show --cc, but \"git log -p\" does not show\nanything. Try:\n\n $ git show 1538f21\n $ git log -p -1 1538f21\n\nSo the current doc seems to be wrong. Should we be fixing the code and\nnot the doc?\n\n-Peff\n"},{"id":"162928","messageId":"7vtyfe22vy.fsf@alter.siamese.dyndns.org","threadId":"26663","inReplyTo":"20110307154712.GA11934@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] documentation fix: git log -p does not imply -c.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-07T18:37:21Z","receivedAt":"2011-03-07T18:37:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sun, Mar 06, 2011 at 04:36:28PM -0800, Junio C Hamano wrote:\n>\n>> The --cc format _is_ the default, not -c format.\n>\n> Hmm. \"git show\" seems to show --cc, but \"git log -p\" does not show\n> anything.\n\nThe intention has always been to default to --cc since 0fe7c1d (built-in\ndiff: assorted updates., 2006-04-29) for \"diff\" if I am not misremembering\nthings, but you are right---\"log\" is a tad different.\n\nThe code does not want to use --cc by default for \"log\", and I don't think\nthat should change. See 1aec791 (git log: don't do merge diffs by\ndefault, 2006-04-19).\n"},{"id":"162930","messageId":"20110307191218.GA20930@sigill.intra.peff.net","threadId":"26663","inReplyTo":"7vtyfe22vy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] documentation fix: git log -p does not imply -c.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-07T19:12:18Z","receivedAt":"2011-03-07T19:12:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 07, 2011 at 10:37:21AM -0800, Junio C Hamano wrote:\n\n> > Hmm. \"git show\" seems to show --cc, but \"git log -p\" does not show\n> > anything.\n> \n> The intention has always been to default to --cc since 0fe7c1d (built-in\n> diff: assorted updates., 2006-04-29) for \"diff\" if I am not misremembering\n> things, but you are right---\"log\" is a tad different.\n> \n> The code does not want to use --cc by default for \"log\", and I don't think\n> that should change. See 1aec791 (git log: don't do merge diffs by\n> default, 2006-04-19).\n\nThanks for the history. I think the doc problem was an inaccuracy that\nsnuck in during 272bd3c (Include diff options in the git-log manpage,\n2007-11-01). Nearly identical text (without the inaccuracy) is in the\n\"Diff Format For Merges\" section in diff-format.txt.\n\nFurthermore, the copied text talks about diff-index and diff-tree, but\ngets included inline in git-log(1) (although the part in diff-format.txt\ndoes not get included in git-log's manpage)[1]. So probably it's\nreasonable to clean it up to something like:\n\ndiff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\nindex 3ac2bea..3d02da9 100644\n--- a/Documentation/diff-generate-patch.txt\n+++ b/Documentation/diff-generate-patch.txt\n@@ -74,10 +74,12 @@ separate lines indicate the old and the new mode.\n combined diff format\n --------------------\n \n-\"git-diff-tree\", \"git-diff-files\" and \"git-diff\" can take '-c' or\n-'--cc' option to produce 'combined diff'. For showing a merge commit\n-with \"git log -p\", this is the default format; you can force showing\n-full diff with the '-m' option.\n+Any diff-generating command can take the `-c` or `--cc` option to\n+produced a 'combined diff' when showing a merge. This is the default\n+format when showing merge conflicts with linkgit:git-diff[1] or a merge\n+commit with linkgit:git-show[1]. Note also that you can vie the full\n+diff with the `-m` option.\n+\n A 'combined diff' format looks like this:\n \n ------------\n\n-- >8 --\n\nIs there any way to get \"git diff\" to show combined-form besides an\nindex with conflicts? I couldn't convince it to show me a merge commit\nbeside its parents, since it doesn't have an equivalent to diff-tree's\n--stdin option.\n\n-Peff\n\n[1] Reading over this, the whole section could use some editing. I think\nthis is another example that needs to be broken out into its own\nuser-visible manpage. That is, we have too much \"if you use the -p\noption to command X, or command Y by default, or command Z without\n--raw, then you see this format\". That's pretty dense. Instead command X\nshould have:\n\n -p::\n --stat::\n --summary::\n [etc]\n Generate diffs in this format. See \"git help diff-formats\" for\n details. The default format is \"-p\".\n\nand then diff-format.txt should _just_ be a description of the diff\nformats, without worrying about commands at all. And probably the text\nabove should be factored out as part of diff-options.txt. But that's all\npart of a much bigger documentation architecture change that I am hoping\nto get to eventually. For now, I think it's worth just tweaking this\ntext to stop being inaccurate.\n"},{"id":"162940","messageId":"1299535063-1020-1-git-send-email-haircut@gmail.com","threadId":"26663","inReplyTo":"20110307191218.GA20930@sigill.intra.peff.net","subject":"[PATCH v3] Documentation fix: git log -p does not imply -c.","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-07T21:57:43Z","receivedAt":"2011-03-07T21:57:43Z","isPatch":true,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"Relates to the thread with subject \"frustrated forensics: hard to find\ndiff that undid a fix\" on the git mailing list.\n\n http://thread.gmane.org/gmane.comp.version-control.git/168481\n\nI don't wish for anyone to repeat my bungled forensics episode.\nHopefully this will help others git along happily.\n\nSigned-off-by: Adam Monsen \n---\nThis is really Peff's patch, I\n* fixed a typo (vie -> view)\n* am sending it as an acutal patch in case that's easier to apply than\n a diff in an email\n\nI wasn't sure what \"Author:\" should be, go ahead and change it to the\nmost appropriate person.\n\nHope this helps,\n-Adam\n\n Documentation/diff-generate-patch.txt | 10 ++++++----\n 1 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\nindex 3ac2bea..5d478c1 100644\n--- a/Documentation/diff-generate-patch.txt\n+++ b/Documentation/diff-generate-patch.txt\n@@ -74,10 +74,12 @@ separate lines indicate the old and the new mode.\n combined diff format\n --------------------\n \n-\"git-diff-tree\", \"git-diff-files\" and \"git-diff\" can take '-c' or\n-'--cc' option to produce 'combined diff'. For showing a merge commit\n-with \"git log -p\", this is the default format; you can force showing\n-full diff with the '-m' option.\n+Any diff-generating command can take the `-c` or `--cc` option to\n+produced a 'combined diff' when showing a merge. This is the default\n+format when showing merge conflicts with linkgit:git-diff[1] or a merge\n+commit with linkgit:git-show[1]. Note also that you can view the full\n+diff with the `-m` option.\n+\n A 'combined diff' format looks like this:\n \n ------------\n-- \n1.7.2.3\n"},{"id":"162951","messageId":"7vsjuyzckd.fsf@alter.siamese.dyndns.org","threadId":"26663","inReplyTo":"1299535063-1020-1-git-send-email-haircut@gmail.com","subject":"Re: [PATCH v3] Documentation fix: git log -p does not imply -c.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-08T00:21:54Z","receivedAt":"2011-03-08T00:21:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Monsen writes:\n\n> This is really Peff's patch, I\n> * fixed a typo (vie -> view)\n> * am sending it as an acutal patch in case that's easier to apply than\n> a diff in an email\n\nThanks. Such a patch to summarize the discussion so far is greatly\nappreciated.\n\n> Documentation/diff-generate-patch.txt | 10 ++++++----\n> 1 files changed, 6 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\n> index 3ac2bea..5d478c1 100644\n> --- a/Documentation/diff-generate-patch.txt\n> +++ b/Documentation/diff-generate-patch.txt\n> @@ -74,10 +74,12 @@ separate lines indicate the old and the new mode.\n> combined diff format\n> --------------------\n> \n> -\"git-diff-tree\", \"git-diff-files\" and \"git-diff\" can take '-c' or\n> -'--cc' option to produce 'combined diff'. For showing a merge commit\n> -with \"git log -p\", this is the default format; you can force showing\n> -full diff with the '-m' option.\n> +Any diff-generating command can take the `-c` or `--cc` option to\n> +produced a 'combined diff' when showing a merge. This is the default\n\ns/produced/produce/, I think.\n\n> +format when showing merge conflicts with linkgit:git-diff[1] or a merge\n> +commit with linkgit:git-show[1]. Note also that you can view the full\n> +diff with the `-m` option.\n\nThis \"Note\" is a bit unclear what command it applies to, isn't it? I know\nit applies to all the commands mentioned in the previous sentence in the\nparagraph, but we are not writing the documentation for me, so perhaps \n\n\tNote also that you can give the `-m' option to any of these\n\tcommands to force generation of diffs with individual parents of a\n\tmerge.\n\nAlso -c and --cc are technically _not_ about \"showing merge conflicts\".\nIt is about \"showing a merge commit\". I don't know if we want to teach\nthe distinction in this part of the document, though.\n\nIf you resolve a conflicted merge taking the results from only one side\nfor a given hunk, --cc won't show anything. If on the other hand, you\nfutz with a clean merge so that your result does not match with any\nparent, --cc will show it.\n\nCf.\n\n http://thread.gmane.org/gmane.comp.version-control.git/89415\n"},{"id":"162952","messageId":"1299545378-22036-1-git-send-email-haircut@gmail.com","threadId":"26663","inReplyTo":"7vsjuyzckd.fsf@alter.siamese.dyndns.org","subject":"[PATCH v4] Documentation fix: git log -p does not imply -c.","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-08T00:49:38Z","receivedAt":"2011-03-08T00:49:38Z","isPatch":true,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"Relates to the thread with subject \"frustrated forensics: hard to find\ndiff that undid a fix\" on the git mailing list.\n\n http://thread.gmane.org/gmane.comp.version-control.git/168481\n\nI don't wish for anyone to repeat my bungled forensics episode.\nHopefully this will help others git along happily.\n\nSee also:\n\n http://thread.gmane.org/gmane.comp.version-control.git/89415\n\nSigned-off-by: Adam Monsen \n---\n\nJunio wrote:\n> s/produced/produce/, I think.\n\nAh, indeed. Fixed, thanks.\n\nI also sidestepped the difference between merge commits and conflicts by\nchanging the wording slightly.\n\nYour changes to the \"Note\" seemed helpful, I put them in. Thanks!\n\n Documentation/diff-generate-patch.txt | 11 +++++++----\n 1 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\nindex 3ac2bea..c57460c 100644\n--- a/Documentation/diff-generate-patch.txt\n+++ b/Documentation/diff-generate-patch.txt\n@@ -74,10 +74,13 @@ separate lines indicate the old and the new mode.\n combined diff format\n --------------------\n \n-\"git-diff-tree\", \"git-diff-files\" and \"git-diff\" can take '-c' or\n-'--cc' option to produce 'combined diff'. For showing a merge commit\n-with \"git log -p\", this is the default format; you can force showing\n-full diff with the '-m' option.\n+Any diff-generating command can take the `-c` or `--cc` option to\n+produce a 'combined diff' when showing a merge. This is the default\n+format when showing merges with linkgit:git-diff[1] or\n+linkgit:git-show[1]. Note also that you can give the `-m' option to any\n+of these commands to force generation of diffs with individual parents\n+of a merge.\n+\n A 'combined diff' format looks like this:\n \n ------------\n-- \n1.7.2.3\n"},{"id":"162955","messageId":"20110308011917.GB21278@sigill.intra.peff.net","threadId":"26663","inReplyTo":"7vsjuyzckd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] Documentation fix: git log -p does not imply -c.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-08T01:19:18Z","receivedAt":"2011-03-08T01:19:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 07, 2011 at 04:21:54PM -0800, Junio C Hamano wrote:\n\n> > +produced a 'combined diff' when showing a merge. This is the default\n> \n> s/produced/produce/, I think.\n\nOops.\n\n> > +format when showing merge conflicts with linkgit:git-diff[1] or a merge\n> > +commit with linkgit:git-show[1]. Note also that you can view the full\n> > +diff with the `-m` option.\n> \n> This \"Note\" is a bit unclear what command it applies to, isn't it? I know\n> it applies to all the commands mentioned in the previous sentence in the\n> paragraph, but we are not writing the documentation for me, so perhaps \n> \n> \tNote also that you can give the `-m' option to any of these\n> \tcommands to force generation of diffs with individual parents of a\n> \tmerge.\n\nYeah, reading it again, I agree it is unclear. Yours is better.\n\n> Also -c and --cc are technically _not_ about \"showing merge conflicts\".\n> It is about \"showing a merge commit\". I don't know if we want to teach\n> the distinction in this part of the document, though.\n\nYeah, I almost said that, but I couldn't figure out a way to convince\n\"git diff\" to show me a merge commit, and clearly it is one of the\ninteresting commands to mention as \"defaults to --cc\". Again, I think\nthis section would be better as \"This is what combined diff looks like\"\nand leave the discussion of \"this is how and when you trigger combined\ndiff\" to the individual command manpages. But that is a much bigger\ncleanup.\n\n> If you resolve a conflicted merge taking the results from only one side\n> for a given hunk, --cc won't show anything. If on the other hand, you\n> futz with a clean merge so that your result does not match with any\n> parent, --cc will show it.\n\nRight, that's the same as \"diff-tree --cc\" would show if you committed\nit, no? Which makes sense to me.\n\n-Peff\n"},{"id":"163007","messageId":"7vmxl5e6ur.fsf@alter.siamese.dyndns.org","threadId":"26663","inReplyTo":"1299545378-22036-1-git-send-email-haircut@gmail.com","subject":"Re: [PATCH v4] Documentation fix: git log -p does not imply -c.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-08T19:43:08Z","receivedAt":"2011-03-08T19:43:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Monsen writes:\n\n> Relates to the thread with subject \"frustrated forensics: hard to find\n> diff that undid a fix\" on the git mailing list.\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/168481\n>\n> I don't wish for anyone to repeat my bungled forensics episode.\n> Hopefully this will help others git along happily.\n>\n> See also:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/89415\n>\n> Signed-off-by: Adam Monsen \n\nPlease don't do this.\n\nRe-read what you wrote above while pretending that you do not have any\nknowledge of the \"frustrated forensics\" you did. Does it convey _any_\nuseful information? Log messages should be sufficiently understandable\noffline without having the web access.\n\nInstead, summarize why the change is necessary. IOW, don't be lazy now\nwhile writing the log, to save time for people who later need to read log.\n\nSomething like\n\n Subject: diff format documentation: clarify --cc and -c\n\n The description was unclear if -c or --cc was the default (--cc is for\n some commands), and incorrectly implied that the default applies to\n all the diff generating commands.\n\n Most importantly, \"log\" does not default to \"--cc\" (it defaults to\n \"--no-merges\") and \"log -p\" obeys the user's wish to see non-combined\n format. Only \"diff\" (during merge and three-blob comparison) and\n \"show\" use --cc as the default.\n\nshould be sufficient.\n"},{"id":"163015","messageId":"4D7695A9.8070403@gmail.com","threadId":"26663","inReplyTo":"7vmxl5e6ur.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4] Documentation fix: git log -p does not imply -c.","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-08T20:46:33Z","receivedAt":"2011-03-08T20:46:33Z","isPatch":true,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"Junio C Hamano wrote:\n> Log messages should be sufficiently understandable offline without \n> having the web access.\n\nOk! Makes sense.\n\nI read some stuff before writing it (like\nDocumentation/SubmittingPatches), but what I should have done is just\nthumb through the log. Many commit messages are as you say they should be.\n\n> Something like ...... should be sufficient.\n\nThanks, I'll use that. It includes history and code details I didn't know.\n\nThis is good advice about how to fit in to the git community... would\nyou like a \"commit message guide\"? I did something like this for another\ncommunity (Mifos), and they found it helpful. Here's a rough draft:\n\n-----------8<-----------\n\nCommit message guide\n====================\n\nThe suggested *format* of a commit message is covered in DISCUSSION in\ngit-commit(1). This guide covers philosophy of commit messages.\n\n- Read previous commit messages. Emulate the best ones.\n- Reveal your intentions.\n- Answer questions you anticipate others will ask.\n- Imagine you are reading this same commit message 10 years from now.\n What would be most helpful for you to quickly recall why these\n changes were made?\n- Imagine someone else is reading this same commit message 10 years\n from now. What would be most helpful for them to quickly understand\n what this commit changes and why it was done?\n- Commit messages should be sufficiently understandable without access\n to any online content.\n- Be verbose!\n- This is your chance to use time- and context-sensitive information\n relevant to code changed.\n- Refer to related content.\n - other commits\n - mailing list discussions (but not in lieu of a proper description)\n\n----------->8-----------\n\nIf you want a guide like this, some questions:\n* do you want asciidoc, something else, or don't care?\n* name it Documentation/CommitMessageGuide ? or something else?\n"},{"id":"163014","messageId":"1299617497-17447-1-git-send-email-haircut@gmail.com","threadId":"26663","inReplyTo":"7vmxl5e6ur.fsf@alter.siamese.dyndns.org","subject":"[PATCH v5] diff format documentation: clarify --cc and -c","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-08T20:51:37Z","receivedAt":"2011-03-08T20:51:37Z","isPatch":true,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"The description was unclear if -c or --cc was the default (--cc is for\nsome commands), and incorrectly implied that the default applies to\nall the diff generating commands.\n\nMost importantly, \"log\" does not default to \"--cc\" (it defaults to\n\"--no-merges\") and \"log -p\" obeys the user's wish to see non-combined\nformat. Only \"diff\" (during merge and three-blob comparison) and\n\"show\" use --cc as the default.\n\nSigned-off-by: Adam Monsen \n---\n\nHere's another try at \"my\" first git patch, a one-paragraph\ndocumentation change. Now featuring a much-improved commit message by\nJunio.\n\n Documentation/diff-generate-patch.txt | 11 +++++++----\n 1 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\nindex 3ac2bea..c57460c 100644\n--- a/Documentation/diff-generate-patch.txt\n+++ b/Documentation/diff-generate-patch.txt\n@@ -74,10 +74,13 @@ separate lines indicate the old and the new mode.\n combined diff format\n --------------------\n \n-\"git-diff-tree\", \"git-diff-files\" and \"git-diff\" can take '-c' or\n-'--cc' option to produce 'combined diff'. For showing a merge commit\n-with \"git log -p\", this is the default format; you can force showing\n-full diff with the '-m' option.\n+Any diff-generating command can take the `-c` or `--cc` option to\n+produce a 'combined diff' when showing a merge. This is the default\n+format when showing merges with linkgit:git-diff[1] or\n+linkgit:git-show[1]. Note also that you can give the `-m' option to any\n+of these commands to force generation of diffs with individual parents\n+of a merge.\n+\n A 'combined diff' format looks like this:\n \n ------------\n-- \n1.7.2.3\n"},{"id":"163020","messageId":"1299618236-17933-1-git-send-email-haircut@gmail.com","threadId":"26663","inReplyTo":"7vmxl5e6ur.fsf@alter.siamese.dyndns.org","subject":"[PATCH v6] diff format documentation: clarify --cc and -c","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-08T21:03:56Z","receivedAt":"2011-03-08T21:03:56Z","isPatch":true,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"The description was unclear if -c or --cc was the default (--cc is for\nsome commands), and incorrectly implied that the default applies to\nall diff generating commands.\n\nMost importantly, \"log\" does not default to \"--cc\" (it defaults to\n\"--no-merges\") and \"log -p\" obeys the user's wish to see non-combined\nformat. Only \"diff\" (during merge and three-blob comparison) and\n\"show\" use --cc as the default.\n\nThe genesis of this patch was me getting frustrated trying to find\nchanges hidden in conflict resolutions of a merge commit. Jeff King\nproposed a documentation fix. I made it into a patch, and worked on it\nwith Junio. See the thread \"frustrated forensics: hard to find diff\nthat undid a fix\" on the git mailing list:\n\n http://thread.gmane.org/gmane.comp.version-control.git/168481\n\nFor more historical information about viewing merge conflict\nresolutions, see this post by Linus from 2008:\n\n http://article.gmane.org/gmane.comp.version-control.git/89415\n\nSigned-off-by: Adam Monsen \n---\n\nPlease ignore v5. I forgot to include links to mailing list archives\nin that version.\n\n Documentation/diff-generate-patch.txt | 11 +++++++----\n 1 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt\nindex 3ac2bea..c57460c 100644\n--- a/Documentation/diff-generate-patch.txt\n+++ b/Documentation/diff-generate-patch.txt\n@@ -74,10 +74,13 @@ separate lines indicate the old and the new mode.\n combined diff format\n --------------------\n \n-\"git-diff-tree\", \"git-diff-files\" and \"git-diff\" can take '-c' or\n-'--cc' option to produce 'combined diff'. For showing a merge commit\n-with \"git log -p\", this is the default format; you can force showing\n-full diff with the '-m' option.\n+Any diff-generating command can take the `-c` or `--cc` option to\n+produce a 'combined diff' when showing a merge. This is the default\n+format when showing merges with linkgit:git-diff[1] or\n+linkgit:git-show[1]. Note also that you can give the `-m' option to any\n+of these commands to force generation of diffs with individual parents\n+of a merge.\n+\n A 'combined diff' format looks like this:\n \n ------------\n-- \n1.7.2.3\n"},{"id":"163043","messageId":"7vtyfdaz4k.fsf@alter.siamese.dyndns.org","threadId":"26663","inReplyTo":"4D7695A9.8070403@gmail.com","subject":"Re: [PATCH v4] Documentation fix: git log -p does not imply -c.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-09T00:58:19Z","receivedAt":"2011-03-09T00:58:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Monsen writes:\n\n> I read some stuff before writing it (like\n> Documentation/SubmittingPatches), but what I should have done is just\n> thumb through the log. Many commit messages are as you say they should be.\n\nSee below...\n\n> The suggested *format* of a commit message is covered in DISCUSSION in\n> git-commit(1). This guide covers philosophy of commit messages.\n>\n> - Read previous commit messages. Emulate the best ones.\n\nI thought you were trying to teach new people how to gauge the \"goodness\"\nwith this list. How would they know which ones are \"best\"? ;-)\n\n> - Answer questions you anticipate others will ask.\n\nNot quite sure. I'd rather see people ask questions to themselves while\nworking on the change, so that the end product does not have even a room\nfor questioning.\n\n> - Reveal your intentions.\n> - Imagine you are reading this same commit message 10 years from now.\n> What would be most helpful for you to quickly recall why these\n> changes were made?\n> - Imagine someone else is reading this same commit message 10 years\n> from now. What would be most helpful for them to quickly understand\n> what this commit changes and why it was done?\n\nThese three points are important and are the same thing. I often\nencourage people to write their logs while keeping these points in mind:\n\n (1) Remember that only the first paragraph is shown in MUA (as Subject:)\n and output from tools like \"git shortlog\", \"git log --oneline\", \"gitk\"\n and \"gitweb\". Say what the change is about concisely and clearly\n there.\n\n A good \"Subject:\" greatly helps me writing Release Notes; if shortlog\n output does not remind me what the change is about, I would need to\n go back to \"git show\", which is very time consuming.\n\n (2) Explain the problem you are trying to solve first. Don't stop at\n saying \"'git xyzzy' ran with the --frotz option does not work\".\n\n Clearly define why you think the current behaviour is broken and what\n you think is expected. I.e. say \"'git xyzzy' ran with the --frotz\n option does not show nitfol from its output; it should\" instead of\n just \"does not work\".\n\n You may discuss earlier design decisions that you do not agree with\n (e.g. \"The commit that introduced the option explains why nitfol does\n not matter under --frotz mode, but it is useful to have in this use\n case...\") here.\n\n The second paragraph is primarily to make sure that future people\n reading the log know what effect the change _wanted_ to make, in case\n the code gets broken later and started doing different things, or the\n change is buggy from day one and didn't work as advertised in the\n commit log message.\n\n A good problem description also helps the reviewers to spot X-Y\n problem at the design level. A patch that addresses a non-existent\n problem is not worth reading nor commenting on---the time is better\n spent on giving an alternative solution to the real problem the patch\n author wanted to address.\n\n (3) Then describe your solution. You may want to give observations on\n the current code (e.g. \"This is because the code in the frotz()\n function that calls xyzzy_output() forgets to set the nitfol flag\")\n if this is about an implementation bug whose solution is subtle.\n\n Optionally discuss alternative approaches you considered, if any, and\n state the reason you ended up solving the problem differently.\n\n This section is to give hints to later people about other approaches\n already attempted and failed (or not tried due to lack of time, but\n may be worth pursuing).\n\n (4) If it is a performance fix/enhancement, benchmarking result would\n come here.\n\nI try to give this list to new contributors early in their initiation\nprocess (ideally before their patches hit the codebase). That is probably\nwhy many of the existing commits you saw in \"git log\" more or less\nconformed to the recommendation.\n\n> - Be verbose!\n\nPlease don't. We want sufficiently detailed description, but we don't\nwant verbosity.\n\n> If you want a guide like this, some questions:\n> * do you want asciidoc, something else, or don't care?\n> * name it Documentation/CommitMessageGuide ? or something else?\n\nI think people who wrote the existing \"Checklist\" section on \"Commits:\"\nmeant to outline what constitutes a good commit log message (look for \"the\nbody should\"). It already says \"motivation\" and \"contrast with previous\nbehaviour\". Cf. 47afed5 (SubmittingPatches: itemize and reflect upon\nwell written changes, 2009-04-28).\n\nI unfortunately don't seem to be able to parse what the last part of what\n47afed5 brought in is trying to say. Perhaps a bad cut-and-paste job?\n\nHow about this as a not-too-verbose compromise?\n\n-- >8 --\nSubject: SubmittingPatches: clarify the expected commit log description\n\nEarlier, 47afed5 (SubmittingPatches: itemize and reflect upon well written\nchanges, 2009-04-28) added a discussion on the contents of the commit log\nmessage, but the last part of the new paragraph didn't make much sense.\nReword it slightly to make it more readable.\n\nUpdate the \"quicklist\" to clarify what we mean by \"motivation\" and\n\"contrast\". Also mildly discourage external references.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/SubmittingPatches | 26 ++++++++++++++++++--------\n 1 files changed, 18 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex 72741eb..c3b0816 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -10,10 +10,18 @@ Checklist (and a short version for the impatient):\n \t description (50 characters is the soft limit, see DISCUSSION\n \t in git-commit(1)), and should skip the full stop\n \t- the body should provide a meaningful commit message, which:\n-\t\t- uses the imperative, present tense: \"change\",\n-\t\t not \"changed\" or \"changes\".\n-\t\t- includes motivation for the change, and contrasts\n-\t\t its implementation with previous behaviour\n+\t . explains the problem the change tries to solve, iow, what\n+\t is wrong with the current code without the change.\n+\t . justifies the way the change solves the problem, iow, why\n+\t the result with the change is better.\n+\t . alternate solutions considered but discarded, if any.\n+\t- describe changes in imperative mood, e.g. \"make xyzzy do frotz\"\n+\t instead of \"[This patch] makes xyzzy do frotz\" or \"[I] changed\n+\t xyzzy to do frotz\", as if you are giving orders to the codebase\n+\t to change its behaviour.\n+\t- try to make sure your explanation can be understood without\n+\t external resources. Instead of giving a URL to a mailing list\n+\t archive, summarize the relevant points of the discussion.\n \t- add a \"Signed-off-by: Your Name \" line to the\n \t commit message (or just use the option \"-s\" when committing)\n \t to confirm that you agree to the Developer's Certificate of Origin\n@@ -90,7 +98,10 @@ your commit head. Instead, always make a commit with complete\n commit message and generate a series of patches from your\n repository. It is a good discipline.\n \n-Describe the technical detail of the change(s).\n+Give an explanation for the change(s) that is detailed enough so\n+that people can judge if it is good thing to do, without reading\n+the actual patch text to determine how well the code does what\n+the explanation promises to do.\n \n If your description starts to get too long, that's a sign that you\n probably need to split up your commit to finer grained pieces.\n@@ -99,9 +110,8 @@ help reviewers check the patch, and future maintainers understand\n the code, are the most beautiful patches. Descriptions that summarise\n the point in the subject well, and describe the motivation for the\n change, the approach taken by the change, and if relevant how this\n-differs substantially from the prior version, can be found on Usenet\n-archives back into the late 80's. Consider it like good Netiquette,\n-but for code.\n+differs substantially from the prior version, are all good things\n+to have.\n \n Oh, another thing. I am picky about whitespaces. Make sure your\n changes do not trigger errors with the sample pre-commit hook shipped\n"},{"id":"163086","messageId":"4D77F03B.4050605@gmail.com","threadId":"26663","inReplyTo":"7vtyfdaz4k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4] Documentation fix: git log -p does not imply -c.","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-09T21:25:15Z","receivedAt":"2011-03-09T21:25:15Z","isPatch":true,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"Junio C Hamano wrote:\n>> - Read previous commit messages. Emulate the best ones.\n> \n> I thought you were trying to teach new people how to gauge the \"goodness\"\n> with this list. How would they know which ones are \"best\"? ;-)\n\nHeh, glad you asked. I was being intentionally vague. The list just\nprovides some ideas. \"Best\" is an ideal. It's subjective. I don't want\npeople to just do what I say, I want to inspire them to do something better.\n\nMaybe:\n\n Read previous commit messages. Strive to make yours better.\n\nAgain, we're talking about commit messages, not raising children or\nanything. But commit messages help others, so why not try to make them the\n\"best\" they can be?\n\n> I try to give this list to new contributors early in their initiation\n> process (ideally before their patches hit the codebase). That is probably\n> why many of the existing commits you saw in \"git log\" more or less\n> conformed to the recommendation.\n\nCool. That's well-written and helpful.\n\n>> - Be verbose!\n> \n> Please don't. We want sufficiently detailed description, but we don't\n> want verbosity.\n\n:) fair enough. I wrote this because, in the Mifos community, I struggle\ngetting people to write enough of *anything* in commit messages. Good to\nknow that's not a problem in git land.\n\n> How about this as a not-too-verbose compromise?\n\nI like it. Some specific comments below.\n\n> -\t\t- includes motivation for the change, and contrasts\n> -\t\t its implementation with previous behaviour\n> +\t . explains the problem the change tries to solve, iow, what\n> +\t is wrong with the current code without the change.\n\nGood, but spell out \"in other words\". I had to look up that acronym.\n\n> +\t . justifies the way the change solves the problem, iow, why\n> +\t the result with the change is better.\n\nDitto.\n\n> +\t . alternate solutions considered but discarded, if any.\n\nPrepend with \"mentions\" or something.\n\n> +\t- try to make sure your explanation can be understood without\n> +\t external resources. Instead of giving a URL to a mailing list\n> +\t archive, summarize the relevant points of the discussion.\n\n+1, but I prefer both the summary *and* the link.\n\nSo maybe:\n\n Instead of providing only a URL to a mailing list archive, summarize\n the relevant points of the discussion.\n\n> -Describe the technical detail of the change(s).\n> +Give an explanation for the change(s) that is detailed enough so\n> +that people can judge if it is good thing to do, without reading\n> +the actual patch text to determine how well the code does what\n> +the explanation promises to do.\n\n+1!\n\nI noticed a few grammar errors in the doc, too. I'll reply with\nanother patch.\n\nSorry if this continued work on the SubmittingPatches\ndocumentation is getting annoying. It's been useful for me to learn\nhow things are done around here. And it's important that the\ndocument be well written, so people actually use it. I just thought\nwe might as well improve it a little more since we've already started.\n"},{"id":"163088","messageId":"1299706069-5463-1-git-send-email-haircut@gmail.com","threadId":"26663","inReplyTo":"4D77F03B.4050605@gmail.com","subject":"[PATCH] SubmittingPatches: clean up commit message tips","fromName":"Adam Monsen","fromEmail":"haircut@gmail.com","sentAt":"2011-03-09T21:27:49Z","receivedAt":"2011-03-09T21:27:49Z","isPatch":true,"sender":{"key":"haircut@gmail.com","avatar":"https://avatars.githubusercontent.com/u/50639?v=4"},"body":"Removed uncommon acronyms in the \"Checklist\" section. I had to look up\n\"iow\" online, but this documentation should stand alone (without online\naccess) as well as commit messages.\n\nLeave wiggle room for including URLs in commit messages. Having both\nsummaries of mailing list discussions as well as URLs in commit messages\ngives the reader the choice of how deep into history they wish to dig.\n\nModify the section about trivial changes slightly... it makes more sense\nthat it is discouraging diffs pasted in emails as opposed to patches\ngenerated with \"git am\".\n\nRemove recommendations on commit messages from the \"Make separate\ncommits for logically separate changes\" section, they are covered\nwell and explicitly in the \"Checklist\" section on top. This section need\nnot repeat same, and the deleted text doesn't really apply to the\nsection it was under. Also remove irrelevant text about good commit\nmessages. The only relevant part of that paragraph was the first\nsentence about breaking apart big commits into separate patches.\n\nConsistently use the phrase \"commit message\" to mean the chunk of text\nthat describes a commit.\n\nAlso in \"Checklist\", make the third bullet under \"...a meaningful\ncommit message, which:\" flow better grammatically by adding \"mentions\".\n\nRelevant mailing list discussion:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/168481/focus=168717\n\nSigned-off-by: Adam Monsen \n---\n\nI realize that this commit probably violates the very virtues it recommends by\nhaving a long commit message and fixing several things in one shot. Let me know\nif you'd like me to break it apart. Also, I may of course have made further\nerrors. It does adhere to the subject of cleaning up commit message tips.\n\nHope this helps,\n-Adam\n\n Documentation/SubmittingPatches | 38 ++++++++++++++++----------------------\n 1 files changed, 16 insertions(+), 22 deletions(-)\n\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex c3b0816..3f8bff5 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -10,18 +10,19 @@ Checklist (and a short version for the impatient):\n \t description (50 characters is the soft limit, see DISCUSSION\n \t in git-commit(1)), and should skip the full stop\n \t- the body should provide a meaningful commit message, which:\n-\t . explains the problem the change tries to solve, iow, what\n-\t is wrong with the current code without the change.\n-\t . justifies the way the change solves the problem, iow, why\n-\t the result with the change is better.\n-\t . alternate solutions considered but discarded, if any.\n+\t . explains the problem the change tries to solve, in other words,\n+\t what is wrong with the current code without the change.\n+\t . justifies the way the change solves the problem, in other words,\n+\t why the result with the change is better.\n+\t . mentions alternate solutions considered but discarded, if any.\n \t- describe changes in imperative mood, e.g. \"make xyzzy do frotz\"\n \t instead of \"[This patch] makes xyzzy do frotz\" or \"[I] changed\n \t xyzzy to do frotz\", as if you are giving orders to the codebase\n \t to change its behaviour.\n \t- try to make sure your explanation can be understood without\n-\t external resources. Instead of giving a URL to a mailing list\n-\t archive, summarize the relevant points of the discussion.\n+\t external resources. Instead of providing only a URL to a\n+\t mailing list archive, summarize the relevant points of the\n+\t discussion.\n \t- add a \"Signed-off-by: Your Name \" line to the\n \t commit message (or just use the option \"-s\" when committing)\n \t to confirm that you agree to the Developer's Certificate of Origin\n@@ -52,14 +53,14 @@ Checklist (and a short version for the impatient):\n \n Long version:\n \n-I started reading over the SubmittingPatches document for Linux\n+I started reading over the SubmittingPatches document for the Linux\n kernel, primarily because I wanted to have a document similar to\n-it for the core GIT to make sure people understand what they are\n-doing when they write \"Signed-off-by\" line.\n+it for core GIT to make sure people understand what they are\n+doing when they include a \"Signed-off-by\" line.\n \n But the patch submission requirements are a lot more relaxed\n-here on the technical/contents front, because the core GIT is\n-thousand times smaller ;-). So here is only the relevant bits.\n+here on the technical/contents front, because the core GIT is a\n+thousand times smaller ;-). So here are just the relevant bits.\n \n (0) Decide what to base your work on.\n \n@@ -93,7 +94,7 @@ commit is the tip of the topic branch.\n (1) Make separate commits for logically separate changes.\n \n Unless your patch is really trivial, you should not be sending\n-out a patch that was generated between your working tree and\n+out a diff that was generated between your working tree and\n your commit head. Instead, always make a commit with complete\n commit message and generate a series of patches from your\n repository. It is a good discipline.\n@@ -103,15 +104,8 @@ that people can judge if it is good thing to do, without reading\n the actual patch text to determine how well the code does what\n the explanation promises to do.\n \n-If your description starts to get too long, that's a sign that you\n-probably need to split up your commit to finer grained pieces.\n-That being said, patches which plainly describe the things that\n-help reviewers check the patch, and future maintainers understand\n-the code, are the most beautiful patches. Descriptions that summarise\n-the point in the subject well, and describe the motivation for the\n-change, the approach taken by the change, and if relevant how this\n-differs substantially from the prior version, are all good things\n-to have.\n+If your commit message starts to get too long, that's a sign that you\n+probably need to split your commit into finer grained pieces.\n \n Oh, another thing. I am picky about whitespaces. Make sure your\n changes do not trigger errors with the sample pre-commit hook shipped\n-- \n1.7.2.3\n"},{"id":"163095","messageId":"7voc5k7x76.fsf@alter.siamese.dyndns.org","threadId":"26663","inReplyTo":"1299706069-5463-1-git-send-email-haircut@gmail.com","subject":"Re: [PATCH] SubmittingPatches: clean up commit message tips","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-09T22:20:29Z","receivedAt":"2011-03-09T22:20:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Monsen writes:\n\n> Removed uncommon acronyms in the \"Checklist\" section. I had to look up\n> \"iow\" online, but this documentation should stand alone (without online\n> access) as well as commit messages.\n\nWe have only a handful of instances of that phrase in the codebase (run\n\"git grep\" for them), so I am not strongly opposed to spelling it out, but\nuse of IOW is quite common on this list---I suspect that it is largely\nbecause Linus uses the phrase quite often. Cf.\n\n $ git log | grep -e IOW -e '^Author: ' | grep -B1 -e IOW\n\n> Leave wiggle room for including URLs in commit messages.\n\nI don't think the updated text is too bad, but I don't very much like the\nabove \"wiggle room\".\n\nThe guideline is written to suggest what you absolutely should include; it\nis obviously Ok to add other things as necessary. If common sense tells\nthe reader external references will help recollection, the guideline does\nnot forbid to include them. IOW, there are enough wiggle rooms already.\n\n> Modify the section about trivial changes slightly... it makes more sense\n> that it is discouraging diffs pasted in emails as opposed to patches\n> generated with \"git am\".\n\ns/am/format-patch/;\n\n> Remove recommendations on commit messages from the \"Make separate\n> commits for logically separate changes\" section,...\n> sentence about breaking apart big commits into separate patches.\n\nWhile I do not particularly hate this part, I think people who did the\n\"Checklist vs Long Version\" meant to make each of them stand on its own.\nLazy people (or people who think they are experienced enough) read the\nformer, while the others who pride themselves being thorough will skip the\n\"for-lazy-people\" digest version and read only \"the real thing\".\n\nSo overall, I am not enthused by this version. Input from others may\nbe appreciated.\n"}]}