threads / patch / 63136

patchdocs: clarify meaning of core.commentString=auto

Subject: [PATCH] docs: clarify meaning of core.commentString=auto

## tl;dr

8 messages between Mar 15, 2025 and Mar 21, 2025. Diffs are folded; open one to read it.

replies: 7people: 4as markdown or json

Oswald Buddenhagen· Mar 15, 2025, 14:09 UTC · lore

I had to read the source to make sense of the feature, which is clearly not an acceptable state. Make the docu more specific and less misleading.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
Cc: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
given the rather crippling limitations of this feature, does anyone
actually use it?
---
 Documentation/config/core.adoc | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
Show changes to Documentation/config/core.adoc +5 −2
diff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc
index 8f6d8e7754..b470da72ba 100644
--- a/Documentation/config/core.adoc
+++ b/Documentation/config/core.adoc
@@ -526,8 +526,11 @@ core.commentString::
 	commented, and removes them after the editor returns
 	(default '#').
 +
-If set to "auto", `git-commit` would select a character that is not
-the beginning character of any line in existing commit messages.
+If set to "auto", `git-commit` will select the first character
+from the set "#;@!$%^&|:" that does not appear at the beginning
+of any line in the prepared commit message prior to editing.
+Note that this makes it impossible to include comments in the
+prepare-commit-msg hook's output or the commit message template.
 +
 Note that these two variables are aliases of each other, and in modern
 versions of Git you are free to use a string (e.g., `//` or `⁑⁕⁑`) with
-- 
2.49.0.416.g2f302f2ef0.dirty
Junio C Hamano· Mar 17, 2025, 20:17 UTC · re: Oswald Buddenhagen · lore

Re: [PATCH] docs: clarify meaning of core.commentString=auto

Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
> -If set to "auto", `git-commit` would select a character that is not
> -the beginning character of any line in existing commit messages.

This is so far in the past but I suspect this was deliberately left vague so that we can add (or subtract) the set of possible letters to use.

> +If set to "auto", `git-commit` will select the first character
> +from the set "#;@!$%^&|:" that does not appear at the beginning
> +of any line in the prepared commit message prior to editing.
So I am not sure if this is an improvement.
> +Note that this makes it impossible to include comments in the
> +prepare-commit-msg hook's output or the commit message template.

Care to rephrase? There are degrees of possibilities and "makes it impossible" is being overly broad.

I suspect you are saying that it is not nice to make it the responsibility of the end-user who chooses "auto" to ensure that they adjust the default '#' comments injected from the template or hook output when

 - they have a line that begins with '#' in their message;
 - the "auto" mechanism chooses to use ';' as the comment character;
 - the template is written assuming '#' as the comment character and
   has comments.

before making a commit. But "this makes it impossible" does not quite convey that to casual readers.

Thanks.
Taylor Blau· Mar 17, 2025, 21:34 UTC · re: Junio C Hamano · lore

Re: [PATCH] docs: clarify meaning of core.commentString=auto

On Mon, Mar 17, 2025 at 01:17:54PM -0700, Junio C Hamano wrote:
Show 14 quoted lines
> Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
>
> > -If set to "auto", `git-commit` would select a character that is not
> > -the beginning character of any line in existing commit messages.
>
> This is so far in the past but I suspect this was deliberately left
> vague so that we can add (or subtract) the set of possible letters
> to use.
>
> > +If set to "auto", `git-commit` will select the first character
> > +from the set "#;@!$%^&|:" that does not appear at the beginning
> > +of any line in the prepared commit message prior to editing.
>
> So I am not sure if this is an improvement.

I had a similar thought while reading. The vague wording of the existing text gives us freedom to change that set of characters in the code without the possibility of the documentation becoming stale.

That's pretty academic, though, so I don't have a strong feeling against this portion of the patch, but I do vaguely prefer the existing wording.

Thanks, Taylor

Oswald Buddenhagen· Mar 18, 2025, 11:43 UTC · re: Taylor Blau · lore

Re: [PATCH] docs: clarify meaning of core.commentString=auto

