{"thread":{"id":"63155","subject":"[PATCH] format-patch: use raw format for notes","startedAt":"2025-03-18T18:03:16Z","lastAt":"2025-03-19T00:52:54Z","messageCount":5,"participants":["Tuomas Ahola","brian m. carlson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"514543","messageId":"20250318180251.3712-1-taahol@utu.fi","threadId":"63155","inReplyTo":null,"subject":"[PATCH] format-patch: use raw format for notes","fromName":"Tuomas Ahola","fromEmail":"taahol@utu.fi","sentAt":"2025-03-18T18:02:51Z","receivedAt":"2025-03-18T18:03:16Z","isPatch":true,"sender":{"key":"taahol@utu.fi","avatar":"https://avatars.githubusercontent.com/u/114303477?v=4"},"body":"The default formatting of commit notes by git format-patch --notes\ndoesn't make a very good fit. It would be more beneficial to use the\nraw format for CMIT_FMT_EMAIL and CMIT_FMT_MBOXRD.\n\nSigned-off-by: Tuomas Ahola \n---\n log-tree.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 8b184d6776..c40a7599d0 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -857,7 +857,9 @@ void show_log(struct rev_info *opt)\n \t\tint raw;\n \t\tstruct strbuf notebuf = STRBUF_INIT;\n \n-\t\traw = (opt->commit_format == CMIT_FMT_USERFORMAT);\n+\t\traw = (opt->commit_format == CMIT_FMT_USERFORMAT ||\n+\t\t opt->commit_format == CMIT_FMT_EMAIL ||\n+\t\t opt->commit_format == CMIT_FMT_MBOXRD);\n \t\tformat_display_notes(&commit->object.oid, ¬ebuf,\n \t\t\t\t get_log_output_encoding(), raw);\n \t\tctx.notes_message = strbuf_detach(¬ebuf, NULL);\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\n-- \n2.30.2\n\n"},{"id":"514553","messageId":"Z9niQ9v-SjsNgTJR@tapette.crustytoothpaste.net","threadId":"63155","inReplyTo":"20250318180251.3712-1-taahol@utu.fi","subject":"Re: [PATCH] format-patch: use raw format for notes","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-03-18T21:14:43Z","receivedAt":"2025-03-18T21:14:45Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-03-18 at 18:02:51, Tuomas Ahola wrote:\n> The default formatting of commit notes by git format-patch --notes\n> doesn't make a very good fit. It would be more beneficial to use the\n> raw format for CMIT_FMT_EMAIL and CMIT_FMT_MBOXRD.\n\nI don't really use notes, so I don't have a strong opinion, but I think\n\"doesn't make a very good fit\" isn't really a compelling argument, since\nit's very opinionated and short on details. Maybe you could explain\nthe current status in terms of the output one receives and mention in\ndetail why it's unsuitable, and then explain the benefits of the raw\nformat in terms of its output and why it's better.\n\nIdeally, I, someone who has touched the notes code but is not intimately\nfamiliar with it, would be able to understand the advantages and\ndisadvantages of the change by reading the commit message, and I'm\nafraid I don't right now.\n\nMy guess, based on the very small amount of code I've touched there and\nmy recollection from that, is that there's some sort of prefix printed\nin the format-patch output, and that prevents the notes output from\nbeing nicely formatted as an additional explainer when sending a patch,\nso it requires further editing, which is a hassle. Therefore, it would\nbe more convenient for users to not have to do that by using the raw\nmode. But that's just a guess.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"514555","messageId":"xmqqy0x2yr6b.fsf@gitster.g","threadId":"63155","inReplyTo":"20250318180251.3712-1-taahol@utu.fi","subject":"Re: [PATCH] format-patch: use raw format for notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-18T21:17:32Z","receivedAt":"2025-03-18T21:17:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tuomas Ahola writes:\n\n> The default formatting of commit notes by git format-patch --notes\n> doesn't make a very good fit. It would be more beneficial to use the\n> raw format for CMIT_FMT_EMAIL and CMIT_FMT_MBOXRD.\n\nHmph. That is unfortunately quite subjective. \"doesn't make a very\ngood fit\" why? \"more benefitial\" why?\n\nAnd it turns out that using \"raw\" is not a good choice in the\ncontext of e-mailed patches. Read on.\n\n> Signed-off-by: Tuomas Ahola \n> ---\n> log-tree.c | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/log-tree.c b/log-tree.c\n> index 8b184d6776..c40a7599d0 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -857,7 +857,9 @@ void show_log(struct rev_info *opt)\n> \t\tint raw;\n> \t\tstruct strbuf notebuf = STRBUF_INIT;\n> \n> -\t\traw = (opt->commit_format == CMIT_FMT_USERFORMAT);\n> +\t\traw = (opt->commit_format == CMIT_FMT_USERFORMAT ||\n> +\t\t opt->commit_format == CMIT_FMT_EMAIL ||\n> +\t\t opt->commit_format == CMIT_FMT_MBOXRD);\n\nAfter applying this patch and running\n\n $ git format-patch --notes=amlog -1\n\n(where refs/notes/amlog holds commit to original e-mail mapping), I\nget this:\n\n ...\n Subject: [PATCH] format-patch: use raw format for notes\n\n ...\n Signed-off-by: Tuomas Ahola \n Signed-off-by: Junio C Hamano \n ---\n\n Notes (amlog):\n Message-Id: <20250318180251.3712-1-taahol@utu.fi>\n\n log-tree.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n ...\n\nBut with this patch in place, I instead get this:\n\n ...\n Subject: [PATCH] format-patch: use raw format for notes\n\n ...\n Signed-off-by: Tuomas Ahola \n Signed-off-by: Junio C Hamano \n ---\n Message-Id: <20250318180251.3712-1-taahol@utu.fi>\n\n log-tree.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n ...\n\nThere is no indication where the note came from, and more\nimportantly, the contents of the note loses its crucial leading\nspaces that makes sure that any random lines in the note that happen\nto begin with \"diff\", \"---\", etc. are not mistaken as the beginning\nof the first patch.\n\nSo, no, this change is not a good thing to do, at least in its\ncurrent form. Besides, unconditional change like this will break\nexisting users.\n\n"},{"id":"514556","messageId":"20250318.233012.1423505396684882738.taahol@utu.fi","threadId":"63155","inReplyTo":"xmqqy0x2yr6b.fsf@gitster.g","subject":"Re: [PATCH] format-patch: use raw format for notes","fromName":"Tuomas Ahola","fromEmail":"taahol@utu.fi","sentAt":"2025-03-18T21:30:12Z","receivedAt":"2025-03-18T21:30:20Z","isPatch":true,"sender":{"key":"taahol@utu.fi","avatar":"https://avatars.githubusercontent.com/u/114303477?v=4"},"body":"From: Junio C Hamano \nSubject: Re: [PATCH] format-patch: use raw format for notes\nDate: Tue, 18 Mar 2025 14:17:32 -0700\n\n> [--] more importantly, the contents of the note loses its crucial\n> leading spaces that makes sure that any random lines in the note\n> that happen to begin with \"diff\", \"---\", etc. are not mistaken as\n> the beginning of the first patch.\n\nThanks for quick response. That was indeed a compelling point.\n\n> So, no, this change is not a good thing to do, at least in its\n> current form. Besides, unconditional change like this will break\n> existing users.\n\nI see that similar patch was proposed in 2017. I should have searched\nmore thoroughly, I guess.\n\n--Tuomas A.\n"},{"id":"514599","messageId":"xmqq4izpzvrw.fsf@gitster.g","threadId":"63155","inReplyTo":"20250318.233012.1423505396684882738.taahol@utu.fi","subject":"Re: [PATCH] format-patch: use raw format for notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-19T00:52:51Z","receivedAt":"2025-03-19T00:52:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tuomas Ahola writes:\n\n> From: Junio C Hamano \n> Subject: Re: [PATCH] format-patch: use raw format for notes\n> Date: Tue, 18 Mar 2025 14:17:32 -0700\n>\n>> [--] more importantly, the contents of the note loses its crucial\n>> leading spaces that makes sure that any random lines in the note\n>> that happen to begin with \"diff\", \"---\", etc. are not mistaken as\n>> the beginning of the first patch.\n>\n> Thanks for quick response. That was indeed a compelling point.\n>\n>> So, no, this change is not a good thing to do, at least in its\n>> current form. Besides, unconditional change like this will break\n>> existing users.\n>\n> I see that similar patch was proposed in 2017. I should have searched\n> more thoroughly, I guess.\n\nHeh, your archive spelunking skills are far superiour than mine, it\nseems. And in\n\nhttps://lore.kernel.org/git/xmqqingw8ppj.fsf@gitster.mtv.corp.google.com/\n\nI see that I said exactly the same thing to exactly the same patch.\n\nIt is not to say that I've been a good person to be very consistent\n(I do not have to be---over the years I can hear more opinions from\nothers that may sway how I think about the same issue), but says\nthat there aren't new arguments to sway the old decision in the past\n7.5 years.\n\nAnd exactly the same way as back then, I am open to a valid argument\nto add such an output as an optional feature if there is a good use\ncase for it.\n\nThanks.\n\n\n"}]}