On Tue, Mar 5, 2024, at 16:32, Junio C Hamano wrote:

"Kristoffer Haugsbakk" [off-list ref] writes:

quoted

quoted

As an example, with
git config --add core.commentChar •  // Bullet (U+2022)
git does not complain, but it is rejected later.

I think this is more about `git config --add` not doing any
validation. It just sets things. You can do `git config --add
core.commentChar 'ffd'` and get the same effect.

This is not wrong per-se, but it merely explains why "config" takes
it without complaining (the command just does not know anything
about what each variable means and what the valid range of values
are).  core.commentChar is limited to "a byte" so in the context of
everything else (like commit log message in the editor) being UTF-8,
it means ASCII would only work there.

Yep, I neglected to mention that part.

I personally do not see a reason, however, why we need to be limited
to a single byte, though.  If a patch cleanly implements to allow us
to use any one-or-more-byte sequence as core.commentChar, I do not
offhand see a good reason to reject it---it would be fully backward
compatible and allows you to use a UTF-8 charcter outside ASCII, as
well as "//" and the like.

Allow one codepoint or a string? Since a Unicode “character” can be
composed of multiple codepoints. And at that point it might be more work
to validate that it is a “character” compared to allowing any kind of
string.

Maybe introduce `core.commentString` and make it a synonym for
`core.commentChar`?

The core part of "diff" is very much line oriented, and after
chopping your random sequence of bytes at each LF that appears in
it, the code is pretty oblivious to the character boundary, except
for a few cases.  "-w" needs to know what the whitespace characters
are (it knows only the limited basic set like SP HT and probably
VT), "-i" needs to know that "A" and "a" are equivalent (I think it
only knows the ASCII, but I may be misremembering).  Outside the
core part of "diff", there are frills that need to know about
character boundaries, like chopping the function header comment
placed on a hunk header "@@ -1682,7 +1682,7 @@" to a reasonable
length, --color-words/--word-diff that first separates lines into
multi-character tokens and align matching sequences in them, etc.

Ah, interesting. Thanks :)

As you said, we should document core.commentChar as limited to an
ASCII character, at least as a short term solution.

Aha, I see now that the config documentation doesn’t make that clear.

-- >8 --
Subject: [PATCH] config: document `core.commentChar` as ASCII-only

d3b3419f8f2 (config: tell the user that we expect an ASCII character,
2023-03-27) updated an error message to make clear that this option
specifically wants an ASCII character but neglected to consider the
config documentation.

Reported-by: Manlio Perillo <redacted>
Signed-off-by: Kristoffer Haugsbakk <redacted>
---

Notes (series):
    I didn’t find any other relevant occurences with
    
        git grep 'commentChar' -- ':(exclude)po'
    
    `Documentation/git-commit.txt` mentions it but it doesn’t seem like a
    clarification is needed in that context.

 Documentation/config/core.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 0e8c2832bf9..2d4bbdb25fa 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt

@@ -521,7 +521,7 @@ core.editor::
 
 core.commentChar::
 	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 ASCII character
 	commented, and removes them after the editor returns
 	(default '#').
 +

-- 
2.44.0.64.g52b67adbeb2