On Mon, Mar 17, 2025 at 01:17:54PM -0700, Junio C Hamano wrote:
Show 9 quoted lines
>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
>
>> -If set to "auto", `git-commit` would select a character that is not
>> -the beginning character of any line in existing commit messages.
>
>This is so far in the past but I suspect this was deliberately left
>vague so that we can add (or subtract) the set of possible letters
>to use.
>

no such consideration was voiced at any point. https://lore.kernel.org/git/CALy3b+m7YkYB+mPEnAQnjKFAwUS_PqCUFtuxzN7hwhmNfMrw3Q@mail.gmail.com/T/#u

On Mon, Mar 17, 2025 at 05:34:12PM -0400, Taylor Blau wrote:
Show 8 quoted lines
>I had a similar thought while reading. The vague wording of the
>existing
>text gives us freedom to change that set of characters in the code
>without the possibility of the documentation becoming stale.
>
>That's pretty academic, though, so I don't have a strong feeling against
>this portion of the patch, but I do vaguely prefer the existing wording.
>

apart from changing it being academic, the feature is also formally useless without documenting the candidate comment characters. formally, because in practice the user would just guess, but that doesn't make the omission a good thing.

On Mon, Mar 17, 2025 at 01:17:54PM -0700, Junio C Hamano wrote:
Show 19 quoted lines
>> +Note that this makes it impossible to include comments in the
>> +prepare-commit-msg hook's output or the commit message template.
>
>Care to rephrase?  There are degrees of possibilities and "makes it
>impossible" is being overly broad.
>
>I suspect you are saying that it is not nice to make it the
>responsibility of the end-user who chooses "auto" to ensure that
>they adjust the default '#' comments injected from the template or
>hook output when
>
> - they have a line that begins with '#' in their message;
> - the "auto" mechanism chooses to use ';' as the comment character;
> - the template is written assuming '#' as the comment character and
>   has comments.
>
>before making a commit.  But "this makes it impossible" does not
>quite convey that to casual readers.
>

no, i meant what i wrote: it makes it _literally_ impossible. it follows from the preceding sentence that _whatever_ is in the template will NOT be the comment char. the commit that introduced that feature (84c9dc2c5) already mentioned that limitation.

reading through the thread of the original submission, the feature is a workaround for `commit -m` and `commit --amend` being inconsistent wrt. message washing. i find it surprising that this patch didn't get any push-back, even though the thread mentioned the correct way to enforce consistency (use --amend with --no-edit), and the fact that the user should have set the commentChar to non-'#' even if his primary method to create commit messages was with -m. i don't see how (or why) anyone would integrate this option into any practical workflow, and therefore consider it a mis-feature that should be done away with. but knowing how people here react to such proposals, it seems most practical to document the feature sufficiently well to enable users to easily draw the conclusion that it is, in fact, nonsense.

Junio C Hamano· Mar 18, 2025, 17:15 UTC · re: Oswald Buddenhagen · lore

Re: [PATCH] docs: clarify meaning of core.commentString=auto

Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
Show 6 quoted lines
>>before making a commit.  But "this makes it impossible" does not
>>quite convey that to casual readers.
>>
> no, i meant what i wrote: it makes it _literally_ impossible. it follows
> from the preceding sentence that _whatever_ is in the template will NOT
> be the comment char.

OK, it (i.e. the order in which things happen) would be a good thing to add to the explanation, to unconfuse readers who (incorrectly) guess that auto comment character is determined and then template is read, which is where my comment came from.

> reading through the thread of the original submission, the feature is a
> workaround for `commit -m` and `commit --amend` being inconsistent wrt.
> message washing.
Perhaps somebody can be talked into fixing it ;-)

With a clear explanation, I am OK if somebody wants to advocate to deprecate (and remove at Git 3.0 boundary) the "auto" support ;-)

Thanks.
Oswald Buddenhagen· Mar 19, 2025, 18:20 UTC · re: Junio C Hamano · lore

Re: [PATCH] docs: clarify meaning of core.commentString=auto

