breaking-changes: deprecate support for core.commentString=auto

When "core.commentString" is set to "auto" then "git commit" will
automatically select the comment character ensuring that it is not the
first character on any of the lines in the commit message. This was
introduced by commit 84c9dc2c5a2 (commit: allow core.commentChar=auto
for character auto selection, 2014-05-17). The motivation seems to be
to avoid commenting out lines from the existing message when amending
a commit that was created with a message from a file.

Unfortunately this feature does not work with:

 * commit message templates that contain comments.

 * prepare-commit-msg hooks that introduce comments.

 * "git commit --cleanup=strip --edit -F <file>" which means that it
   is incompatible with

   - the "fixup" and "squash" commands of "git rebase -i" as the
     comments added by those commands are then treated as part of
     the commit message.

   - the conflict comments added to the commit message by "git
     cherry-pick", "git rebase" etc. as these comments are then
     treated as part of the commit message.

It is also ignored by "git notes" when amending a note.

The issues with comments coming from a template, hook or file are a
consequence of the design of this feature and are therefore hard to
fix.

As the costs of this feature outweigh the benefits, deprecate it and
remove it in Git 3.0. If someone comes up with some patches that fix
all the issues in a maintainable way then I'd be happy to see this
change reverted.

The next commits will add a warning and some advice for users on how
they can update their config settings.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc
index f8d2eba061c82a6db04f3d2ceeb2489b53597e83..344ce5006031ce0056b75adf721be59ec2f81179 100644 (file)
--- a/Documentation/BreakingChanges.adoc
+++ b/Documentation/BreakingChanges.adoc
@@ -239,6 +239,11 @@ These features will be removed.
 +
 The command will be removed.
 
+* Support for `core.commentString=auto` has been deprecated and will
+  be removed in Git 3.0.
++
+cf. <xmqqa59i45wc.fsf@gitster.g>
+
 == Superseded features that will not be deprecated
 
 Some features have gained newer replacements that aim to improve the design in

diff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc
index 9fde1ab63a70ea3b595434e54f7e52e0ae181995..7133f00c38bdfa44a66c58248926143fb4b3c0f3 100644 (file)
--- a/Documentation/config/core.adoc
+++ b/Documentation/config/core.adoc
@@ -531,9 +531,25 @@ core.commentString::
        commented, and removes them after the editor returns
        (default '#').
 +
-If set to "auto", `git-commit` would select a character that is not
+ifndef::with-breaking-changes[]
+If set to "auto", `git-commit` will select a character that is not
 the beginning character of any line in existing commit messages.
-+
+Support for this value is deprecated and will be removed in Git 3.0
+due to the following limitations:
++
+--
+* It is incompatible with adding comments in a commit message
+  template. This includes the conflicts comments added to
+  the commit message by `cherry-pick`, `merge`, `rebase` and
+  `revert`.
+* It is incompatible with adding comments to the commit message
+  in the `prepare-commit-msg` hook.
+* It is incompatible with the `fixup` and `squash` commands when
+  rebasing,
+* It is not respected by `git notes`
+--
++
+endif::with-breaking-changes[]
 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
 `commentChar`. Versions of Git prior to v2.45.0 will ignore

diff --git a/builtin/commit.c b/builtin/commit.c
index 757f51eac820a9957f8e215bc49b769703ab6fc1..d25cc07a355aaa35d0192daab129a7c27ac0d2ba 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -683,6 +683,7 @@ static int author_date_is_interesting(void)
        return author_message || force_date;
 }
 
+#ifndef WITH_BREAKING_CHANGES
 static void adjust_comment_line_char(const struct strbuf *sb)
 {
        char candidates[] = "#;@!$%^&|:";
@@ -720,6 +721,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)
        free(comment_line_str_to_free);
        comment_line_str = comment_line_str_to_free = xstrfmt("%c", *p);
 }
+#endif /* !WITH_BREAKING_CHANGES */
 
 static void prepare_amend_commit(struct commit *commit, struct strbuf *sb,
                                struct pretty_print_context *ctx)
@@ -916,8 +918,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
        if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
                die_errno(_("could not write commit template"));
 
+#ifndef WITH_BREAKING_CHANGES
        if (auto_comment_line_char)
                adjust_comment_line_char(&sb);
+#endif /* !WITH_BREAKING_CHANGES */
        strbuf_release(&sb);
 
        /* This checks if committer ident is explicitly given */

