Subject: Re: [PATCH 17/16] config: add core.commentString

From: Junio C Hamano <[email-masked]>
Date: Wed, 27 Mar 2024 09:13:31 -0700

Jeff King <[email-masked]> writes:

> Note that you graduated kh/doc-commentchar-is-a-byte, which says "this
> ASCII character" early in the description, which will be incorrect if my
> series is merged.

True.  I could tweak this patch to force a conflict

 core.commentChar::
 core.commentString::
 	Commands such as `commit` and `tag` that let you edit
-	messages consider a line that begins with this character
+	messages consider a line that begins with this string
 	commented, and removes them after the editor returns
 	(default '#').

and let the rerere database to remember the resolution (which will
tweak "string" back to "character").  But I'll prepare a merge-fix
before I forget, which is a cleaner approach.

> An alternative to using "$var cannot ..." in the error messages (if we
> don't like the all-lowercase variable name) is to just say "comment
> strings cannot ...". That vaguely covers both cases, and the message
> printed by the config code itself does mention the actual variable name
> that triggered the error.

OK, because the error() return from this function will trigger
another die() in the caller, e.g.

    error: core.commentchar must have at least one character
    fatal: bad config variable 'core.commentchar' in file '.git/config' at line 6

so we can afford to make the "error" side vague, except that the
"fatal" one is also downcased already, so we are not really solving
anything by making the message vague, I would think.  The posted
patch as-is is prefectly fine.

Side note:
    I wonder if we would later want to somehow _merge_ these two
    error messages, i.e. the lower-level will notice and record the
    nature of the problem instead of calling error(), and the caller
    will use the recorded information while composing the "fatal"
    message to die with.  I actually do not know if it is a good
    idea to begin with.  If we want to do it right, the "record"
    part probably cannot be a simple "stringify into strbuf" that
    will result in lego message that is harder for i18n folks.

$ git diff refs/merge-fix/jk/core-comment-string^!
diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index bd033ab100..bbe869c497 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -522,7 +522,7 @@ core.editor::
 core.commentChar::
 core.commentString::
 	Commands such as `commit` and `tag` that let you edit
-	messages consider a line that begins with this ASCII character
+	messages consider a line that begins with this character
 	commented, and removes them after the editor returns
 	(default '#').
 +