{"thread":{"id":"63136","subject":"[PATCH] docs: clarify meaning of core.commentString=auto","startedAt":"2025-03-15T14:09:15Z","lastAt":"2025-03-21T10:28:27Z","messageCount":8,"participants":["Oswald Buddenhagen","Junio C Hamano","Taylor Blau","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"514351","messageId":"20250315140913.577404-1-oswald.buddenhagen@gmx.de","threadId":"63136","inReplyTo":null,"subject":"[PATCH] docs: clarify meaning of core.commentString=auto","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2025-03-15T14:09:13Z","receivedAt":"2025-03-15T14:09:15Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"I had to read the source to make sense of the feature, which is clearly\nnot an acceptable state. Make the docu more specific and less\nmisleading.\n\nSigned-off-by: Oswald Buddenhagen \n\n---\n\nCc: Nguyễn Thái Ngọc Duy \n\ngiven the rather crippling limitations of this feature, does anyone\nactually use it?\n---\n Documentation/config/core.adoc | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc\nindex 8f6d8e7754..b470da72ba 100644\n--- a/Documentation/config/core.adoc\n+++ b/Documentation/config/core.adoc\n@@ -526,8 +526,11 @@ core.commentString::\n \tcommented, and removes them after the editor returns\n \t(default '#').\n +\n-If set to \"auto\", `git-commit` would select a character that is not\n-the beginning character of any line in existing commit messages.\n+If set to \"auto\", `git-commit` will select the first character\n+from the set \"#;@!$%^&|:\" that does not appear at the beginning\n+of any line in the prepared commit message prior to editing.\n+Note that this makes it impossible to include comments in the\n+prepare-commit-msg hook's output or the commit message template.\n +\n Note that these two variables are aliases of each other, and in modern\n versions of Git you are free to use a string (e.g., `//` or `⁑⁕⁑`) with\n-- \n2.49.0.416.g2f302f2ef0.dirty\n\n"},{"id":"514432","messageId":"xmqqv7s78l8t.fsf@gitster.g","threadId":"63136","inReplyTo":"20250315140913.577404-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH] docs: clarify meaning of core.commentString=auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-17T20:17:54Z","receivedAt":"2025-03-17T20:17:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen writes:\n\n> -If set to \"auto\", `git-commit` would select a character that is not\n> -the beginning character of any line in existing commit messages.\n\nThis is so far in the past but I suspect this was deliberately left\nvague so that we can add (or subtract) the set of possible letters\nto use.\n\n> +If set to \"auto\", `git-commit` will select the first character\n> +from the set \"#;@!$%^&|:\" that does not appear at the beginning\n> +of any line in the prepared commit message prior to editing.\n\nSo I am not sure if this is an improvement.\n\n> +Note that this makes it impossible to include comments in the\n> +prepare-commit-msg hook's output or the commit message template.\n\nCare to rephrase? There are degrees of possibilities and \"makes it\nimpossible\" is being overly broad.\n\nI suspect you are saying that it is not nice to make it the\nresponsibility of the end-user who chooses \"auto\" to ensure that\nthey adjust the default '#' comments injected from the template or\nhook output when\n\n - they have a line that begins with '#' in their message;\n - the \"auto\" mechanism chooses to use ';' as the comment character;\n - the template is written assuming '#' as the comment character and\n has comments.\n\nbefore making a commit. But \"this makes it impossible\" does not\nquite convey that to casual readers.\n\nThanks.\n"},{"id":"514437","messageId":"Z9iVVD988M4XUyYO@nand.local","threadId":"63136","inReplyTo":"xmqqv7s78l8t.fsf@gitster.g","subject":"Re: [PATCH] docs: clarify meaning of core.commentString=auto","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-17T21:34:12Z","receivedAt":"2025-03-17T21:34:15Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Mar 17, 2025 at 01:17:54PM -0700, Junio C Hamano wrote:\n> Oswald Buddenhagen writes:\n>\n> > -If set to \"auto\", `git-commit` would select a character that is not\n> > -the beginning character of any line in existing commit messages.\n>\n> This is so far in the past but I suspect this was deliberately left\n> vague so that we can add (or subtract) the set of possible letters\n> to use.\n>\n> > +If set to \"auto\", `git-commit` will select the first character\n> > +from the set \"#;@!$%^&|:\" that does not appear at the beginning\n> > +of any line in the prepared commit message prior to editing.\n>\n> So I am not sure if this is an improvement.\n\nI had a similar thought while reading. The vague wording of the existing\ntext gives us freedom to change that set of characters in the code\nwithout the possibility of the documentation becoming stale.\n\nThat's pretty academic, though, so I don't have a strong feeling against\nthis portion of the patch, but I do vaguely prefer the existing wording.\n\nThanks,\nTaylor\n"},{"id":"514513","messageId":"Z9lcXR6sL3UWlL33@ugly","threadId":"63136","inReplyTo":"Z9iVVD988M4XUyYO@nand.local","subject":"Re: [PATCH] docs: clarify meaning of core.commentString=auto","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2025-03-18T11:43:25Z","receivedAt":"2025-03-18T11:43:27Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Mon, Mar 17, 2025 at 01:17:54PM -0700, Junio C Hamano wrote:\n>Oswald Buddenhagen writes:\n>\n>> -If set to \"auto\", `git-commit` would select a character that is not\n>> -the beginning character of any line in existing commit messages.\n>\n>This is so far in the past but I suspect this was deliberately left\n>vague so that we can add (or subtract) the set of possible letters\n>to use.\n>\nno such consideration was voiced at any point.\nhttps://lore.kernel.org/git/CALy3b+m7YkYB+mPEnAQnjKFAwUS_PqCUFtuxzN7hwhmNfMrw3Q@mail.gmail.com/T/#u\n\nOn Mon, Mar 17, 2025 at 05:34:12PM -0400, Taylor Blau wrote:\n>I had a similar thought while reading. The vague wording of the\n>existing\n>text gives us freedom to change that set of characters in the code\n>without the possibility of the documentation becoming stale.\n>\n>That's pretty academic, though, so I don't have a strong feeling against\n>this portion of the patch, but I do vaguely prefer the existing wording.\n>\napart from changing it being academic, the feature is also formally\nuseless without documenting the candidate comment characters. formally,\nbecause in practice the user would just guess, but that doesn't make the\nomission a good thing.\n\nOn Mon, Mar 17, 2025 at 01:17:54PM -0700, Junio C Hamano wrote:\n>> +Note that this makes it impossible to include comments in the\n>> +prepare-commit-msg hook's output or the commit message template.\n>\n>Care to rephrase? There are degrees of possibilities and \"makes it\n>impossible\" is being overly broad.\n>\n>I suspect you are saying that it is not nice to make it the\n>responsibility of the end-user who chooses \"auto\" to ensure that\n>they adjust the default '#' comments injected from the template or\n>hook output when\n>\n> - they have a line that begins with '#' in their message;\n> - the \"auto\" mechanism chooses to use ';' as the comment character;\n> - the template is written assuming '#' as the comment character and\n> has comments.\n>\n>before making a commit. But \"this makes it impossible\" does not\n>quite convey that to casual readers.\n>\nno, i meant what i wrote: it makes it _literally_ impossible. it follows\nfrom the preceding sentence that _whatever_ is in the template will NOT\nbe the comment char. the commit that introduced that feature (84c9dc2c5)\nalready mentioned that limitation.\n\nreading through the thread of the original submission, the feature is a\nworkaround for `commit -m` and `commit --amend` being inconsistent wrt.\nmessage washing. i find it surprising that this patch didn't get any\npush-back, even though the thread mentioned the correct way to enforce\nconsistency (use --amend with --no-edit), and the fact that the user\nshould have set the commentChar to non-'#' even if his primary method to\ncreate commit messages was with -m. i don't see how (or why) anyone\nwould integrate this option into any practical workflow, and therefore\nconsider it a mis-feature that should be done away with. but knowing how\npeople here react to such proposals, it seems most practical to document\nthe feature sufficiently well to enable users to easily draw the\nconclusion that it is, in fact, nonsense.\n\n"},{"id":"514538","messageId":"xmqqa59i45wc.fsf@gitster.g","threadId":"63136","inReplyTo":"Z9lcXR6sL3UWlL33@ugly","subject":"Re: [PATCH] docs: clarify meaning of core.commentString=auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-18T17:15:15Z","receivedAt":"2025-03-18T17:15:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen writes:\n\n>>before making a commit. But \"this makes it impossible\" does not\n>>quite convey that to casual readers.\n>>\n> no, i meant what i wrote: it makes it _literally_ impossible. it follows\n> from the preceding sentence that _whatever_ is in the template will NOT\n> be the comment char.\n\nOK, it (i.e. the order in which things happen) would be a good thing\nto add to the explanation, to unconfuse readers who (incorrectly)\nguess that auto comment character is determined and then template is\nread, which is where my comment came from.\n\n> reading through the thread of the original submission, the feature is a\n> workaround for `commit -m` and `commit --amend` being inconsistent wrt.\n> message washing.\n\nPerhaps somebody can be talked into fixing it ;-)\n\nWith a clear explanation, I am OK if somebody wants to advocate to\ndeprecate (and remove at Git 3.0 boundary) the \"auto\" support ;-)\n\nThanks.\n"},{"id":"514674","messageId":"Z9sLAEbE9lAInBXz@ugly","threadId":"63136","inReplyTo":"xmqqa59i45wc.fsf@gitster.g","subject":"Re: [PATCH] docs: clarify meaning of core.commentString=auto","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2025-03-19T18:20:48Z","receivedAt":"2025-03-19T18:20:53Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Tue, Mar 18, 2025 at 10:15:15AM -0700, Junio C Hamano wrote:\n>Oswald Buddenhagen writes:\n>> no, i meant what i wrote: it makes it _literally_ impossible. it\n>> follows\n>> from the preceding sentence that _whatever_ is in the template will NOT\n>> be the comment char.\n>\n>OK, it (i.e. the order in which things happen) would be a good thing\n>to add to the explanation, to unconfuse readers who (incorrectly)\n>guess that auto comment character is determined and then template is\n>read, which is where my comment came from.\n>\ni hoped the formulation \"the prepared commit message prior to editing\"\nwould be unambiguous. did i miss anything? if you just made a thinko and\nactually agree, then i'd leave the patch as-is, as it doesn't seem worth\nexpanding _that_ docu any further.\n\n>> reading through the thread of the original submission, the feature is a\n>> workaround for `commit -m` and `commit --amend` being inconsistent wrt.\n>> message washing.\n>\n>Perhaps somebody can be talked into fixing it ;-)\n>\n>With a clear explanation, I am OK if somebody wants to advocate to\n>deprecate (and remove at Git 3.0 boundary) the \"auto\" support ;-)\n>\nhow would we go about this in practice? just a notice in the docu, or\nsome mechanism which would complain at runtime? under what circumstances\n(i.e., how to enable/squelch it)?\n"},{"id":"514752","messageId":"6a3154e0-e7bc-45ae-b554-67ccab18727a@gmail.com","threadId":"63136","inReplyTo":"Z9sLAEbE9lAInBXz@ugly","subject":"Re: [PATCH] docs: clarify meaning of core.commentString=auto","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-20T10:21:10Z","receivedAt":"2025-03-20T10:21:14Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 19/03/2025 18:20, Oswald Buddenhagen wrote:\n> On Tue, Mar 18, 2025 at 10:15:15AM -0700, Junio C Hamano wrote:\n>>> reading through the thread of the original submission, the feature is a\n>>> workaround for `commit -m` and `commit --amend` being inconsistent wrt.\n>>> message washing.\n>>\n>> Perhaps somebody can be talked into fixing it ;-)\n>>\n>> With a clear explanation, I am OK if somebody wants to advocate to\n>> deprecate (and remove at Git 3.0 boundary) the \"auto\" support ;-)\n\nI think that may be best. Looking at the sequencer I don't think \nappend_conflicts_hint(), the \"fixup\" or \"squash\" commands of \"rebase \n-i\", or the \"--reference\" option of \"git revert\" are compatible with \ncore.commentStr=auto. For rebase making it work would mean scanning the \nmessages of all the commits to be squash before picking the first one \nwhich is a pain.\n\n> how would we go about this in practice? just a notice in the docu, or\n> some mechanism which would complain at runtime? under what circumstances\n> (i.e., how to enable/squelch it)?\n\nI think we'd want to start printing some advice when \ncore.commentStr=auto explaining why it has been deprecated and that it \nwill be removed when Git 3.0 is released. We should allow that advice to \nbe suppressed setting advice.autoCommentStr (other name suggestions \nwelcome). We would also want to add an item to BreakingChanges.adoc \nexplaining why it is being removed and add \"#ifndef \nWITH_BREAKING_CHANGES\" around the code that handles core.commentStr=auto \nin builtin/commit.c and guard the documentation with \n\"ifdef::with_breaking_changes[]\". We may want to make \ncore.commentStr=auto an error when breaking changes are enabled as well.\n\nBest Wishes\n\nPhillip\n"},{"id":"514829","messageId":"xmqqfrj6vfsn.fsf@gitster.g","threadId":"63136","inReplyTo":"6a3154e0-e7bc-45ae-b554-67ccab18727a@gmail.com","subject":"Re: [PATCH] docs: clarify meaning of core.commentString=auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-21T10:28:24Z","receivedAt":"2025-03-21T10:28:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> I think we'd want to start printing some advice when\n> core.commentStr=auto explaining why it has been deprecated and that it\n> will be removed when Git 3.0 is released. We should allow that advice\n> to be suppressed setting advice.autoCommentStr (other name suggestions\n> welcome). We would also want to add an item to BreakingChanges.adoc\n> explaining why it is being removed and add \"#ifndef\n> WITH_BREAKING_CHANGES\" around the code that handles\n> core.commentStr=auto in builtin/commit.c and guard the documentation\n> with \"ifdef::with_breaking_changes[]\".\n\nAll of these are good action items in a good transition plan, I\nwould say.\n\n> We may want to make\n> core.commentStr=auto an error when breaking changes are enabled as\n> well.\n\nI am not so sure. As commentStr is a random string that is used to\nprefix any comment line, \"auto\" is just a (albeit weird) string, so\nnot doing anything special would be good enough.\n"}]}