{"thread":{"id":"26609","subject":"git diff: add option for omitting the contents of deletes","startedAt":"2011-02-26T13:16:46Z","lastAt":"2011-03-08T21:31:34Z","messageCount":32,"participants":["Mart Sõmermaa","Junio C Hamano","Michael J Gruber","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"162313","messageId":"AANLkTi=++gcw5CDQnTRbhUGkvWc9Zm+Ct4Zm5oGGYKUh@mail.gmail.com","threadId":"26609","inReplyTo":null,"subject":"git diff: add option for omitting the contents of deletes","fromName":"Mart Sõmermaa","fromEmail":"mrts.pydev@gmail.com","sentAt":"2011-02-26T13:16:46Z","receivedAt":"2011-02-26T13:16:46Z","isPatch":false,"sender":{"key":"mrts.pydev@gmail.com","avatar":"https://gravatar.com/avatar/52597879a4c009c603b84516a2169a125b875c692534bbfc1968cb42bfe28f16?d=mp&s=160"},"body":"Paging through deletes adds a cognitive burden when reviewing code.\nThus we have\n\n git config --global alias.deleteless-diff 'diff -M --diff-filter=ACMRTUXB'\n\nand using `git deleteless-diff` is standard practice during code reviews.\n\nHowever, the filter leaves out delete information altogether, which is\nerror-prone.\n\nTherefore I'd like to propose adding the -D (or\n--omit-delete-contents) option to git diff\nthat would behave as follows:\n\n$ git rm foo\n$ git diff -D\ndiff --git a/foo b/foo\ndelete foo\n\nI.e. instead of displaying the contents of the file as a diff minus, a\ndescriptive message\nis displayed -- like with renames.\n\nThoughts, suggestions?\nCan 'diff -M --diff-filter=ACMRTUXB' be improved?\n\nBest regards,\nMart Sõmermaa\n"},{"id":"162322","messageId":"7vk4gm7dz3.fsf@alter.siamese.dyndns.org","threadId":"26609","inReplyTo":"AANLkTi=++gcw5CDQnTRbhUGkvWc9Zm+Ct4Zm5oGGYKUh@mail.gmail.com","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-26T20:11:44Z","receivedAt":"2011-02-26T20:11:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mart Sõmermaa writes:\n\n> Paging through deletes adds a cognitive burden when reviewing code.\n\nWasn't the ability to say '/^diff --git ' in your pager invented for\nexactly for that?\n"},{"id":"162364","messageId":"4D6A6291.8050206@drmicha.warpmail.net","threadId":"26609","inReplyTo":"7vk4gm7dz3.fsf@alter.siamese.dyndns.org","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-27T14:41:21Z","receivedAt":"2011-02-27T14:41:21Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 26.02.2011 21:11:\n> Mart Sõmermaa writes:\n> \n>> Paging through deletes adds a cognitive burden when reviewing code.\n> \n> Wasn't the ability to say '/^diff --git ' in your pager invented for\n> exactly for that?\n\nWasn't the pager invented for sifting through output which has to be\nseveral pages, but not not for that which could be more concise? ;)\n\nIn fact, -D would be quite analogous to -M and -C in that respect.\n\nMichael\n"},{"id":"162386","messageId":"AANLkTik+MPPYOm+O5cmFur_2DKKy0bt9i0XeuKbHbaJU@mail.gmail.com","threadId":"26609","inReplyTo":"4D6A6291.8050206@drmicha.warpmail.net","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Mart Sõmermaa","fromEmail":"mrts.pydev@gmail.com","sentAt":"2011-02-27T22:33:07Z","receivedAt":"2011-02-27T22:33:07Z","isPatch":false,"sender":{"key":"mrts.pydev@gmail.com","avatar":"https://gravatar.com/avatar/52597879a4c009c603b84516a2169a125b875c692534bbfc1968cb42bfe28f16?d=mp&s=160"},"body":"Junio, jokes aside, do you think this is useful?\nWould you accept a patch that implements this?\n\nMicheal, thanks for the indirect + :). Is it +0 or +1?\n\nBest,\nMS\n\nOn Sun, Feb 27, 2011 at 4:41 PM, Michael J Gruber\n wrote:\n> Junio C Hamano venit, vidit, dixit 26.02.2011 21:11:\n>> Mart Sõmermaa writes:\n>>\n>>> Paging through deletes adds a cognitive burden when reviewing code.\n>>\n>> Wasn't the ability to say '/^diff --git ' in your pager invented for\n>> exactly for that?\n>\n> Wasn't the pager invented for sifting through output which has to be\n> several pages, but not not for that which could be more concise? ;)\n>\n> In fact, -D would be quite analogous to -M and -C in that respect.\n>\n> Michael\n>\n"},{"id":"162388","messageId":"7v7hclulz0.fsf@alter.siamese.dyndns.org","threadId":"26609","inReplyTo":"4D6A6291.8050206@drmicha.warpmail.net","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-27T22:54:59Z","receivedAt":"2011-02-27T22:54:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Wasn't the pager invented for sifting through output which has to be\n> several pages, but not not for that which could be more concise? ;)\n>\n> In fact, -D would be quite analogous to -M and -C in that respect.\n\nThere is a big difference: -M and -C lets your recipient reproduce the\nstate using the change you are trying to convey with the diff output in\neither direction (iow, \"apply -R\" works), but your \"-D\" would not have\nthat property.\n"},{"id":"162389","messageId":"7v39n9uldp.fsf@alter.siamese.dyndns.org","threadId":"26609","inReplyTo":"7v7hclulz0.fsf@alter.siamese.dyndns.org","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-27T23:07:46Z","receivedAt":"2011-02-27T23:07:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Michael J Gruber writes:\n>\n>> Wasn't the pager invented for sifting through output which has to be\n>> several pages, but not not for that which could be more concise? ;)\n>>\n>> In fact, -D would be quite analogous to -M and -C in that respect.\n>\n> There is a big difference: -M and -C lets your recipient reproduce the\n> state using the change you are trying to convey with the diff output in\n> either direction (iow, \"apply -R\" works), but your \"-D\" would not have\n> that property.\n\nHaving said that we have always valued \"reversibility\" and a casual -D is\nnot in line with that principle, I don't have a strong objection if the\nnew mode of operation is marked clearly as \"nonusable if you are trying to\nproduce appliable diff (iow, don't send such a patch to mailing list--it\nis for viewing purposes only)\", treating it just like the --color-words\nand the --stat options (there isn't even need to mark these as unusable\nfor that purpose, as people with common sense would be able to guess).\n\nIf we were to do this, it probably is a good idea to apply that for a\ntypechange patch (the one that is produced when a symlink turns into a\nregular file and vice versa) as well. It also might make sense to apply\nthe similar principle to shorten the output with -B when a rewrite patch\nis expressed as a single hunk patch that removes everything old and then\nadds everthing new.\n"},{"id":"162403","messageId":"4D6B4F6B.1040209@drmicha.warpmail.net","threadId":"26609","inReplyTo":"7v39n9uldp.fsf@alter.siamese.dyndns.org","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-28T07:31:55Z","receivedAt":"2011-02-28T07:31:55Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 28.02.2011 00:07:\n> Junio C Hamano writes:\n> \n>> Michael J Gruber writes:\n>>\n>>> Wasn't the pager invented for sifting through output which has to be\n>>> several pages, but not not for that which could be more concise? ;)\n>>>\n>>> In fact, -D would be quite analogous to -M and -C in that respect.\n>>\n>> There is a big difference: -M and -C lets your recipient reproduce the\n>> state using the change you are trying to convey with the diff output in\n>> either direction (iow, \"apply -R\" works), but your \"-D\" would not have\n>> that property.\n> \n> Having said that we have always valued \"reversibility\" and a casual -D is\n\nI didn't know, but I guess I haven't come across those issues yet.\n\n> not in line with that principle, I don't have a strong objection if the\n> new mode of operation is marked clearly as \"nonusable if you are trying to\n> produce appliable diff (iow, don't send such a patch to mailing list--it\n> is for viewing purposes only)\", treating it just like the --color-words\n> and the --stat options (there isn't even need to mark these as unusable\n> for that purpose, as people with common sense would be able to guess).\n\nYes, it is purely intended as \"for viewing by humans\". Even in the\nforward direction saying \"delete that path\" is different from saying\n\"delete that content from that path\", i.e. \"diff -D\" output may apply\nwithout conflicts in cases where \"diff\" output does not.\n\nThat aspect is similar to -M and -C, though - unless we check the sha1\nof the blobs before applying the patch (which would be possible for -D\nalso) - do we?\n\n> If we were to do this, it probably is a good idea to apply that for a\n> typechange patch (the one that is produced when a symlink turns into a\n> regular file and vice versa) as well. It also might make sense to apply\n> the similar principle to shorten the output with -B when a rewrite patch\n> is expressed as a single hunk patch that removes everything old and then\n> adds everthing new.\n\nReminds me of my failed attempt to make the diff output for symlinks\nmore human-friendly. The latter can be solved with textconv, though.\n\nMichael\n"},{"id":"162423","messageId":"4D6B71C2.6080203@drmicha.warpmail.net","threadId":"26609","inReplyTo":"AANLkTik+MPPYOm+O5cmFur_2DKKy0bt9i0XeuKbHbaJU@mail.gmail.com","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-28T09:58:26Z","receivedAt":"2011-02-28T09:58:26Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Mart Sõmermaa venit, vidit, dixit 27.02.2011 23:33:\n> Junio, jokes aside, do you think this is useful?\n> Would you accept a patch that implements this?\n> \n> Micheal, thanks for the indirect + :). Is it +0 or +1?\n\nIt's -1 for the spelling of my name (unless it is correct in your\nlanguage), and +1 for having \"-D\". As Junio explained, it is important\nto have this only for \"viewing diffs\" (like the color stuff) and not in\nthe way of generation of patches which are meant to be applied (with -R,\npossibly), so that one can have it \"on\" by default for viewing. OTOH, if\n(it's off by default, naturally, and) there's no config and just let\npeople have their option (like --color-words) I don't see a problem.\n\nMichael\n"},{"id":"162426","messageId":"AANLkTim-vyWhgmFuewbd5e5iJiTC-EHz7g=efr2Sz-RH@mail.gmail.com","threadId":"26609","inReplyTo":"7v39n9uldp.fsf@alter.siamese.dyndns.org","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Mart Sõmermaa","fromEmail":"mrts.pydev@gmail.com","sentAt":"2011-02-28T10:45:35Z","receivedAt":"2011-02-28T10:45:35Z","isPatch":false,"sender":{"key":"mrts.pydev@gmail.com","avatar":"https://gravatar.com/avatar/52597879a4c009c603b84516a2169a125b875c692534bbfc1968cb42bfe28f16?d=mp&s=160"},"body":"Why not just make it reversible then?\n\n$ git diff -M\ndiff --git a/foo b/bar\nsimilarity index 100%\nrename from foo\nrename to bar\n\nis nonreversible without git already (i.e. does not work with plain\npatch AFAIK).\n\nAdding\n\n$ git diff -D\ndiff --git a/foo b/foo\ndeleted file mode 100644\ndelete foo\n\nwould be neither less nor more reversible -- it would also only work\nwith git apply\n(assuming that apply is amended accordingly).\n\nCognitive burdens aside, the \"delete foo\" output is both\nmore explicit and shorter anyway. Occam's razor FTW :)!\n\nJunio, I won't push this further, so it's a final call -- if you give -1,\nthen let -D fall to the colourless abyss of oblivion :), if +1 or +0,\nI'll see if I can come up with a patch.\n\nBest regards,\nMS\n\nOn Mon, Feb 28, 2011 at 1:07 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Michael J Gruber writes:\n>>\n>>> Wasn't the pager invented for sifting through output which has to be\n>>> several pages, but not not for that which could be more concise? ;)\n>>>\n>>> In fact, -D would be quite analogous to -M and -C in that respect.\n>>\n>> There is a big difference: -M and -C lets your recipient reproduce the\n>> state using the change you are trying to convey with the diff output in\n>> either direction (iow, \"apply -R\" works), but your \"-D\" would not have\n>> that property.\n>\n> Having said that we have always valued \"reversibility\" and a casual -D is\n> not in line with that principle, I don't have a strong objection if the\n> new mode of operation is marked clearly as \"nonusable if you are trying to\n> produce appliable diff (iow, don't send such a patch to mailing list--it\n> is for viewing purposes only)\", treating it just like the --color-words\n> and the --stat options (there isn't even need to mark these as unusable\n> for that purpose, as people with common sense would be able to guess).\n>\n> If we were to do this, it probably is a good idea to apply that for a\n> typechange patch (the one that is produced when a symlink turns into a\n> regular file and vice versa) as well.  It also might make sense to apply\n> the similar principle to shorten the output with -B when a rewrite patch\n> is expressed as a single hunk patch that removes everything old and then\n> adds everthing new.\n>\n"},{"id":"162427","messageId":"AANLkTikZwG8tpUZWU+1OEGi8trKY6=NqHqkL5T+b1nDS@mail.gmail.com","threadId":"26609","inReplyTo":"4D6B71C2.6080203@drmicha.warpmail.net","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Mart Sõmermaa","fromEmail":"mrts.pydev@gmail.com","sentAt":"2011-02-28T10:51:17Z","receivedAt":"2011-02-28T10:51:17Z","isPatch":false,"sender":{"key":"mrts.pydev@gmail.com","avatar":"https://gravatar.com/avatar/52597879a4c009c603b84516a2169a125b875c692534bbfc1968cb42bfe28f16?d=mp&s=160"},"body":"On Mon, Feb 28, 2011 at 11:58 AM, Michael J Gruber\n wrote:\n> Mart Sõmermaa venit, vidit, dixit 27.02.2011 23:33:\n>> Junio, jokes aside, do you think this is useful?\n>> Would you accept a patch that implements this?\n>>\n>> Micheal, thanks for the indirect + :). Is it +0 or +1?\n>\n> It's -1 for the spelling of my name (unless it is correct in your\n> language\n\nI'm terribly sorry :( and no, it is not.\n\nTo make amends, I hereby officially grant you\nthe right to call me Marat or Marta (which is the\nfemale form of Mart) or Mrat or Mrata from now on :).\n\nDygsrahpically yours,\nMrat\n"},{"id":"162430","messageId":"20110228121726.GA5197@sigill.intra.peff.net","threadId":"26609","inReplyTo":"4D6B4F6B.1040209@drmicha.warpmail.net","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-28T12:17:26Z","receivedAt":"2011-02-28T12:17:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 28, 2011 at 08:31:55AM +0100, Michael J Gruber wrote:\n\n> > Junio C Hamano writes:\n> >>\n> >> There is a big difference: -M and -C lets your recipient reproduce the\n> >> state using the change you are trying to convey with the diff output in\n> >> either direction (iow, \"apply -R\" works), but your \"-D\" would not have\n> >> that property.\n>\n> [...]\n>\n> Yes, it is purely intended as \"for viewing by humans\". Even in the\n> forward direction saying \"delete that path\" is different from saying\n> \"delete that content from that path\", i.e. \"diff -D\" output may apply\n> without conflicts in cases where \"diff\" output does not.\n> \n> That aspect is similar to -M and -C, though - unless we check the sha1\n> of the blobs before applying the patch (which would be possible for -D\n> also) - do we?\n\nYes, I think we do check the sha1s for a \"-M\" patch. And we should do so\nfor a \"-D\" patch, too. Which would make it just as likely to conflict as\na version with the actual patch content, _except_ that it relies on the\nrecipient having that sha1.\n\nI don't see how \"-D\" is any less reversible than -M or -C, though. If I\nget your -D patch, I use the index line to see that the blob went from\n1234abcd to 0000000, check that we are at 1234abcd, and then delete the\nfile. To reverse it, I reinstate 1234abcd from nothing (and conflict if\nthe file exists). _Neither_ case works without the sha1.\n\nSo I think the problem is not about \"this cannot be reversed\" but about\n\"the recipient must have your sha1 to make sense of it, in either\ndirection\". Which is the same case as with \"-M\" and \"-C\", and why we\nhave long cautioned about their use on mailing lists. But the right rule\nis not \"do not use on mailing lists\" but rather \"do not use on mailing\nlists for projects where the recipients will not be using git to apply\".\n\n> Reminds me of my failed attempt to make the diff output for symlinks\n> more human-friendly. The latter can be solved with textconv, though.\n\nI am still carrying around my \"symlinks as a special class of diff\"\npatches if you are interested:\n\n https://github.com/peff/git/tree/jk/userdiff-symlinks\n\n-Peff\n"},{"id":"162431","messageId":"20110228122335.GB5197@sigill.intra.peff.net","threadId":"26609","inReplyTo":"20110228121726.GA5197@sigill.intra.peff.net","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-28T12:23:35Z","receivedAt":"2011-02-28T12:23:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 28, 2011 at 07:17:26AM -0500, Jeff King wrote:\n\n> > That aspect is similar to -M and -C, though - unless we check the sha1\n> > of the blobs before applying the patch (which would be possible for -D\n> > also) - do we?\n> \n> Yes, I think we do check the sha1s for a \"-M\" patch. And we should do so\n> for a \"-D\" patch, too. Which would make it just as likely to conflict as\n> a version with the actual patch content, _except_ that it relies on the\n> recipient having that sha1.\n> \n> I don't see how \"-D\" is any less reversible than -M or -C, though. If I\n> get your -D patch, I use the index line to see that the blob went from\n> 1234abcd to 0000000, check that we are at 1234abcd, and then delete the\n> file. To reverse it, I reinstate 1234abcd from nothing (and conflict if\n> the file exists). _Neither_ case works without the sha1.\n> \n> So I think the problem is not about \"this cannot be reversed\" but about\n> \"the recipient must have your sha1 to make sense of it, in either\n> direction\". Which is the same case as with \"-M\" and \"-C\", and why we\n> have long cautioned about their use on mailing lists. But the right rule\n> is not \"do not use on mailing lists\" but rather \"do not use on mailing\n> lists for projects where the recipients will not be using git to apply\".\n\nActually, thinking on this a bit more, I guess \"-M\" and \"-C\" are usable\nwithout the sha1. In fact, we don't even provide it for a strict 100%\nrename, and for a rename-with-patch, you can apply the patch, assuming\nyou have the original file in any form. So they are really about \"is\nyour recipient using git\", not \"is your recipient using git _and_ will\nhe/she have the right sha1\".\n\nI do still think that in practice among git users that \"-D\" patches will\nbe able to be applied. You won't typically be sending a patch to delete\nsomething that other people don't have. I would expect most uses of\n\"apply -R\" to be reverting an existing delete, which means you will have\nthe sha1.\n\n-Peff\n"},{"id":"162432","messageId":"4D6B95E3.7040603@drmicha.warpmail.net","threadId":"26609","inReplyTo":"20110228122335.GB5197@sigill.intra.peff.net","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-28T12:32:35Z","receivedAt":"2011-02-28T12:32:35Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 28.02.2011 13:23:\n> On Mon, Feb 28, 2011 at 07:17:26AM -0500, Jeff King wrote:\n> \n>>> That aspect is similar to -M and -C, though - unless we check the sha1\n>>> of the blobs before applying the patch (which would be possible for -D\n>>> also) - do we?\n>>\n>> Yes, I think we do check the sha1s for a \"-M\" patch. And we should do so\n>> for a \"-D\" patch, too. Which would make it just as likely to conflict as\n>> a version with the actual patch content, _except_ that it relies on the\n>> recipient having that sha1.\n>>\n>> I don't see how \"-D\" is any less reversible than -M or -C, though. If I\n>> get your -D patch, I use the index line to see that the blob went from\n>> 1234abcd to 0000000, check that we are at 1234abcd, and then delete the\n>> file. To reverse it, I reinstate 1234abcd from nothing (and conflict if\n>> the file exists). _Neither_ case works without the sha1.\n>>\n>> So I think the problem is not about \"this cannot be reversed\" but about\n>> \"the recipient must have your sha1 to make sense of it, in either\n>> direction\". Which is the same case as with \"-M\" and \"-C\", and why we\n>> have long cautioned about their use on mailing lists. But the right rule\n>> is not \"do not use on mailing lists\" but rather \"do not use on mailing\n>> lists for projects where the recipients will not be using git to apply\".\n> \n> Actually, thinking on this a bit more, I guess \"-M\" and \"-C\" are usable\n> without the sha1. In fact, we don't even provide it for a strict 100%\n> rename, and for a rename-with-patch, you can apply the patch, assuming\n> you have the original file in any form. So they are really about \"is\n> your recipient using git\", not \"is your recipient using git _and_ will\n> he/she have the right sha1\".\n\n$ git mv Makefile Dofile\n$ git staged # yadayada\ndiff --git c/Makefile i/Dofile\nsimilarity index 100%\nrename from Makefile\nrename to Dofile\n\nSame with copy.\n\nBut that's not good, is it? I mean, Alice sends me her \"copy patch\" and\nI send her my Makefile patch, both on top of the same base. We both\napply each other's patch cleanly. We end up with different \"Dofile\".\nChecking the sha1 would prevent this. It's no surprise that patch\napplication is non-commutative, but shouldn't we catch this?\n\nMichael\n"},{"id":"162434","messageId":"4D6B981E.5090206@drmicha.warpmail.net","threadId":"26609","inReplyTo":"20110228121726.GA5197@sigill.intra.peff.net","subject":"symling diff driver (Was: Re: git diff: add option for omitting the contents of deletes)","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-28T12:42:06Z","receivedAt":"2011-02-28T12:42:06Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 28.02.2011 13:17:\n> On Mon, Feb 28, 2011 at 08:31:55AM +0100, Michael J Gruber wrote:\n> \n[non symlink stuff snipped]\n>> Reminds me of my failed attempt to make the diff output for symlinks\n>> more human-friendly. The latter can be solved with textconv, though.\n> \n> I am still carrying around my \"symlinks as a special class of diff\"\n> patches if you are interested:\n> \n> https://github.com/peff/git/tree/jk/userdiff-symlinks\n\nThis is marvelous (except for s/perl -pe/sed -e/, of course).\n\nIs there anything left to do to get this in pu? (The reference to\n\"previous patch\" may need to be more explicit.)\n\nMichael\n"},{"id":"162435","messageId":"20110228125909.GA9054@sigill.intra.peff.net","threadId":"26609","inReplyTo":"4D6B95E3.7040603@drmicha.warpmail.net","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-28T12:59:09Z","receivedAt":"2011-02-28T12:59:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 28, 2011 at 01:32:35PM +0100, Michael J Gruber wrote:\n\n> > Actually, thinking on this a bit more, I guess \"-M\" and \"-C\" are usable\n> > without the sha1. In fact, we don't even provide it for a strict 100%\n> > rename, and for a rename-with-patch, you can apply the patch, assuming\n> > you have the original file in any form. So they are really about \"is\n> > your recipient using git\", not \"is your recipient using git _and_ will\n> > he/she have the right sha1\".\n> \n> $ git mv Makefile Dofile\n> $ git staged # yadayada\n> diff --git c/Makefile i/Dofile\n> similarity index 100%\n> rename from Makefile\n> rename to Dofile\n> \n> Same with copy.\n> \n> But that's not good, is it? I mean, Alice sends me her \"copy patch\" and\n> I send her my Makefile patch, both on top of the same base. We both\n> apply each other's patch cleanly. We end up with different \"Dofile\".\n> Checking the sha1 would prevent this. It's no surprise that patch\n> application is non-commutative, but shouldn't we catch this?\n\nWon't you either get a conflict or end up with the same Dofile? Clearly\nyou will have a Dofile with your Makefile changes, as you applied the\nmovement on top of your changes.\n\nAlice will either:\n\n 1. Apply not using rename detection (e.g., not using git, or using\n \"git am\" without \"-3\"). In this case, she gets a conflict because\n she no longer has Makefile.\n\n 2. Apply using rename detection (e.g., via \"git am -3\"). In this case,\n we will notice the movement of Makefile to Dofile, and apply the\n patch to Dofile.\n\nStill, I do wonder if we should be including an index line on a straight\nrename patch. It lets the recipient check that what is being renamed is\nwhat they have (IOW, it gives the same check that they would do if they\nahd the whole patch text). And then the recipient can decide how to\nresolve the conflict.\n\n-Peff\n"},{"id":"162438","messageId":"4D6B9DA1.4070805@drmicha.warpmail.net","threadId":"26609","inReplyTo":"20110228125909.GA9054@sigill.intra.peff.net","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-28T13:05:37Z","receivedAt":"2011-02-28T13:05:37Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 28.02.2011 13:59:\n> On Mon, Feb 28, 2011 at 01:32:35PM +0100, Michael J Gruber wrote:\n> \n>>> Actually, thinking on this a bit more, I guess \"-M\" and \"-C\" are usable\n>>> without the sha1. In fact, we don't even provide it for a strict 100%\n>>> rename, and for a rename-with-patch, you can apply the patch, assuming\n>>> you have the original file in any form. So they are really about \"is\n>>> your recipient using git\", not \"is your recipient using git _and_ will\n>>> he/she have the right sha1\".\n>>\n>> $ git mv Makefile Dofile\n>> $ git staged # yadayada\n>> diff --git c/Makefile i/Dofile\n>> similarity index 100%\n>> rename from Makefile\n>> rename to Dofile\n>>\n>> Same with copy.\n>>\n>> But that's not good, is it? I mean, Alice sends me her \"copy patch\" and\n>> I send her my Makefile patch, both on top of the same base. We both\n>> apply each other's patch cleanly. We end up with different \"Dofile\".\n>> Checking the sha1 would prevent this. It's no surprise that patch\n>> application is non-commutative, but shouldn't we catch this?\n> \n> Won't you either get a conflict or end up with the same Dofile? Clearly\n> you will have a Dofile with your Makefile changes, as you applied the\n> movement on top of your changes.\n> \n> Alice will either:\n> \n> 1. Apply not using rename detection (e.g., not using git, or using\n> \"git am\" without \"-3\"). In this case, she gets a conflict because\n> she no longer has Makefile.\n> \n\nWhen I said \"copy patch\" I actually meant a patch which records the copy\n\"Makefile -> Dofile\". What is it today? Is it me? I know I wrote the\n\"mv\" example first, but still :)\n\nI mean, Alice:\ncp Makefile Dofile\nsends me a -C patch\n\nI:\nBreak everything by hacking Makefile\nsend her a crappy patch\n\nBoth:\napply the received patch\n\nNow I end up with a borked Makefile and a borked Dofile, but Alice still\nhas a good Dofile, and it's all my fault, so I don't deserve any better.\nBut still.\n\n> 2. Apply using rename detection (e.g., via \"git am -3\"). In this case,\n> we will notice the movement of Makefile to Dofile, and apply the\n> patch to Dofile.\n> \n> Still, I do wonder if we should be including an index line on a straight\n> rename patch. It lets the recipient check that what is being renamed is\n> what they have (IOW, it gives the same check that they would do if they\n> ahd the whole patch text). And then the recipient can decide how to\n> resolve the conflict.\n\nI think so. (Or make -f force it.)\n\nThis is orthogonal to the \"-D\" suggestion\", but \"-D\" could write the\nindex line to start with.\n\nMichael\n"},{"id":"162437","messageId":"20110228130850.GB9054@sigill.intra.peff.net","threadId":"26609","inReplyTo":"4D6B981E.5090206@drmicha.warpmail.net","subject":"Re: symling diff driver (Was: Re: git diff: add option for omitting the contents of deletes)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-28T13:08:50Z","receivedAt":"2011-02-28T13:08:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 28, 2011 at 01:42:06PM +0100, Michael J Gruber wrote:\n\n> > I am still carrying around my \"symlinks as a special class of diff\"\n> > patches if you are interested:\n> > \n> > https://github.com/peff/git/tree/jk/userdiff-symlinks\n> \n> This is marvelous (except for s/perl -pe/sed -e/, of course).\n\nI used perl because many older seds have trouble with files not ending\nin newline.\n\n> Is there anything left to do to get this in pu? (The reference to\n> \"previous patch\" may need to be more explicit.)\n\nSee this subthread for discussion:\n\n http://article.gmane.org/gmane.comp.version-control.git/156760\n\nOn the one hand, there were doubts that anybody would actually want to\nuse it (though it looks like you may be the counterexample to that). On\nthe other hand, it may be that the solution does not go far enough, and\nthat .gitattributes should become aware of file types in its matching.\nThat would be more flexible, and it would also fix other broken areas\n(like the fact that a symlink to foo.pdf might have its symlink text\nmerged as if it were a pdf).\n\n-Peff\n"},{"id":"162445","messageId":"f35b1b4b57a563f57f169602440b3b0abdef5a07.1298906628.git.git@drmicha.warpmail.net","threadId":"26609","inReplyTo":"20110228130850.GB9054@sigill.intra.peff.net","subject":"[PATCH/WIP] attr: make attributes depend on file type","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-28T15:26:18Z","receivedAt":"2011-02-28T15:26:18Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"This is a whether balloon patch (to check whether this balloons or not,\nuhm). Hit it hard, not me ;)\n\nIt's part of the way to revive the old idea of making attributes depend\non file type. I am not following the old path\n\npattern is_symlink otherattr\n\nbecause that would mean something which looks like an attribute\n(is_symlink) but is not. Instead,\n\nsymlink:pattern attrs\n\nis to specify attrs for pattern if it is a symlink. So, e.g.,\n\nsymlink:* diff=symlink\n\ntogether with a nice diff.symlink.textconv displays symlinks nicely.\nMost notable rough edges:\n\n- git_checkattr() gets a new mode parameter. I've stuck it in where I\n could, 0 anywhere else. This is appropriate in many places (where we\nknow IS_REG a forteriori) but I'm not sure about all of them.\n\n- We/someone could teach git_checkattr other prefixes.\n\n- It is tested with one (1) symlink and works!\n\n- I would bundle it with Jeff's doc and test ;)\n\n- This is not a real commit message.\n\nSigned-off-by: Michael J Gruber \n---\n archive.c | 2 +-\n attr.c | 5 ++++-\n attr.h | 2 +-\n builtin/check-attr.c | 7 ++++++-\n builtin/pack-objects.c | 2 +-\n convert.c | 4 ++--\n diff.c | 5 ++---\n grep.c | 2 +-\n ll-merge.c | 4 ++--\n userdiff.c | 4 ++--\n userdiff.h | 2 +-\n ws.c | 2 +-\n 12 files changed, 24 insertions(+), 17 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 1944ed4..2870f04 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -124,7 +124,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n \tpath_without_prefix = path.buf + args->baselen;\n \n \tsetup_archive_check(check);\n-\tif (!git_checkattr(path_without_prefix, ARRAY_SIZE(check), check)) {\n+\tif (!git_checkattr(path_without_prefix, ARRAY_SIZE(check), check, mode)) {\n \t\tif (ATTR_TRUE(check[0].value))\n \t\t\treturn 0;\n \t\tconvert = ATTR_TRUE(check[1].value);\ndiff --git a/attr.c b/attr.c\nindex 6aff695..c4481ac 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -708,7 +708,7 @@ static int macroexpand_one(int attr_nr, int rem)\n \treturn rem;\n }\n \n-int git_checkattr(const char *path, int num, struct git_attr_check *check)\n+int git_checkattr(const char *path, int num, struct git_attr_check *check, unsigned short mode)\n {\n \tstruct attr_stack *stk;\n \tconst char *cp;\n@@ -718,6 +718,9 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)\n \tfor (i = 0; i < attr_nr; i++)\n \t\tcheck_all_attr[i].value = ATTR__UNKNOWN;\n \n+\tif (S_ISLNK(mode))\n+\t\tpath = prefix_filename(\"symlink:\", strlen(\"symlink:\"), path);\n+\t/* else if (IS_WHATEVER(mode)) do_what_you_feel_like */\n \tpathlen = strlen(path);\n \tcp = strrchr(path, '/');\n \tif (!cp)\ndiff --git a/attr.h b/attr.h\nindex 8b3f19b..1af0bd4 100644\n--- a/attr.h\n+++ b/attr.h\n@@ -29,7 +29,7 @@ struct git_attr_check {\n \tconst char *value;\n };\n \n-int git_checkattr(const char *path, int, struct git_attr_check *);\n+int git_checkattr(const char *path, int, struct git_attr_check *, unsigned short mode);\n \n enum git_attr_direction {\n \tGIT_ATTR_CHECKIN,\ndiff --git a/builtin/check-attr.c b/builtin/check-attr.c\nindex 3016d29..30be2c2 100644\n--- a/builtin/check-attr.c\n+++ b/builtin/check-attr.c\n@@ -24,7 +24,12 @@ static void check_attr(int cnt, struct git_attr_check *check,\n \tconst char** name, const char *file)\n {\n \tint j;\n-\tif (git_checkattr(file, cnt, check))\n+\tunsigned int mode = 0;\n+\tstruct stat st;\n+\n+\tif (!lstat(file, &st))\n+\t\tmode = st.st_mode;\n+\tif (git_checkattr(file, cnt, check, mode))\n \t\tdie(\"git_checkattr died\");\n \tfor (j = 0; j < cnt; j++) {\n \t\tconst char *value = check[j].value;\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex b0503b2..4e9ff68 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -633,7 +633,7 @@ static int no_try_delta(const char *path)\n \tstruct git_attr_check check[1];\n \n \tsetup_delta_attr_check(check);\n-\tif (git_checkattr(path, ARRAY_SIZE(check), check))\n+\tif (git_checkattr(path, ARRAY_SIZE(check), check, 0))\n \t\treturn 0;\n \tif (ATTR_FALSE(check->value))\n \t\treturn 1;\ndiff --git a/convert.c b/convert.c\nindex d5aebed..d5c2b9c 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -736,7 +736,7 @@ int convert_to_git(const char *path, const char *src, size_t len,\n \tconst char *filter = NULL;\n \n \tsetup_convert_check(check);\n-\tif (!git_checkattr(path, ARRAY_SIZE(check), check)) {\n+\tif (!git_checkattr(path, ARRAY_SIZE(check), check, 0)) {\n \t\tstruct convert_driver *drv;\n \t\taction = git_path_check_crlf(path, check + 4);\n \t\tif (action == CRLF_GUESS)\n@@ -773,7 +773,7 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \tconst char *filter = NULL;\n \n \tsetup_convert_check(check);\n-\tif (!git_checkattr(path, ARRAY_SIZE(check), check)) {\n+\tif (!git_checkattr(path, ARRAY_SIZE(check), check, 0)) {\n \t\tstruct convert_driver *drv;\n \t\taction = git_path_check_crlf(path, check + 4);\n \t\tif (action == CRLF_GUESS)\ndiff --git a/diff.c b/diff.c\nindex 6640857..22b5d94 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1784,8 +1784,7 @@ static void diff_filespec_load_driver(struct diff_filespec *one)\n \tif (one->driver)\n \t\treturn;\n \n-\tif (S_ISREG(one->mode))\n-\t\tone->driver = userdiff_find_by_path(one->path);\n+\tone->driver = userdiff_find_by_path(one->path, one->mode);\n \n \t/* Fallback to default settings */\n \tif (!one->driver)\n@@ -2683,7 +2682,7 @@ static void run_diff_cmd(const char *pgm,\n \tif (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))\n \t\tpgm = NULL;\n \telse {\n-\t\tstruct userdiff_driver *drv = userdiff_find_by_path(attr_path);\n+\t\tstruct userdiff_driver *drv = userdiff_find_by_path(attr_path, two->mode);\n \t\tif (drv && drv->external)\n \t\t\tpgm = drv->external;\n \t}\ndiff --git a/grep.c b/grep.c\nindex 63c4280..05a77fc 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -877,7 +877,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,\n \tmemset(&xecfg, 0, sizeof(xecfg));\n \tif (opt->funcname && !opt->unmatch_name_only && !opt->status_only &&\n \t !opt->name_only && !binary_match_only && !collect_hits) {\n-\t\tstruct userdiff_driver *drv = userdiff_find_by_path(name);\n+\t\tstruct userdiff_driver *drv = userdiff_find_by_path(name, 0);\n \t\tif (drv && drv->funcname.pattern) {\n \t\t\tconst struct userdiff_funcname *pe = &drv->funcname;\n \t\t\txdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 6ce512e..3f351e3 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -330,7 +330,7 @@ static int git_path_check_merge(const char *path, struct git_attr_check check[2]\n \t\tcheck[0].attr = git_attr(\"merge\");\n \t\tcheck[1].attr = git_attr(\"conflict-marker-size\");\n \t}\n-\treturn git_checkattr(path, 2, check);\n+\treturn git_checkattr(path, 2, check, 0);\n }\n \n static void normalize_file(mmfile_t *mm, const char *path)\n@@ -387,7 +387,7 @@ int ll_merge_marker_size(const char *path)\n \n \tif (!check.attr)\n \t\tcheck.attr = git_attr(\"conflict-marker-size\");\n-\tif (!git_checkattr(path, 1, &check) && check.value) {\n+\tif (!git_checkattr(path, 1, &check, 0) && check.value) {\n \t\tmarker_size = atoi(check.value);\n \t\tif (marker_size <= 0)\n \t\t\tmarker_size = DEFAULT_CONFLICT_MARKER_SIZE;\ndiff --git a/userdiff.c b/userdiff.c\nindex 1ff4797..e5d0adf 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -245,7 +245,7 @@ struct userdiff_driver *userdiff_find_by_name(const char *name) {\n \treturn userdiff_find_by_namelen(name, len);\n }\n \n-struct userdiff_driver *userdiff_find_by_path(const char *path)\n+struct userdiff_driver *userdiff_find_by_path(const char *path, unsigned short mode)\n {\n \tstatic struct git_attr *attr;\n \tstruct git_attr_check check;\n@@ -256,7 +256,7 @@ struct userdiff_driver *userdiff_find_by_path(const char *path)\n \n \tif (!path)\n \t\treturn NULL;\n-\tif (git_checkattr(path, 1, &check))\n+\tif (git_checkattr(path, 1, &check, mode))\n \t\treturn NULL;\n \n \tif (ATTR_TRUE(check.value))\ndiff --git a/userdiff.h b/userdiff.h\nindex 942d594..e516f1d 100644\n--- a/userdiff.h\n+++ b/userdiff.h\n@@ -21,6 +21,6 @@ struct userdiff_driver {\n \n int userdiff_config(const char *k, const char *v);\n struct userdiff_driver *userdiff_find_by_name(const char *name);\n-struct userdiff_driver *userdiff_find_by_path(const char *path);\n+struct userdiff_driver *userdiff_find_by_path(const char *path, unsigned short mode);\n \n #endif /* USERDIFF */\ndiff --git a/ws.c b/ws.c\nindex 9fb9b14..51929d3 100644\n--- a/ws.c\n+++ b/ws.c\n@@ -88,7 +88,7 @@ unsigned whitespace_rule(const char *pathname)\n \tstruct git_attr_check attr_whitespace_rule;\n \n \tsetup_whitespace_attr_check(&attr_whitespace_rule);\n-\tif (!git_checkattr(pathname, 1, &attr_whitespace_rule)) {\n+\tif (!git_checkattr(pathname, 1, &attr_whitespace_rule, 0)) {\n \t\tconst char *value;\n \n \t\tvalue = attr_whitespace_rule.value;\n-- \n1.7.4.1.257.gb09fa\n"},{"id":"162446","messageId":"4D6BC8DE.1090402@drmicha.warpmail.net","threadId":"26609","inReplyTo":"AANLkTim-vyWhgmFuewbd5e5iJiTC-EHz7g=efr2Sz-RH@mail.gmail.com","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-28T16:10:06Z","receivedAt":"2011-02-28T16:10:06Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Mart Sõmermaa venit, vidit, dixit 28.02.2011 11:45:\n> Why not just make it reversible then?\n> \n> $ git diff -M\n> diff --git a/foo b/bar\n> similarity index 100%\n> rename from foo\n> rename to bar\n> \n> is nonreversible without git already (i.e. does not work with plain\n> patch AFAIK).\n\nThat \"i.e.\" is a misunderstanding:\n\nI'm sure Junio meant that \"git apply -R\" should make sense on a path\nlike that.\n\nYou can easily undo (reverse) a rename without knowing the file\ncontents; undeleting a file is more difficult :)\n\n> Adding\n> \n> $ git diff -D\n> diff --git a/foo b/foo\n> deleted file mode 100644\n> delete foo\n> \n> would be neither less nor more reversible -- it would also only work\n> with git apply\n> (assuming that apply is amended accordingly).\n\nTo reverse it, we would need the sah1 lines, and the repo would need\nthat blob.\n\n> Cognitive burdens aside, the \"delete foo\" output is both\n> more explicit and shorter anyway. Occam's razor FTW :)!\n\nYes.\n\n> Junio, I won't push this further, so it's a final call -- if you give -1,\n> then let -D fall to the colourless abyss of oblivion :), if +1 or +0,\n> I'll see if I can come up with a patch.\n\nYou can always submit a patch, and you can never know what happens ;)\n\n> Best regards,\n> MS\n\nUhm, and please don't do that top post thing and the cc culling.\nReadding J&J.\n\n[quote snipped]\n"},{"id":"162454","messageId":"20110228173037.GC24915@sigill.intra.peff.net","threadId":"26609","inReplyTo":"f35b1b4b57a563f57f169602440b3b0abdef5a07.1298906628.git.git@drmicha.warpmail.net","subject":"Re: [PATCH/WIP] attr: make attributes depend on file type","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-28T17:30:37Z","receivedAt":"2011-02-28T17:30:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 28, 2011 at 04:26:18PM +0100, Michael J Gruber wrote:\n\n> This is a whether balloon patch (to check whether this balloons or not,\n> uhm). Hit it hard, not me ;)\n\nOuch.\n\n> It's part of the way to revive the old idea of making attributes depend\n> on file type. I am not following the old path\n> \n> pattern is_symlink otherattr\n> \n> because that would mean something which looks like an attribute\n> (is_symlink) but is not. Instead,\n> \n> symlink:pattern attrs\n> \n> is to specify attrs for pattern if it is a symlink. So, e.g.,\n\nThis is way better than what I proposed. From the user's perspective, it\nis visually clearer that the symlink bit is part of the selector, and\nnot an attribute. And it syntactically disallows nonsense like:\n\n pattern is_symlink is_gitlink otherattr\n\nThe only downside is that it is technically a regression if somebody was\nusing gitattributes for the bizarrely named file \"symlink:\". It seems\npretty unlikely, but possibly we should be carving out a syntactic\nnamespace like:\n\n ^[a-z]+:\n\nor even:\n\n ^[a-z]+(=[^:]*)?:\n\nAnd that would later allow stuff like \"submodule:\" if people wanted to\nattach specific bits to submodules (though perhaps it is not necessary,\nbecause we have the entire separate .gitmodules file). I don't know if\nthere are other non-name elements people would be interested in\nselecting on. So maybe that is over-engineering.\n\nWe also need some way of quoting. Duy has a 1.8.0 proposal to handle\nthis. If we're going to make a syntactic change to gitattributes for\nthis, it should probably be related (since the quoting mechanism is the\nway you would fix it if you _are_ affected), and should probably follow\nthe same migration mechanism (it looks like there is talk of a \"#\nfeature: foo bar\" line).\n\nSee:\n\n http://article.gmane.org/gmane.comp.version-control.git/165970\n\nI didn't test, but do we now assume that a pattern like \"foo.*\ndiff=bar\" will only match when foo is a regular file? I think that would\nfix stuff like \"*.pdf merge=pdf\" when there is a symlink named\n\"file.pdf\", which I'm pretty sure is currently broken (but did not\ntest).\n\nOTOH, that is yet another behavior change if somebody had something like\n\"foo diff=symlink\", which we would be breaking. So perhaps we will need\nmany \"# feature\" markers to make this right. :)\n\n-Peff\n"},{"id":"162455","messageId":"7v39n8jbj2.fsf@alter.siamese.dyndns.org","threadId":"26609","inReplyTo":"20110228173037.GC24915@sigill.intra.peff.net","subject":"Re: [PATCH/WIP] attr: make attributes depend on file type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-28T17:48:17Z","receivedAt":"2011-02-28T17:48:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> The only downside is that it is technically a regression if somebody was\n> using gitattributes for the bizarrely named file \"symlink:\". It seems\n> pretty unlikely, but possibly we should be carving out a syntactic\n> namespace like:\n>\n> ^[a-z]+:\n>\n> or even:\n>\n> ^[a-z]+(=[^:]*)?:\n\nOr even '^:[a-z]+(=[^,=])?(,[a-z]+(=[^,=])?)*:' to\n\n (1) always have some special character at the beginning, to limit the\n extent of the damage to existing funny pathnames (i.e. to collide\n your pathname must begin with such a special character). This also\n has a nice side effect of making it clear that something special is\n going on; and\n\n (2) allow more than one such special on the line, comma-separated.\n\nBut that is just a small bikeshed. Other than that, I like what Michael\nand you are aiming for (I am only commenting on the general direction at\nthis moment, as I haven't looked at the patch at all yet).\n"},{"id":"162457","messageId":"7vy650hvwa.fsf@alter.siamese.dyndns.org","threadId":"26609","inReplyTo":"20110228122335.GB5197@sigill.intra.peff.net","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-28T18:11:17Z","receivedAt":"2011-02-28T18:11:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Actually, thinking on this a bit more, I guess \"-M\" and \"-C\" are usable\n> without the sha1. In fact, we don't even provide it for a strict 100%\n> rename, and for a rename-with-patch, you can apply the patch, assuming\n> you have the original file in any form.\n\nYes, you got it correctly. A patch is about giving you an ability to\npropagate a change to a similar tree, that does not have to be identical\nto yours (otherwise you can just send tarball of the whole thing ;-)).\n\nAnd a patch is not purely technical; it has human component in that the\nrecipient needs to know enough about the context of the patch to be able\nto judge if the change is applicable to his tree. By seeing some context\nlines in -M/-C (many of them are not pure renames), the recipient can be\nsure that the patch is being applied to his tree that is \"similar enough\"\nto what the patch was originally produced against. It would certainly\nhelp to have git-apply that understands the rename patches to apply such a\npatch mechanically, but that is not a strict requirement; the recipient\ncan move or copy the original file to manually create the target and apply\nthe patch by hand to produce the desired result, so \"do you have git?\" is\nmostly irrelevant.\n\nThe proposed -D would apply to any tree that happens to have the path\nbeing mentioned in the patch regardless of how similar the target tree is\nto the original. Pure renaming -M/-C patch shares the same risk, but\nunlike these modes, -D goes one step too far by making it impossible to\nrecover from lossage without having a backup.\n\nBut all of the above is only about principles. As I said, I am not\nstrongly opposed to have an output mode that is primarily for reviewing,\njust like we have --color-words, that are not suitable for patch\napplication, as long as users understand the implications.\n\nIt may make sense to show such a patch still with a bit of context,\nperhaps like this:\n\n README | 54 ------------------------------------------------------\n 1 files changed, 0 insertions(+), 54 deletions(-)\n\n diff --git -D a/README b/README\n deleted file mode 100644\n index 67cfeb2..0000000\n --- a/README\n +++ /dev/null\n @@ -1,54 +0,0 @@\n -////////////////////////////////////////////////////////////////\n -\n -\tGIT - the stupid content tracker\n -...\n -the discussion following them on the mailing list give a good\n -reference for project status, development direction and\n -remaining tasks.\n\nso that the reader can have some warm and fuzzy feeling that the correct\nfile is being deleted, though.\n"},{"id":"162482","messageId":"20110228215454.GB5854@sigill.intra.peff.net","threadId":"26609","inReplyTo":"4D6B9DA1.4070805@drmicha.warpmail.net","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-28T21:54:54Z","receivedAt":"2011-02-28T21:54:54Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 28, 2011 at 02:05:37PM +0100, Michael J Gruber wrote:\n\n> When I said \"copy patch\" I actually meant a patch which records the copy\n> \"Makefile -> Dofile\". What is it today? Is it me? I know I wrote the\n> \"mv\" example first, but still :)\n\nYeah, I looked at the \"mv\" example and totally glossed over the word\n\"copy\" in your text.\n\n> I mean, Alice:\n> cp Makefile Dofile\n> sends me a -C patch\n> \n> I:\n> Break everything by hacking Makefile\n> send her a crappy patch\n> \n> Both:\n> apply the received patch\n> \n> Now I end up with a borked Makefile and a borked Dofile, but Alice still\n> has a good Dofile, and it's all my fault, so I don't deserve any better.\n> But still.\n\nOK, this is a much better example. Yes, you have different state at the\nend, and there were no conflicts. I don't think we can resolve the\nsituation automagically, but I think it would at least be nice to\nmention the conflict.\n\nI guess the big question is how to mention it. Should it cause the patch\napplication to fail? I'm worried about that creating unnecessary false\npositives for cases that are really quite harmless. Should patch\napplication just give a warning unless --strict-renames or something is\nused? I dunno. This is one of those corner cases where we can see that\nthere is a potential problem, but it hasn't actually come up in\npractice, so it's difficult to see what would be most useful in the real\nworld. Maybe that means we are wasting our time thinking about it. :)\n\n> This is orthogonal to the \"-D\" suggestion\", but \"-D\" could write the\n> index line to start with.\n\nYeah. I had just assumed that \"-D\" would be the same as the current\ntext, minus the actual patch lines. IOW:\n\n diff --git a/Makefile b/Makefile\n deleted file mode 100644\n index c9ff69c..0000000\n\n-Peff\n"},{"id":"162484","messageId":"20110228222352.GC5854@sigill.intra.peff.net","threadId":"26609","inReplyTo":"7vy650hvwa.fsf@alter.siamese.dyndns.org","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-28T22:23:52Z","receivedAt":"2011-02-28T22:23:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 28, 2011 at 10:11:17AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Actually, thinking on this a bit more, I guess \"-M\" and \"-C\" are usable\n> > without the sha1. In fact, we don't even provide it for a strict 100%\n> > rename, and for a rename-with-patch, you can apply the patch, assuming\n> > you have the original file in any form.\n> \n> Yes, you got it correctly. A patch is about giving you an ability to\n> propagate a change to a similar tree, that does not have to be identical\n> to yours (otherwise you can just send tarball of the whole thing ;-)).\n\nYeah, I guess I am just curious whether \"similar enough\" is really\nrelevant with file deletion. Obviously if you have the sha1 then\neverything applies, no problem. But if you don't, how useful is the\npatch text? The patch application will error out with a conflict. You\nknow in either case that the person wanted the file deleted. The patch\ntext shows you what _their_ version of the file looked like. But that's\nnot likely to be useful; what you really care about is what's in _your_\nversion of the file, and whether that should be deleted or not.\n\nIf you did really care about what was in their file, the giant deletion\ndiff is almost certainly not interesting. What you really care about is\nwhat's different in their version versus yours. Which we don't provide a\nsimple way to access. But more likely, you're just going to email them\nback and say \"what is this based on?\".\n\nSo yeah, the patch gives more information, but I am skeptical that\nanybody uses it in practice.\n\n> And a patch is not purely technical; it has human component in that the\n> recipient needs to know enough about the context of the patch to be able\n> to judge if the change is applicable to his tree. By seeing some context\n> lines in -M/-C (many of them are not pure renames), the recipient can be\n> sure that the patch is being applied to his tree that is \"similar enough\"\n> to what the patch was originally produced against.\n\nBut we don't provide context lines for pure renames. Yet the sender\nmight have a different version of the file than the recipient, and the\nrecipient has no idea. We don't even detect that situation, let alone\ngive the user any clue about what might be different.\n\n> It would certainly help to have git-apply that understands the rename\n> patches to apply such a patch mechanically, but that is not a strict\n> requirement; the recipient can move or copy the original file to\n> manually create the target and apply the patch by hand to produce the\n> desired result, so \"do you have git?\" is mostly irrelevant.\n\nIsn't that the same for a delete? I can see you wanted the file deleted.\nIt didn't match up with my sha1, but I can go ahead and do the delete\nmanually if I want.\n\n> The proposed -D would apply to any tree that happens to have the path\n> being mentioned in the patch regardless of how similar the target tree is\n> to the original. Pure renaming -M/-C patch shares the same risk, but\n> unlike these modes, -D goes one step too far by making it impossible to\n> recover from lossage without having a backup.\n\nI'm not quite sure what lossage you mean. On the recipient's end? They\ncan just \"git revert\", no? Or do you mean somebody who gets a conflict\nand manually does the deletion anyway? If they have the patch text, then\nyes, they have something, but it is _not_ what they deleted. Otherwise\nthey would not have had a conflict.\n\n> But all of the above is only about principles. As I said, I am not\n> strongly opposed to have an output mode that is primarily for reviewing,\n> just like we have --color-words, that are not suitable for patch\n> application, as long as users understand the implications.\n\nI am not sure I agree about the danger, but certainly I agree that for\nviewing it is not a problem at all. I have to admit to not caring all\n_that_ much about the topic. I do find deletion diffs unnecessarily\nspammy, but they just don't happen enough for me to be really annoyed.\n\n> It may make sense to show such a patch still with a bit of context,\n> perhaps like this:\n> \n> README | 54 ------------------------------------------------------\n> 1 files changed, 0 insertions(+), 54 deletions(-)\n> \n> diff --git -D a/README b/README\n> deleted file mode 100644\n> index 67cfeb2..0000000\n> --- a/README\n> +++ /dev/null\n> @@ -1,54 +0,0 @@\n> -////////////////////////////////////////////////////////////////\n> -\n> -\tGIT - the stupid content tracker\n> -...\n> -the discussion following them on the mailing list give a good\n> -reference for project status, development direction and\n> -remaining tasks.\n> \n> so that the reader can have some warm and fuzzy feeling that the correct\n> file is being deleted, though.\n\nHmm. I think that is even worse. It's _still_ spammy, and it's broken as\na diff (the sha1 in the index line doesn't match the preimage that the\ndiff text shows). Yeah, it gives you warm fuzzy feelings that the file\nthey were thinking about deleting is at least vaguely related to what\nyou wanted to delete. But how often do you get a deletion patch where\nseeing the first 10 lines is actually useful? That is, what is the case\nwhere you see the first 10 lines and immediately say \"Wait, this\ndeletion is totally bogus!\" and _didn't_ just see that there was a\nconflicting deletion, read the commit message or email and say \"Wait,\nwhy are we deleting this file?\"\n\n-Peff\n"},{"id":"162492","messageId":"7vfwr7hh7f.fsf@alter.siamese.dyndns.org","threadId":"26609","inReplyTo":"20110228222352.GC5854@sigill.intra.peff.net","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-28T23:28:36Z","receivedAt":"2011-02-28T23:28:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Yeah, I guess I am just curious whether \"similar enough\" is really\n> relevant with file deletion. Obviously if you have the sha1 then\n> everything applies, no problem. But if you don't, how useful is the\n> patch text? The patch application will error out with a conflict. You\n> know in either case that the person wanted the file deleted. The patch\n> text shows you what _their_ version of the file looked like. But that's\n> not likely to be useful; what you really care about is what's in _your_\n> version of the file, and whether that should be deleted or not.\n\nExactly; in order to intelligently decide if that should be deleted or\nnot, you need to be able to judge if the deletion makes sense in the\ncontext of _your_ tree. Hopefully proposed commit log message alone may\nclarify if the change applies to your situation, or you need to be able to\nsee if their version and yours are still serving similar purposes in the\nsystem (e.g. you don't want to lose the definition of a function they\ndidn't have when they decided to remove the file saying \"nobody calls any\nfunction in this library anymore\").\n\n> If you did really care about what was in their file, the giant deletion\n> diff is almost certainly not interesting. What you really care about is\n> what's different in their version versus yours.\n\nCorrect; the deletion diff has half of the necessary information (the\nother half being what you have).\n\n> I'm not quite sure what lossage you mean. On the recipient's end? They\n> can just \"git revert\", no?\n\nNo, I was not particularly interested in limiting the discussion to\ngit-using population. As I said, just like rename patch can be manually\napplied by first moving and then eyeballing, you can see \"an instruction\nto delete--ok, 'rm'\" to apply it and the file is gone (unlike the case\nwhere you manually did \"mv\"). That is what I meant by \"additionally -D\nhas lossage\".\n\n>> It may make sense to show such a patch still with a bit of context,\n>> perhaps like this:\n>> \n>> README | 54 ------------------------------------------------------\n>> 1 files changed, 0 insertions(+), 54 deletions(-)\n>> \n>> diff --git -D a/README b/README\n>> deleted file mode 100644\n>> index 67cfeb2..0000000\n>> --- a/README\n>> +++ /dev/null\n>> @@ -1,54 +0,0 @@\n>> -////////////////////////////////////////////////////////////////\n>> -\n>> -\tGIT - the stupid content tracker\n>> -...\n>> -the discussion following them on the mailing list give a good\n>> -reference for project status, development direction and\n>> -remaining tasks.\n>> \n>> so that the reader can have some warm and fuzzy feeling that the correct\n>> file is being deleted, though.\n>\n> Hmm. I think that is even worse. It's _still_ spammy, and it's broken as\n> a diff (the sha1 in the index line doesn't match the preimage that the\n\nThat is very much intentional; I don't think we want the format to apply\nmechanically. I was not talking about patch application safety at all in\nthis thread; I already dismissed \"-D\" as too unreliable as a patch format\nto be used for conveying a change for mechanical consumption, didn't I?\nWhen you want to do so, you just generate the output without -D.\n\nThe suggestion to give some context is only for people who want to eyeball\ntheir own patches, helping them to make sure that they are removing what\nthey wanted to remove. If they can tell from the filename alone, that is\njust as fine. I am not interested that deeply here; the only thing I care\nabout is to make sure we don't have to keep carrying crappy options as if\nthey are sane.\n"},{"id":"162496","messageId":"7vy64zg0ms.fsf@alter.siamese.dyndns.org","threadId":"26609","inReplyTo":"7vfwr7hh7f.fsf@alter.siamese.dyndns.org","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-01T00:11:55Z","receivedAt":"2011-03-01T00:11:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> I'm not quite sure what lossage you mean. On the recipient's end? They\n>> can just \"git revert\", no?\n\nThis is beating on a dead horse, but in my previous life, in a project I\nwasn't deeply involved in, a deployment procedure used was to:\n\n - give git-archive tarball of a major release to the target machine that\n does not have git repository, and extract the tarball;\n\n - maintain 'maint' branch, and show the tip of 'maint' on a test server,\n but give the client vetoing power on individual changes;\n\n - hence ending up preparing a format-patch output for selected changes,\n and running git-apply on the target machine to update it.\n\nThe -M/-C options would have worked well in this scenario, including the\nrecovery from \"oops--that didn't work, please revert asap\" (I don't know\nif they actually used -M/-C when preparing the incremental updates,\nthough). The -D option does not have the same reversibility.\n\nI am (have been) only reacting to the earlier statement by Michael that -D\nis the same as -M/-C and reversibility does not matter. It does in such a\nsituation.\n\nNow that the project with that deployment procedure is totally behind me,\nI probably shouldn't care too much about such a workflow, but I suspect\nthere still are people using that sort of workflow, and this would matter\nto them.\n\nIn any case, a minimum patch to give what Mart wanted to see would\nprobably look like this. I'll leave bugfixes, documentation and tests to\nthe readers ;-).\n\n diff.c | 12 +++++++++---\n diff.h | 1 +\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 5422c43..5c66a53 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1943,7 +1943,11 @@ static void builtin_diff(const char *name_a,\n \t\t}\n \t}\n \n-\tif (!DIFF_OPT_TST(o, TEXT) &&\n+\tif (o->irreversible_delete && lbl[1][0] == '/') {\n+\t\tfprintf(o->file, \"%s\", header.buf);\n+\t\tstrbuf_reset(&header);\n+\t\tgoto free_ab_and_return;\n+\t} else if (!DIFF_OPT_TST(o, TEXT) &&\n \t ( (!textconv_one && diff_filespec_is_binary(one)) ||\n \t (!textconv_two && diff_filespec_is_binary(two)) )) {\n \t\tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n@@ -1963,8 +1967,7 @@ static void builtin_diff(const char *name_a,\n \t\t\tfprintf(o->file, \"%sBinary files %s and %s differ\\n\",\n \t\t\t\tline_prefix, lbl[0], lbl[1]);\n \t\to->found_changes = 1;\n-\t}\n-\telse {\n+\t} else {\n \t\t/* Crazy xdl interfaces.. */\n \t\tconst char *diffopts = getenv(\"GIT_DIFF_OPTS\");\n \t\txpparam_t xpp;\n@@ -3160,6 +3163,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\t\treturn error(\"invalid argument to -M: %s\", arg+2);\n \t\toptions->detect_rename = DIFF_DETECT_RENAME;\n \t}\n+\telse if (!strcmp(arg, \"-D\") || !strcmp(arg, \"--irreversible-delete\")) {\n+\t\toptions->irreversible_delete = 1;\n+\t}\n \telse if (!prefixcmp(arg, \"-C\") || !prefixcmp(arg, \"--find-copies=\") ||\n \t\t !strcmp(arg, \"--find-copies\")) {\n \t\tif (options->detect_rename == DIFF_DETECT_COPY)\ndiff --git a/diff.h b/diff.h\nindex 310bd6b..11d13cf 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -104,6 +104,7 @@ struct diff_options {\n \tint interhunkcontext;\n \tint break_opt;\n \tint detect_rename;\n+\tint irreversible_delete;\n \tint skip_stat_unmatch;\n \tint line_termination;\n \tint output_format;\n"},{"id":"162515","messageId":"4D6CA45F.5000001@drmicha.warpmail.net","threadId":"26609","inReplyTo":"7v39n8jbj2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/WIP] attr: make attributes depend on file type","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-01T07:46:39Z","receivedAt":"2011-03-01T07:46:39Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 28.02.2011 18:48:\n> Jeff King writes:\n> \n>> The only downside is that it is technically a regression if somebody was\n>> using gitattributes for the bizarrely named file \"symlink:\". It seems\n>> pretty unlikely, but possibly we should be carving out a syntactic\n>> namespace like:\n>>\n>> ^[a-z]+:\n>>\n>> or even:\n>>\n>> ^[a-z]+(=[^:]*)?:\n> \n> Or even '^:[a-z]+(=[^,=])?(,[a-z]+(=[^,=])?)*:' to\n> \n> (1) always have some special character at the beginning, to limit the\n> extent of the damage to existing funny pathnames (i.e. to collide\n> your pathname must begin with such a special character). This also\n> has a nice side effect of making it clear that something special is\n> going on; and\n> \n> (2) allow more than one such special on the line, comma-separated.\n> \n> But that is just a small bikeshed. Other than that, I like what Michael\n> and you are aiming for (I am only commenting on the general direction at\n> this moment, as I haven't looked at the patch at all yet).\n\nThanks, Junio, and Jeff, for the helpful comments. I isolated the naming\nconvention (now \"symlink:\") to a single spot in attr.c so that changing\nit (then \":symlink:\") is always simple while this develops. (I was\nafraid that the low likelikehood that someone's file names would collide\ncould be too much already.)\n\nHaving a standard pattern applied to regular files only is an intended\nside-effect; more precisely: to non-symlinks right now. We could tighten\nthat to IS_REG, of course, and easily add others (or :other:, :nonreg:).\n\nWhat I really would need help with is checking all call sites of\ngit_checkattr(), which gained a new mode parameter: I used the proper\nmode where available (from diff_filespec, e.g., i.e.: taking mode from\nindex or tree) or by looking it up in the fs (using lstat() in\nbuiltin/check-attr.c), and I used mode=0 in other cases, assuming that\nthis conveys IS_REG and that we're sure those call sites are about\nregular files or blobs. I tried to avoid unmotivated lstat().\n\nMichael\n"},{"id":"162936","messageId":"AANLkTinBTWPrAOq9BOkBJc5GOhFkzsr__9wqEWnWdwYR@mail.gmail.com","threadId":"26609","inReplyTo":"7vy64zg0ms.fsf@alter.siamese.dyndns.org","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Mart Sõmermaa","fromEmail":"mrts.pydev@gmail.com","sentAt":"2011-03-07T20:38:03Z","receivedAt":"2011-03-07T20:38:03Z","isPatch":false,"sender":{"key":"mrts.pydev@gmail.com","avatar":"https://gravatar.com/avatar/52597879a4c009c603b84516a2169a125b875c692534bbfc1968cb42bfe28f16?d=mp&s=160"},"body":"On Tue, Mar 1, 2011 at 2:11 AM, Junio C Hamano wrote:\n> In any case, a minimum patch to give what Mart wanted to see would\n> probably look like this.  I'll leave bugfixes, documentation and tests to\n> the readers ;-).\n\nThe minimum looks to be the optimum -- IMHO this is entirely sufficient,\nthank you very much!\n\nWhat would the bugfixes be (i.e. do you have any doubts about the\nimplementation)?\nThe change looks quite straightforward, to the point and bug-free to my eyes.\n\nI try to get to writing tests ASAP.\n\nAs for the documentation, would the following section be sufficient in\nDocumentation/diff-options.txt?\n\n-D::\n--irreversible-delete::\n\tOmit file contents when file has been deleted and only\n\toutput the header. This is useful during diff review\n\tbut should not be used in actual patches as these would\n\tbe non-reversible due to the omitted file contents.\n\nBest regards and warm thanks once more,\nMS\n"},{"id":"162962","messageId":"4D75D768.1080701@drmicha.warpmail.net","threadId":"26609","inReplyTo":"AANLkTinBTWPrAOq9BOkBJc5GOhFkzsr__9wqEWnWdwYR@mail.gmail.com","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-08T07:14:48Z","receivedAt":"2011-03-08T07:14:48Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Mart Sõmermaa venit, vidit, dixit 07.03.2011 21:38:\n> On Tue, Mar 1, 2011 at 2:11 AM, Junio C Hamano wrote:\n>> In any case, a minimum patch to give what Mart wanted to see would\n>> probably look like this. I'll leave bugfixes, documentation and tests to\n>> the readers ;-).\n> \n> The minimum looks to be the optimum -- IMHO this is entirely sufficient,\n> thank you very much!\n> \n> What would the bugfixes be (i.e. do you have any doubts about the\n> implementation)?\n> The change looks quite straightforward, to the point and bug-free to my eyes.\n> \n> I try to get to writing tests ASAP.\n> \n> As for the documentation, would the following section be sufficient in\n> Documentation/diff-options.txt?\n> \n> -D::\n> --irreversible-delete::\n> \tOmit file contents when file has been deleted and only\n\n\"if the file has\"\n\n> \toutput the header. This is useful during diff review\n> \tbut should not be used in actual patches as these would\n> \tbe non-reversible due to the omitted file contents.\n\nWith the current state of git-apply, they could not even be applied, so\nmaybe:\n\n\tbut produces patches which can not be applied by linkgit:git-apply[1].\n\nYou can revert to the previous description once you've taught git-apply\nthe new diff header :)\n\nMichael\n"},{"id":"163008","messageId":"7vipvte6jc.fsf@alter.siamese.dyndns.org","threadId":"26609","inReplyTo":"AANLkTinBTWPrAOq9BOkBJc5GOhFkzsr__9wqEWnWdwYR@mail.gmail.com","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-08T19:49:59Z","receivedAt":"2011-03-08T19:49:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mart Sõmermaa writes:\n\n> On Tue, Mar 1, 2011 at 2:11 AM, Junio C Hamano wrote:\n>> In any case, a minimum patch to give what Mart wanted to see would\n>> probably look like this.  I'll leave bugfixes, documentation and tests to\n>> the readers ;-).\n>\n> The minimum looks to be the optimum -- IMHO this is entirely sufficient,\n\nI suspect not, as I don't think I ever did anything to the codepath for -B\noutput, and I also recall spending some time thinking about this issue in\nan earlier message that I mentioned -B besides \"deletion\", none of the\nsuggestions in which I don't think I tried to implement.\n\nI also vaguely recall that I suspected that the output may not have the\nusual \"index deadbeaf..000000 mode\" line and that I chose to ignore it.\n\nSo, no, not good enough ;-).\n"},{"id":"163024","messageId":"AANLkTim9juvhBJKraHBkV8AC=zY75xt_1BnyhTDTcoUZ@mail.gmail.com","threadId":"26609","inReplyTo":"7vipvte6jc.fsf@alter.siamese.dyndns.org","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Mart Sõmermaa","fromEmail":"mrts.pydev@gmail.com","sentAt":"2011-03-08T21:25:04Z","receivedAt":"2011-03-08T21:25:04Z","isPatch":false,"sender":{"key":"mrts.pydev@gmail.com","avatar":"https://gravatar.com/avatar/52597879a4c009c603b84516a2169a125b875c692534bbfc1968cb42bfe28f16?d=mp&s=160"},"body":"On Tue, Mar 8, 2011 at 9:49 PM, Junio C Hamano wrote:\n> Mart Sõmermaa writes:\n>\n>> On Tue, Mar 1, 2011 at 2:11 AM, Junio C Hamano wrote:\n>>> In any case, a minimum patch to give what Mart wanted to see would\n>>> probably look like this.  I'll leave bugfixes, documentation and tests to\n>>> the readers ;-).\n>>\n>> The minimum looks to be the optimum -- IMHO this is entirely sufficient,\n>\n> I suspect not, as I don't think I ever did anything to the codepath for -B\n> output, and I also recall spending some time thinking about this issue in\n> an earlier message that I mentioned -B besides \"deletion\", none of the\n> suggestions in which I don't think I tried to implement.\n\nYour suggestion is as follows:\n\n\"It also might make sense to apply the similar principle to shorten the output\nwith -B when a rewrite patch is expressed as a single hunk patch that removes\neverything old and then adds everthing new.\"\n\nI have to admit that I've never used -B, only -M, so please don't mind that\nits semantics and exact behavior are a bit foreign to me.\n\nAfter running t/t4130-apply-criss-cross-rename.sh,\n`git diff -M -B` outputs the following:\n\ndiff --git a/file2 b/file1\nsimilarity index 100%\nrename from file2\nrename to file1\ndiff --git a/file1 b/file2\nsimilarity index 100%\nrename from file1\nrename to file2\n\nCan you bring a similar example of changes in the output after\nthe above-mentioned similar principle has been implemented for -B?\n\n> I also vaguely recall that I suspected that the output may not have the\n> usual \"index deadbeaf..000000 mode\" line and that I chose to ignore it.\n\n'deadbeaf' is present:\n\n$ ../git/bin-wrappers/git diff -D HEAD\ndiff --git a/foo.txt b/foo.txt\ndeleted file mode 100644\nindex 257cc56..0000000\n\nCan we perhaps consider the -B behavior orthogonal to -D (unless I am missing\nsomething very important here)?\n\nBest regards,\nMS\n"},{"id":"163027","messageId":"20110308213133.GA5786@sigill.intra.peff.net","threadId":"26609","inReplyTo":"AANLkTim9juvhBJKraHBkV8AC=zY75xt_1BnyhTDTcoUZ@mail.gmail.com","subject":"Re: git diff: add option for omitting the contents of deletes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-08T21:31:34Z","receivedAt":"2011-03-08T21:31:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 08, 2011 at 11:25:04PM +0200, Mart Sõmermaa wrote:\n\n> \"It also might make sense to apply the similar principle to shorten the output\n> with -B when a rewrite patch is expressed as a single hunk patch that removes\n> everything old and then adds everthing new.\"\n> \n> I have to admit that I've never used -B, only -M, so please don't mind that\n> its semantics and exact behavior are a bit foreign to me.\n> \n> After running t/t4130-apply-criss-cross-rename.sh,\n> `git diff -M -B` outputs the following:\n> \n> diff --git a/file2 b/file1\n> similarity index 100%\n> rename from file2\n> rename to file1\n> diff --git a/file1 b/file2\n> similarity index 100%\n> rename from file1\n> rename to file2\n> \n> Can you bring a similar example of changes in the output after\n> the above-mentioned similar principle has been implemented for -B?\n\nTry:\n\n $ git init\n $ perl -e 'print \"a\\n\" for (1 .. 1000)' >file\n $ git add file && git commit -m one\n $ perl -e 'print \"b\\n\" for (1 .. 1000)' >file\n\nNow you can see that -B breaks notes it as a rewrite:\n\n $ git diff --stat --summary -B\n file | 2000 +++++++++++++++++++++++++++----------------------------\n 1 files changed, 1000 insertions(+), 1000 deletions(-)\n rewrite file (100%)\n\nAnd the diff is long:\n\n $ git diff -B\n diff --git a/file b/file\n dissimilarity index 100%\n index 5cfafaa..a7b871a 100644\n --- a/file\n +++ b/file\n @@ -1,1000 +1,1000 @@\n -a\n -a\n [... x 1000]\n +b\n +b\n [... x 1000]\n\nBut we could perhaps drop the actual 1000-line hunks (or maybe even just\nthe deletion half).\n\n-Peff\n"}]}