commit: 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 does not
match the start of any of the lines in the commit message. This was
introduced by commit 84c9dc2 (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 including the
   conflict comments added by "git commands" etc.

 - prepare-commit-msg hooks that introduce comments.

 - the 'fixup' and 'squash' commands of "git rebase -i".

 - when creating a commit with
   "git commit --cleanup=strip -F <file>"

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

The issues with comments comming from a template or a hook and the
failure to strip comments from a file are a consequence of the design
of this feature and are therefore hard to fix. The other issues could
in principle be fixed but it would complicate the implementation.

As the costs of this feature seem to outweigh the befits 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.

Signed-off-by: Phillip Wood <[email protected]>

Author: phillipwood

Documentation/BreakingChanges.adoc

@@ -183,6 +183,10 @@ These features will be removed.
   timeframe, in preference to its synonym "--annotate-stdin".  Git 3.0
   removes the support for "--stdin" altogether.
 
+* Support for `core.commentString=auto` has been deprecated and will
+  be removed in Git 3.0.
++
+cf. <[email protected]>
 
 == Superseded features that will not be deprecated
 

Documentation/config/advice.adoc

@@ -38,6 +38,9 @@ all advice messages.
 		configuration variable for how to set a given remote
 		to be used by default in some situations where this
 		advice would be printed.
+	commitAutoCommentChar::
+		Shown when `core.commentString` is set to the deprecated
+		value `auto`.
 	commitBeforeMerge::
 		Shown when linkgit:git-merge[1] refuses to
 		merge to avoid overwriting local changes.

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

advice.c

@@ -50,6 +50,7 @@ static struct {
 	[ADVICE_AMBIGUOUS_FETCH_REFSPEC]		= { "ambiguousFetchRefspec" },
 	[ADVICE_AM_WORK_DIR] 				= { "amWorkDir" },
 	[ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME] 	= { "checkoutAmbiguousRemoteBranchName" },
+	[ADVICE_COMMIT_AUTO_COMMENT_CHAR]               = { "commitAutoCommentChar" },
 	[ADVICE_COMMIT_BEFORE_MERGE]			= { "commitBeforeMerge" },
 	[ADVICE_DEFAULT_BRANCH_NAME]			= { "defaultBranchName" },
 	[ADVICE_DETACHED_HEAD]				= { "detachedHead" },

advice.h

@@ -17,6 +17,7 @@ enum advice_type {
 	ADVICE_AMBIGUOUS_FETCH_REFSPEC,
 	ADVICE_AM_WORK_DIR,
 	ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME,
+	ADVICE_COMMIT_AUTO_COMMENT_CHAR,
 	ADVICE_COMMIT_BEFORE_MERGE,
 	ADVICE_DEFAULT_BRANCH_NAME,
 	ADVICE_DETACHED_HEAD,

builtin/commit.c

@@ -683,12 +683,17 @@ 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[] = "#;@!$%^&|:";
 	char *candidate;
 	const char *p;
 
+	advise_if_enabled(ADVICE_COMMIT_AUTO_COMMENT_CHAR,
+		_("Setting 'core.commentString=auto' is deprecated and "
+		  "will be removed in git 3.0"));
+
 	if (!memchr(sb->buf, candidates[0], sb->len)) {
 		free(comment_line_str_to_free);
 		comment_line_str = comment_line_str_to_free =
@@ -716,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)
@@ -912,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 */

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

t/t7502-commit-porcelain.sh

@@ -956,13 +956,15 @@ 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 &&
+	GIT_EDITOR=.git/FAKE_EDITOR git -c core.commentChar=auto commit --amend 2>actual &&
+	test_grep "${SQ}core.commentString=auto${SQ} is deprecated" actual &&
+	test_grep advice.commitAutoCommentChar actual &&
 	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