{"thread":{"id":"56288","subject":"git format-patch -s enhancement","startedAt":"2021-08-15T23:08:03Z","lastAt":"2021-08-16T21:49:50Z","messageCount":4,"participants":["jim.cromie@gmail.com","Bagas Sanjaya","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"432771","messageId":"CAJfuBxxT_7weC8_O=KYScSbDcSeBdb3v5d_gtn-NXzW_fKLrsA@mail.gmail.com","threadId":"56288","inReplyTo":null,"subject":"git format-patch -s enhancement","fromName":"","fromEmail":"jim.cromie@gmail.com","sentAt":"2021-08-15T23:07:34Z","receivedAt":"2021-08-15T23:08:03Z","isPatch":false,"sender":{"key":"jim.cromie@gmail.com","avatar":null},"body":"hi all\n\ngit format-patch -s is sub-optimal :\nit appends the SoB,\nwhich falls after the snips\n---\nchangelog ...\nthat the commit message may contain\n\n\nSo it misfires on any maintainer scripts\nexpecting the SoB above the 1st snip.\n\nThe workaround is manual SoBs above any snips.\n\nI note this in -s doc,\n\n           Add a Signed-off-by trailer to the commit message, using\nthe committer identity of yourself.\n           See the signoff option in git-commit(1) for more information.\n\n\"trailer\" is really \"document current working behavior\"\n(normative docu-speak, so to speak;)\n\nIdeal behavior is to find 1st in-body  --- snip\nand insert there\n"},{"id":"432783","messageId":"c308b4e5-066e-d0d8-ac14-5769d1a47684@gmail.com","threadId":"56288","inReplyTo":"CAJfuBxxT_7weC8_O=KYScSbDcSeBdb3v5d_gtn-NXzW_fKLrsA@mail.gmail.com","subject":"Re: git format-patch -s enhancement","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-08-16T07:56:55Z","receivedAt":"2021-08-16T07:57:06Z","isPatch":false,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 16/08/21 06.07, jim.cromie@gmail.com wrote:\n> hi all\n> \n> git format-patch -s is sub-optimal :\n> it appends the SoB,\n> which falls after the snips\n> ---\n> changelog ...\n> that the commit message may contain\n> \n> \n> So it misfires on any maintainer scripts\n> expecting the SoB above the 1st snip.\n> \n> The workaround is manual SoBs above any snips.\n> \n> I note this in -s doc,\n> \n>             Add a Signed-off-by trailer to the commit message, using\n> the committer identity of yourself.\n>             See the signoff option in git-commit(1) for more information.\n> \n> \"trailer\" is really \"document current working behavior\"\n> (normative docu-speak, so to speak;)\n> \n> Ideal behavior is to find 1st in-body  --- snip\n> and insert there\n> \n\nIt seems like you don't tell us what snip means. Can you describe your \nenvironment and reproduction steps so that I can reproduce this issue?\n\nAnd next time you can use `git bugreport`.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"432812","messageId":"YRqUK3cRFJmANzDd@coredump.intra.peff.net","threadId":"56288","inReplyTo":"CAJfuBxxT_7weC8_O=KYScSbDcSeBdb3v5d_gtn-NXzW_fKLrsA@mail.gmail.com","subject":"Re: git format-patch -s enhancement","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-16T16:36:59Z","receivedAt":"2021-08-16T16:37:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 15, 2021 at 05:07:34PM -0600, jim.cromie@gmail.com wrote:\n\n> git format-patch -s is sub-optimal :\n> it appends the SoB,\n> which falls after the snips\n> ---\n> changelog ...\n> that the commit message may contain\n> \n> \n> So it misfires on any maintainer scripts\n> expecting the SoB above the 1st snip.\n> \n> The workaround is manual SoBs above any snips.\n> \n> I note this in -s doc,\n> \n>            Add a Signed-off-by trailer to the commit message, using\n> the committer identity of yourself.\n>            See the signoff option in git-commit(1) for more information.\n> \n> \"trailer\" is really \"document current working behavior\"\n> (normative docu-speak, so to speak;)\n> \n> Ideal behavior is to find 1st in-body  --- snip\n> and insert there\n\nThe big disconnect here is that \"---\" snip lines are not meant to be\nmeaningful within commit messages themselves. They are part of the\nprocess of sticking a commit message into an email. So format-patch and\ngit-am know about them, but \"git commit\" for example doesn't.\n\nSo \"git commit --signoff\" probably shouldn't take them into account when\ndeciding the end of a commit message. The user might or might not have\nmeant \"---\" to be syntactically meaningful, depending on whether they\nplan to send the message with format-patch (and changing the behavior\nnow is questionable).\n\nDoing so with \"git format-patch --signoff\" is a slightly different\nquestion.  The current behavior is working as intended, in the sense\nthat it signs off just as \"commit -s\" would, and then separately sticks\nthe result into the email. The fact that \"---\" in the commit message is\nindistinguishable from the ones added by format-patch is mostly an\naccident.\n\nThat said, it's kind of a useful accident for some workflows, exactly\nbecause you can carry these non-commit-message notes inside the commit\nmessage. And since we know how any in-commit-message \"---\" will be\ntreated by git-am on the other side, it might be reasonable for\nformat-patch to start considering them to be syntactically significant.\n\nSo I guess I would disagree that it's a bug exactly, in that the\nworkflow you're advocating was never meant to be supported. But I don't\nsee any reason we couldn't be a little friendlier to it, if somebody\nwanted to teach format-patch to do so.\n\nAn alternative workflow would be to use git-notes to attach the\nchangelog data to the commit. Those are shown after the \"---\" by\nformat-patch already. Unfortunately, keeping them up to date is kind of\nannoying. Ages ago, I had a patch to let you modify them while editing\nthe commit message, which makes it pretty seamless:\n\n  https://lore.kernel.org/git/20110225133056.GA1026@sigill.intra.peff.net/\n\nI carried the patch in my local build for a while, but never really\nended up using it. So I never polished it further. But I think it's\nstill fundamentally a reasonable idea, if somebody is interested in\ncarrying it forward. If so, here's the version I've been rebasing\nforward over the years:\n\n  https://github.com/peff/git jk/commit-notes-wip\n\nbut it doesn't seem to actually pass its own tests anymore (so it may or\nmay not be a helpful starting point. ;) ).\n\n-Peff\n"},{"id":"432879","messageId":"CAJfuBxxuGf4aHjD6S0sLHgM0_SkqwY5tgEVBPvTANbak+5DFLA@mail.gmail.com","threadId":"56288","inReplyTo":"YRqUK3cRFJmANzDd@coredump.intra.peff.net","subject":"Re: git format-patch -s enhancement","fromName":"","fromEmail":"jim.cromie@gmail.com","sentAt":"2021-08-16T21:49:21Z","receivedAt":"2021-08-16T21:49:50Z","isPatch":false,"sender":{"key":"jim.cromie@gmail.com","avatar":null},"body":"On Mon, Aug 16, 2021 at 10:37 AM Jeff King <peff@peff.net> wrote:\n>\n> On Sun, Aug 15, 2021 at 05:07:34PM -0600, jim.cromie@gmail.com wrote:\n>\n> > git format-patch -s is sub-optimal :\n> > it appends the SoB,\n> > which falls after the snips\n> > ---\n> > changelog ...\n> > that the commit message may contain\n> >\n> >\n> > So it misfires on any maintainer scripts\n> > expecting the SoB above the 1st snip.\n> >\n> > The workaround is manual SoBs above any snips.\n> >\n> > I note this in -s doc,\n> >\n> >            Add a Signed-off-by trailer to the commit message, using\n> > the committer identity of yourself.\n> >            See the signoff option in git-commit(1) for more information.\n> >\n> > \"trailer\" is really \"document current working behavior\"\n> > (normative docu-speak, so to speak;)\n> >\n> > Ideal behavior is to find 1st in-body  --- snip\n> > and insert there\n>\n> The big disconnect here is that \"---\" snip lines are not meant to be\n> meaningful within commit messages themselves. They are part of the\n> process of sticking a commit message into an email. So format-patch and\n> git-am know about them, but \"git commit\" for example doesn't.\n>\n> So \"git commit --signoff\" probably shouldn't take them into account when\n> deciding the end of a commit message. The user might or might not have\n> meant \"---\" to be syntactically meaningful, depending on whether they\n> plan to send the message with format-patch (and changing the behavior\n> now is questionable).\n>\n> Doing so with \"git format-patch --signoff\" is a slightly different\n> question.  The current behavior is working as intended, in the sense\n> that it signs off just as \"commit -s\" would, and then separately sticks\n> the result into the email. The fact that \"---\" in the commit message is\n> indistinguishable from the ones added by format-patch is mostly an\n> accident.\n>\n> That said, it's kind of a useful accident for some workflows, exactly\n> because you can carry these non-commit-message notes inside the commit\n> message. And since we know how any in-commit-message \"---\" will be\n> treated by git-am on the other side, it might be reasonable for\n> format-patch to start considering them to be syntactically significant.\n>\n> So I guess I would disagree that it's a bug exactly, in that the\n> workflow you're advocating was never meant to be supported. But I don't\n> see any reason we couldn't be a little friendlier to it, if somebody\n> wanted to teach format-patch to do so.\n>\n\nagreed, notabug.\n\nbut it might fall afoul of others' mail handler scripts,\nIve had a couple replys implying missed delivery,\nmaybe because of details like '---'\n\nIm just gonna add my SoB either at commit time, or manually.\nIt will be interesting to see what happens to an SoB in a commit\nwhen its revised and --- changelogged\n\nthanks\n\n\n> An alternative workflow would be to use git-notes to attach the\n> changelog data to the commit. Those are shown after the \"---\" by\n> format-patch already. Unfortunately, keeping them up to date is kind of\n> annoying. Ages ago, I had a patch to let you modify them while editing\n> the commit message, which makes it pretty seamless:\n>\n>   https://lore.kernel.org/git/20110225133056.GA1026@sigill.intra.peff.net/\n>\n> I carried the patch in my local build for a while, but never really\n> ended up using it. So I never polished it further. But I think it's\n> still fundamentally a reasonable idea, if somebody is interested in\n> carrying it forward. If so, here's the version I've been rebasing\n> forward over the years:\n>\n>   https://github.com/peff/git jk/commit-notes-wip\n>\n> but it doesn't seem to actually pass its own tests anymore (so it may or\n> may not be a helpful starting point. ;) ).\n>\n> -Peff\n"}]}