diff --git a/environment.c b/environment.c
index a0ac5934b37b30ebfeca2538ab6fc38f7e7f958e..4c87876d483143221c001b9b4da42d3e8a3986b0 100644 (file)
--- a/environment.c
+++ b/environment.c
@@ -122,7 +122,9 @@ int protect_ntfs = PROTECT_NTFS_DEFAULT;
  */
 const char *comment_line_str = "#";
 char *comment_line_str_to_free;
+#ifndef WITH_BREAKING_CHANGES
 int auto_comment_line_char;
+#endif /* !WITH_BREAKING_CHANGES */
 
 /* This is set by setup_git_directory_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
@@ -459,18 +461,22 @@ static int git_default_core_config(const char *var, const char *value,
 
        if (!strcmp(var, "core.commentchar") ||
            !strcmp(var, "core.commentstring")) {
-               if (!value)
+               if (!value) {
                        return config_error_nonbool(var);
-               else if (!strcasecmp(value, "auto")) {
+#ifndef WITH_BREAKING_CHANGES
+               } else if (!strcasecmp(value, "auto")) {
                        auto_comment_line_char = 1;
                        FREE_AND_NULL(comment_line_str_to_free);
                        comment_line_str = "#";
+#endif /* !WITH_BREAKING_CHANGES */
                } else if (value[0]) {
                        if (strchr(value, '\n'))
                                return error(_("%s cannot contain newline"), var);
                        comment_line_str = value;
                        FREE_AND_NULL(comment_line_str_to_free);
+#ifndef WITH_BREAKING_CHANGES
                        auto_comment_line_char = 0;
+#endif /* !WITH_BREAKING_CHANGES */
                } else
                        return error(_("%s must have at least one character"), var);
                return 0;

diff --git a/environment.h b/environment.h
index 8cfce41015b3c8ec32686fde91224b62ab3853cb..e75c4abb388670e951aff1fb617f77202c8b57d0 100644 (file)
--- a/environment.h
+++ b/environment.h
@@ -208,7 +208,9 @@ extern char *excludes_file;
  */
 extern const char *comment_line_str;
 extern char *comment_line_str_to_free;
+#ifndef WITH_BREAKING_CHANGES
 extern int auto_comment_line_char;
+#endif /* !WITH_BREAKING_CHANGES */
 
 # endif /* USE_THE_REPOSITORY_VARIABLE */
 #endif /* ENVIRONMENT_H */

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 6bac217ed3555e73edad9c3f9612608b0bfd15b4..ce0aebb9a7ec7d6b207c33909f0b6c2dfd1f20c1 100755 (executable)
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1176,7 +1176,7 @@ test_expect_success 'rebase -i respects core.commentchar' '
        test B = $(git cat-file commit HEAD^ | sed -ne \$p)
 '
 
-test_expect_success 'rebase -i respects core.commentchar=auto' '
+test_expect_success !WITH_BREAKING_CHANGES 'rebase -i respects core.commentchar=auto' '
        test_config core.commentchar auto &&
        write_script copy-edit-script.sh <<-\EOF &&
        cp "$1" edit-script

diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index b8a8dd77e74408bda02f2f63f8329222823819f0..f9b8999db50f1b77f03ec51cec01ace8849a9137 100755 (executable)
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -328,7 +328,7 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
        test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
 '
 
-test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
+test_expect_success !WITH_BREAKING_CHANGES 'no change in comment character due to conflicts markers with core.commentChar=auto' '
        git checkout -b branch-a &&
        test_commit A F1 &&
        git checkout -b branch-b HEAD^ &&

diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index b37e2018a74a7b28725fa277a95cca516daf5cbe..65b4519a715094fea989c9708e0932eac820fd4b 100755 (executable)
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh
@@ -956,13 +956,13 @@ test_expect_success 'commit --status with custom comment character' '
        test_grep "^; Changes to be committed:" .git/COMMIT_EDITMSG
 '
 
-test_expect_success 'switch core.commentchar' '
+test_expect_success !WITH_BREAKING_CHANGES 'switch core.commentchar' '
        test_commit "#foo" foo &&
        GIT_EDITOR=.git/FAKE_EDITOR git -c core.commentChar=auto commit --amend &&
        test_grep "^; Changes to be committed:" .git/COMMIT_EDITMSG
 '
 
-test_expect_success 'switch core.commentchar but out of options' '
+test_expect_success !WITH_BREAKING_CHANGES 'switch core.commentchar but out of options' '
        cat >text <<\EOF &&
 # 1
 ; 2