Hi Matheus
On 07/09/2022 15:44, Matheus Tavares wrote:
When applying a patch, `git am` looks for special delimiter strings
(such as "---") to know where the message ends and the actual diff
starts. If one of these strings appears in the commit message itself,
`am` might get confused and fail to apply the patch properly. This has
already caused inconveniences in the past [1][2]. To help avoid such
problem, let's make `git format-patch` warn on commit messages
containing one of the said strings.
Thanks for working on this, having a warning for this is a useful
addition. If the user embeds a diff in their commit message then they
will receive three warnings
warning: commit message has a patch delimiter: 'diff --git a/file b/file'
warning: commit message has a patch delimiter: '--- file'
warning: git am might fail to apply this patch. Consider indenting the
offending lines.
I guess it's helpful to show all the lines that are considered
delimiters but it gets quite noisy.
diff --git a/pretty.c b/pretty.c
index 6d819103fb..913d974b3a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -2107,6 +2108,14 @@ void pp_remainder(struct pretty_print_context *pp,
if (!linelen)
break;
+ if (pp->check_in_body_patch_breaks &&
+ (patchbreak(line, linelen) || is_scissors_line(line, linelen))) {
+ warning(_("commit message has a patch delimiter: '%.*s'"),
+ line[linelen - 1] == '\n' ? linelen - 1 : linelen,
+ line);
+ found_delimiter = 1;
+ }
+
if (is_blank_line(line, &linelen)) {
if (first)
continue;
@@ -2133,6 +2142,11 @@ void pp_remainder(struct pretty_print_context *pp,
}
strbuf_addch(sb, '\n');
}
+
+ if (found_delimiter) {
+ warning(_("git am might fail to apply this patch. "
+ "Consider indenting the offending lines."));
The message says the patch might fail to apply, but isn't it guaranteed
to fail?
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index fbec8ad2ef..4bbf1156e9 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -2329,4 +2329,30 @@ test_expect_success 'interdiff: solo-patch' '
test_cmp expect actual
'
+test_expect_success 'warn if commit message contains patch delimiter' '
+ >delim &&
+ git add delim &&
+ cat >msg <<-\EOF &&
+ title
+
+ ---
+ EOF
+ git commit -F msg &&
+ git format-patch -1 2>stderr &&
+ grep "warning: commit message has a patch delimiter" stderr
I think it would be worth checking for the second message as well in the
tests.
Best Wishes
Phillip
