freshlybakedca.ke / git
Git fork
fork atom

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>

authored by

Phillip Wood and committed by
Junio C Hamano fdae4114 084681b1

+40 -7
unified split
+5
Documentation/BreakingChanges.adoc
··· 239 + 240 The command will be removed. 241 242 == Superseded features that will not be deprecated 243 244 Some features have gained newer replacements that aim to improve the design in
··· 239 + 240 The command will be removed. 241 242 + * Support for `core.commentString=auto` has been deprecated and will 243 + be removed in Git 3.0. 244 + + 245 + cf. <xmqqa59i45wc.fsf@gitster.g> 246 + 247 == Superseded features that will not be deprecated 248 249 Some features have gained newer replacements that aim to improve the design in
+17 -1
Documentation/config/core.adoc
··· 531 commented, and removes them after the editor returns 532 (default '#'). 533 + 534 - If set to "auto", `git-commit` would select a character that is not 535 the beginning character of any line in existing commit messages. 536 + 537 Note that these two variables are aliases of each other, and in modern 538 versions of Git you are free to use a string (e.g., `//` or `⁑⁕⁑`) with 539 `commentChar`. Versions of Git prior to v2.45.0 will ignore
··· 531 commented, and removes them after the editor returns 532 (default '#'). 533 + 534 + ifndef::with-breaking-changes[] 535 + If set to "auto", `git-commit` will select a character that is not 536 the beginning character of any line in existing commit messages. 537 + Support for this value is deprecated and will be removed in Git 3.0 538 + due to the following limitations: 539 + 540 + -- 541 + * It is incompatible with adding comments in a commit message 542 + template. This includes the conflicts comments added to 543 + the commit message by `cherry-pick`, `merge`, `rebase` and 544 + `revert`. 545 + * It is incompatible with adding comments to the commit message 546 + in the `prepare-commit-msg` hook. 547 + * It is incompatible with the `fixup` and `squash` commands when 548 + rebasing, 549 + * It is not respected by `git notes` 550 + -- 551 + + 552 + endif::with-breaking-changes[] 553 Note that these two variables are aliases of each other, and in modern 554 versions of Git you are free to use a string (e.g., `//` or `⁑⁕⁑`) with 555 `commentChar`. Versions of Git prior to v2.45.0 will ignore
+4
builtin/commit.c
··· 683 return author_message || force_date; 684 } 685 686 static void adjust_comment_line_char(const struct strbuf *sb) 687 { 688 char candidates[] = "#;@!$%^&|:"; ··· 720 free(comment_line_str_to_free); 721 comment_line_str = comment_line_str_to_free = xstrfmt("%c", *p); 722 } 723 724 static void prepare_amend_commit(struct commit *commit, struct strbuf *sb, 725 struct pretty_print_context *ctx) ··· 916 if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len) 917 die_errno(_("could not write commit template")); 918 919 if (auto_comment_line_char) 920 adjust_comment_line_char(&sb); 921 strbuf_release(&sb); 922 923 /* This checks if committer ident is explicitly given */
··· 683 return author_message || force_date; 684 } 685 686 + #ifndef WITH_BREAKING_CHANGES 687 static void adjust_comment_line_char(const struct strbuf *sb) 688 { 689 char candidates[] = "#;@!$%^&|:"; ··· 721 free(comment_line_str_to_free); 722 comment_line_str = comment_line_str_to_free = xstrfmt("%c", *p); 723 } 724 + #endif /* !WITH_BREAKING_CHANGES */ 725 726 static void prepare_amend_commit(struct commit *commit, struct strbuf *sb, 727 struct pretty_print_context *ctx) ··· 918 if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len) 919 die_errno(_("could not write commit template")); 920 921 + #ifndef WITH_BREAKING_CHANGES 922 if (auto_comment_line_char) 923 adjust_comment_line_char(&sb); 924 + #endif /* !WITH_BREAKING_CHANGES */ 925 strbuf_release(&sb); 926 927 /* This checks if committer ident is explicitly given */
+8 -2
environment.c
··· 122 */ 123 const char *comment_line_str = "#"; 124 char *comment_line_str_to_free; 125 int auto_comment_line_char; 126 127 /* This is set by setup_git_directory_gently() and/or git_default_config() */ 128 char *git_work_tree_cfg; ··· 459 460 if (!strcmp(var, "core.commentchar") || 461 !strcmp(var, "core.commentstring")) { 462 - if (!value) 463 return config_error_nonbool(var); 464 - else if (!strcasecmp(value, "auto")) { 465 auto_comment_line_char = 1; 466 FREE_AND_NULL(comment_line_str_to_free); 467 comment_line_str = "#"; 468 } else if (value[0]) { 469 if (strchr(value, '\n')) 470 return error(_("%s cannot contain newline"), var); 471 comment_line_str = value; 472 FREE_AND_NULL(comment_line_str_to_free); 473 auto_comment_line_char = 0; 474 } else 475 return error(_("%s must have at least one character"), var); 476 return 0;
··· 122 */ 123 const char *comment_line_str = "#"; 124 char *comment_line_str_to_free; 125 + #ifndef WITH_BREAKING_CHANGES 126 int auto_comment_line_char; 127 + #endif /* !WITH_BREAKING_CHANGES */ 128 129 /* This is set by setup_git_directory_gently() and/or git_default_config() */ 130 char *git_work_tree_cfg; ··· 461 462 if (!strcmp(var, "core.commentchar") || 463 !strcmp(var, "core.commentstring")) { 464 + if (!value) { 465 return config_error_nonbool(var); 466 + #ifndef WITH_BREAKING_CHANGES 467 + } else if (!strcasecmp(value, "auto")) { 468 auto_comment_line_char = 1; 469 FREE_AND_NULL(comment_line_str_to_free); 470 comment_line_str = "#"; 471 + #endif /* !WITH_BREAKING_CHANGES */ 472 } else if (value[0]) { 473 if (strchr(value, '\n')) 474 return error(_("%s cannot contain newline"), var); 475 comment_line_str = value; 476 FREE_AND_NULL(comment_line_str_to_free); 477 + #ifndef WITH_BREAKING_CHANGES 478 auto_comment_line_char = 0; 479 + #endif /* !WITH_BREAKING_CHANGES */ 480 } else 481 return error(_("%s must have at least one character"), var); 482 return 0;
+2
environment.h
··· 208 */ 209 extern const char *comment_line_str; 210 extern char *comment_line_str_to_free; 211 extern int auto_comment_line_char; 212 213 # endif /* USE_THE_REPOSITORY_VARIABLE */ 214 #endif /* ENVIRONMENT_H */
··· 208 */ 209 extern const char *comment_line_str; 210 extern char *comment_line_str_to_free; 211 + #ifndef WITH_BREAKING_CHANGES 212 extern int auto_comment_line_char; 213 + #endif /* !WITH_BREAKING_CHANGES */ 214 215 # endif /* USE_THE_REPOSITORY_VARIABLE */ 216 #endif /* ENVIRONMENT_H */
+1 -1
t/t3404-rebase-interactive.sh
··· 1176 test B = $(git cat-file commit HEAD^ | sed -ne \$p) 1177 ' 1178 1179 - test_expect_success 'rebase -i respects core.commentchar=auto' ' 1180 test_config core.commentchar auto && 1181 write_script copy-edit-script.sh <<-\EOF && 1182 cp "$1" edit-script
··· 1176 test B = $(git cat-file commit HEAD^ | sed -ne \$p) 1177 ' 1178 1179 + test_expect_success !WITH_BREAKING_CHANGES 'rebase -i respects core.commentchar=auto' ' 1180 test_config core.commentchar auto && 1181 write_script copy-edit-script.sh <<-\EOF && 1182 cp "$1" edit-script
+1 -1
t/t3418-rebase-continue.sh
··· 328 test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec 329 ' 330 331 - test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' ' 332 git checkout -b branch-a && 333 test_commit A F1 && 334 git checkout -b branch-b HEAD^ &&
··· 328 test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec 329 ' 330 331 + test_expect_success !WITH_BREAKING_CHANGES 'no change in comment character due to conflicts markers with core.commentChar=auto' ' 332 git checkout -b branch-a && 333 test_commit A F1 && 334 git checkout -b branch-b HEAD^ &&
+2 -2
t/t7502-commit-porcelain.sh
··· 956 test_grep "^; Changes to be committed:" .git/COMMIT_EDITMSG 957 ' 958 959 - test_expect_success 'switch core.commentchar' ' 960 test_commit "#foo" foo && 961 GIT_EDITOR=.git/FAKE_EDITOR git -c core.commentChar=auto commit --amend && 962 test_grep "^; Changes to be committed:" .git/COMMIT_EDITMSG 963 ' 964 965 - test_expect_success 'switch core.commentchar but out of options' ' 966 cat >text <<\EOF && 967 # 1 968 ; 2
··· 956 test_grep "^; Changes to be committed:" .git/COMMIT_EDITMSG 957 ' 958 959 + test_expect_success !WITH_BREAKING_CHANGES 'switch core.commentchar' ' 960 test_commit "#foo" foo && 961 GIT_EDITOR=.git/FAKE_EDITOR git -c core.commentChar=auto commit --amend && 962 test_grep "^; Changes to be committed:" .git/COMMIT_EDITMSG 963 ' 964 965 + test_expect_success !WITH_BREAKING_CHANGES 'switch core.commentchar but out of options' ' 966 cat >text <<\EOF && 967 # 1 968 ; 2