Re: [PATCH 1/2] breaking-changes: deprecate support for core.commentString=auto

>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> When "core.commentString" is set to "auto" then "git commit"
> will automatically select the comment character ensuring that it
> does 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 commit will add some advice for users on how they can update
> their config settings.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  Documentation/BreakingChanges.adoc |  4 ++++
>  Documentation/config/core.adoc     | 20 ++++++++++++++++++--
>  builtin/commit.c                   |  4 ++++
>  config.c                           |  4 ++++
>  environment.c                      |  2 ++
>  environment.h                      |  2 ++
>  t/t3404-rebase-interactive.sh      |  2 +-
>  t/t7502-commit-porcelain.sh        |  4 ++--
>  8 files changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc
> index 61bdd586b9e..f38ba1de6e4 100644
> --- a/Documentation/BreakingChanges.adoc
> +++ b/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. <xmqqa59i45wc.fsf@gitster.g>
>
>  == Superseded features that will not be deprecated
>
> diff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc
> index 9fde1ab63a7..7133f00c38b 100644
> --- 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 fba0dded64a..8794b24572b 100644
> --- 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[] = "#;@!$%^&|:";
> @@ -716,6 +717,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 +914,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/config.c b/config.c
> index eb60c293ab3..f99496b16c0 100644
> --- a/config.c
> +++ b/config.c
> @@ -1537,14 +1537,18 @@ static int git_default_core_config(const char *var, const char *value,
>             !strcmp(var, "core.commentstring")) {
>                 if (!value)
>                         return config_error_nonbool(var);
> +#ifndef WITH_BREAKING_CHANGES
>                 else if (!strcasecmp(value, "auto"))
>                         auto_comment_line_char = 1;
> +#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.c b/environment.c
> index 7bf0390a335..6804380889f 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -111,7 +111,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;
> diff --git a/environment.h b/environment.h
> index 9a3d05d414a..871596afcef 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -207,7 +207,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 6bac217ed35..ce0aebb9a7e 100755
> --- 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/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
> index b37e2018a74..65b4519a715 100755
> --- 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
> --
> 2.49.0.897.gfad3eb7d210
>