On Tue, Mar 18, 2025 at 10:15:15AM -0700, Junio C Hamano wrote:
Show 11 quoted lines
>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
>> no, i meant what i wrote: it makes it _literally_ impossible. it
>> follows
>> from the preceding sentence that _whatever_ is in the template will NOT
>> be the comment char.
>
>OK, it (i.e. the order in which things happen) would be a good thing
>to add to the explanation, to unconfuse readers who (incorrectly)
>guess that auto comment character is determined and then template is
>read, which is where my comment came from.
>

i hoped the formulation "the prepared commit message prior to editing" would be unambiguous. did i miss anything? if you just made a thinko and actually agree, then i'd leave the patch as-is, as it doesn't seem worth expanding _that_ docu any further.

Show 9 quoted lines
>> reading through the thread of the original submission, the feature is a
>> workaround for `commit -m` and `commit --amend` being inconsistent wrt.
>> message washing.
>
>Perhaps somebody can be talked into fixing it ;-)
>
>With a clear explanation, I am OK if somebody wants to advocate to
>deprecate (and remove at Git 3.0 boundary) the "auto" support ;-)
>

how would we go about this in practice? just a notice in the docu, or some mechanism which would complain at runtime? under what circumstances (i.e., how to enable/squelch it)?

Phillip Wood· Mar 20, 2025, 10:21 UTC · re: Oswald Buddenhagen · lore

Re: [PATCH] docs: clarify meaning of core.commentString=auto

On 19/03/2025 18:20, Oswald Buddenhagen wrote:
Show 9 quoted lines
> On Tue, Mar 18, 2025 at 10:15:15AM -0700, Junio C Hamano wrote:
>>> reading through the thread of the original submission, the feature is a
>>> workaround for `commit -m` and `commit --amend` being inconsistent wrt.
>>> message washing.
>>
>> Perhaps somebody can be talked into fixing it ;-)
>>
>> With a clear explanation, I am OK if somebody wants to advocate to
>> deprecate (and remove at Git 3.0 boundary) the "auto" support ;-)

I think that may be best. Looking at the sequencer I don't think append_conflicts_hint(), the "fixup" or "squash" commands of "rebase -i", or the "--reference" option of "git revert" are compatible with core.commentStr=auto. For rebase making it work would mean scanning the messages of all the commits to be squash before picking the first one which is a pain.

> how would we go about this in practice? just a notice in the docu, or
> some mechanism which would complain at runtime? under what circumstances
> (i.e., how to enable/squelch it)?

I think we'd want to start printing some advice when core.commentStr=auto explaining why it has been deprecated and that it will be removed when Git 3.0 is released. We should allow that advice to be suppressed setting advice.autoCommentStr (other name suggestions welcome). We would also want to add an item to BreakingChanges.adoc explaining why it is being removed and add "#ifndef WITH_BREAKING_CHANGES" around the code that handles core.commentStr=auto in builtin/commit.c and guard the documentation with "ifdef::with_breaking_changes[]". We may want to make core.commentStr=auto an error when breaking changes are enabled as well.

Best Wishes
Phillip
Junio C Hamano· Mar 21, 2025, 10:28 UTC · re: Phillip Wood · lore

Re: [PATCH] docs: clarify meaning of core.commentString=auto

Phillip Wood <phillip.wood123@gmail.com> writes:
Show 9 quoted lines
> I think we'd want to start printing some advice when
> core.commentStr=auto explaining why it has been deprecated and that it
> will be removed when Git 3.0 is released. We should allow that advice
> to be suppressed setting advice.autoCommentStr (other name suggestions
> welcome). We would also want to add an item to BreakingChanges.adoc
> explaining why it is being removed and add "#ifndef
> WITH_BREAKING_CHANGES" around the code that handles
> core.commentStr=auto in builtin/commit.c and guard the documentation
> with "ifdef::with_breaking_changes[]".

All of these are good action items in a good transition plan, I would say.

> We may want to make
> core.commentStr=auto an error when breaking changes are enabled as
> well.

I am not so sure. As commentStr is a random string that is used to prefix any comment line, "auto" is just a (albeit weird) string, so not doing anything special would be good enough.

← back to recent threads