config: allow multi-byte core.commentChar

Now that all of the code handles multi-byte comment characters, it's safe to allow users to set them. There is one special case I kept: we still will not allow an empty string for the commentChar. While it might make sense in some contexts (e.g., output where you don't want any comment prefix), there are plenty where it will behave badly (e.g., all of our starts_with() checks will indicate that every line is a comment!). It might be reasonable to assign some meaningful semantics, but it would probably involve checking how each site behaves. In the interim let's forbid it and we can loosen things later. Likewise, the "commentChar cannot be a newline" rule is now extended to "it cannot contain a newline" (for the same reason: it can confuse our parsing loops). Since comment_line_str is used in many parts of the code, it's hard to cover all possibilities with tests. We can convert the existing double-semicolon prefix test to show that "git status" works. And we'll give it a more challenging case in t7507, where we confirm that git-commit strips out the commit template along with any --verbose text when reading the edited commit message back in. That covers the basics, though it's possible there could be issues in more exotic spots (e.g., the sequencer todo list uses its own code). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Mar 12, 2024 at 05:17 UTC 8b311478ad16b2fe9d2f5b5febec9f5e8f7fd52d
5 files changed +27 -8
Documentation/config/core.txt
+3 -1
@@ -523,7 +523,9 @@ core.commentChar::
523 Commands such as `commit` and `tag` that let you edit
524 messages consider a line that begins with this character
525 commented, and removes them after the editor returns
526 - (default '#').
526 + (default '#'). Note that this option can take values larger than
527 + a byte (whether a single multi-byte character, or you
528 + could even go wild with a multi-character sequence).
529 +
530 If set to "auto", `git-commit` would select a character that is not
531 the beginning character of any line in existing commit messages.
config.c
+5 -5
@@ -1565,13 +1565,13 @@ static int git_default_core_config(const char *var, const char *value,
1565 return config_error_nonbool(var);
1566 else if (!strcasecmp(value, "auto"))
1567 auto_comment_line_char = 1;
1568 - else if (value[0] && !value[1]) {
1569 - if (value[0] == '\n')
1570 - return error(_("core.commentChar cannot be newline"));
1571 - comment_line_str = xstrfmt("%c", value[0]);
1568 + else if (value[0]) {
1569 + if (strchr(value, '\n'))
1570 + return error(_("core.commentChar cannot contain newline"));
1571 + comment_line_str = xstrdup(value);
1572 auto_comment_line_char = 0;
1573 } else
1574 - return error(_("core.commentChar should only be one ASCII character"));
1574 + return error(_("core.commentChar must have at least one character"));
1575 return 0;
1576 }
1577
t/t0030-stripspace.sh
+6 -1
@@ -403,7 +403,12 @@ test_expect_success 'strip comments with changed comment char' '
403
404 test_expect_success 'newline as commentchar is forbidden' '
405 test_must_fail git -c core.commentChar="$LF" stripspace -s 2>err &&
406 - grep "core.commentChar cannot be newline" err
406 + grep "core.commentChar cannot contain newline" err
407 +'
408 +
409 +test_expect_success 'empty commentchar is forbidden' '
410 + test_must_fail git -c core.commentchar= stripspace -s 2>err &&
411 + grep "core.commentChar must have at least one character" err
412 '
413
414 test_expect_success '-c with single line' '
t/t7507-commit-verbose.sh
+10
@@ -101,6 +101,16 @@ test_expect_success 'verbose diff is stripped out with set core.commentChar' '
101 test_grep "Aborting commit due to empty commit message." err
102 '
103
104 +test_expect_success 'verbose diff is stripped with multi-byte comment char' '
105 + (
106 + GIT_EDITOR=cat &&
107 + export GIT_EDITOR &&
108 + test_must_fail git -c core.commentchar="foo>" commit -a -v >out 2>err
109 + ) &&
110 + grep "^foo> " out &&
111 + test_grep "Aborting commit due to empty commit message." err
112 +'
113 +
114 test_expect_success 'status does not verbose without --verbose' '
115 git status >actual &&
116 ! grep "^diff --git" actual
t/t7508-status.sh
+3 -1
@@ -1403,7 +1403,9 @@ test_expect_success "status (core.commentchar with submodule summary)" '
1403
1404 test_expect_success "status (core.commentchar with two chars with submodule summary)" '
1405 test_config core.commentchar ";;" &&
1406 - test_must_fail git -c status.displayCommentPrefix=true status
1406 + sed "s/^/;/" <expect >expect.double &&
1407 + git -c status.displayCommentPrefix=true status >output &&
1408 + test_cmp expect.double output
1409 '
1410
1411 test_expect_success "--ignore-submodules=all suppresses submodule summary" '