* [PATCH] docs: clarify meaning of core.commentString=auto
@ 2025-03-15 14:09 Oswald Buddenhagen
2025-03-17 20:17 ` Junio C Hamano
0 siblings, 1 reply; 8+ messages in thread
From: Oswald Buddenhagen @ 2025-03-15 14:09 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
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(-)
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] docs: clarify meaning of core.commentString=auto
2025-03-15 14:09 [PATCH] docs: clarify meaning of core.commentString=auto Oswald Buddenhagen
@ 2025-03-17 20:17 ` Junio C Hamano
2025-03-17 21:34 ` Taylor Blau
2025-03-18 11:43 ` Oswald Buddenhagen
0 siblings, 2 replies; 8+ messages in thread
From: Junio C Hamano @ 2025-03-17 20:17 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: git, Nguyễn Thái Ngọc Duy
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] docs: clarify meaning of core.commentString=auto
2025-03-17 20:17 ` Junio C Hamano
@ 2025-03-17 21:34 ` Taylor Blau
2025-03-18 11:43 ` Oswald Buddenhagen
1 sibling, 0 replies; 8+ messages in thread
From: Taylor Blau @ 2025-03-17 21:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Oswald Buddenhagen, git, Nguyễn Thái Ngọc Duy
On Mon, Mar 17, 2025 at 01:17:54PM -0700, Junio C Hamano wrote:
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] docs: clarify meaning of core.commentString=auto
2025-03-17 20:17 ` Junio C Hamano
2025-03-17 21:34 ` Taylor Blau
@ 2025-03-18 11:43 ` Oswald Buddenhagen
2025-03-18 17:15 ` Junio C Hamano
1 sibling, 1 reply; 8+ messages in thread
From: Oswald Buddenhagen @ 2025-03-18 11:43 UTC (permalink / raw)
To: Junio C Hamano, Taylor Blau; +Cc: git, Nguyễn Thái Ngọc Duy
On Mon, Mar 17, 2025 at 01:17:54PM -0700, Junio C Hamano wrote:
>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:
>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:
>> +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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] docs: clarify meaning of core.commentString=auto
2025-03-18 11:43 ` Oswald Buddenhagen
@ 2025-03-18 17:15 ` Junio C Hamano
2025-03-19 18:20 ` Oswald Buddenhagen
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2025-03-18 17:15 UTC (permalink / raw)
To: Oswald Buddenhagen
Cc: Taylor Blau, git, Nguyễn Thái Ngọc Duy
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
>>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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] docs: clarify meaning of core.commentString=auto
2025-03-18 17:15 ` Junio C Hamano
@ 2025-03-19 18:20 ` Oswald Buddenhagen
2025-03-20 10:21 ` Phillip Wood
0 siblings, 1 reply; 8+ messages in thread
From: Oswald Buddenhagen @ 2025-03-19 18:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Taylor Blau, git, Nguyễn Thái Ngọc Duy
On Tue, Mar 18, 2025 at 10:15:15AM -0700, Junio C Hamano wrote:
>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.
>> 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)?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] docs: clarify meaning of core.commentString=auto
2025-03-19 18:20 ` Oswald Buddenhagen
@ 2025-03-20 10:21 ` Phillip Wood
2025-03-21 10:28 ` Junio C Hamano
0 siblings, 1 reply; 8+ messages in thread
From: Phillip Wood @ 2025-03-20 10:21 UTC (permalink / raw)
To: Oswald Buddenhagen, Junio C Hamano; +Cc: Taylor Blau, git
On 19/03/2025 18:20, Oswald Buddenhagen wrote:
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] docs: clarify meaning of core.commentString=auto
2025-03-20 10:21 ` Phillip Wood
@ 2025-03-21 10:28 ` Junio C Hamano
0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2025-03-21 10:28 UTC (permalink / raw)
To: Phillip Wood; +Cc: Oswald Buddenhagen, Taylor Blau, git
Phillip Wood <phillip.wood123@gmail.com> writes:
> 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-03-21 10:28 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-15 14:09 [PATCH] docs: clarify meaning of core.commentString=auto Oswald Buddenhagen
2025-03-17 20:17 ` Junio C Hamano
2025-03-17 21:34 ` Taylor Blau
2025-03-18 11:43 ` Oswald Buddenhagen
2025-03-18 17:15 ` Junio C Hamano
2025-03-19 18:20 ` Oswald Buddenhagen
2025-03-20 10:21 ` Phillip Wood
2025-03-21 10:28 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).