Re: Verbose Commit Ignore Line Fails via CRLF Line Endings

[Junio C Hamano <gitster@pobox.com>]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

>> Is there any chance for git to support a CRLF magic ignore line,
>> particularly considering the variation in standard line ending across
>> different platforms? I tried autocrlf=input as well and it sadly
>> doesn't normalize the commit message file itself. Either way (magic
>> ignore with CRLF or normalizing line endings in the commit message),
>> would be appreciated for mixed line ending workflows (especially
>> considering WSL)
>
> The answer is essentially that I don't know.  We typically make
> decisions on whether we'll accept features when we see the patch.  My
> guess is that, assuming someone (maybe you) sends a patch, it will
> probably be accepted, since I wouldn't expect it would be very difficult
> to do or have major impacts on the code.  It might, as with any patch,
> take a couple of rounds, though.
>
> I use Linux or rarely other Unix systems and always use LF endings, so I
> don't plan to send a patch since this doesn't affect me, but assuming
> the patch looked reasonable, I don't see myself having an objection to
> it.

I do not offhand see a downside if we loosened the detection of the
cut line a bit.  What is happening on the Git end (I do not care how
Editor screws up and munges what we wrote before "Do not modify or
remove the line above"---to us, it looks like the user did not honor
that instruction) is:

wt-status.c defines cut_line[] to be

        static const char cut_line[] =
        "------------------------ >8 ------------------------\n";

IOW, we insist that after the dashes we have LF.

Then wt-status.c:wt_status_locate_end() does this:

        size_t wt_status_locate_end(const char *s, size_t len)
        {
                const char *p;
                struct strbuf pattern = STRBUF_INIT;

                strbuf_addf(&pattern, "\n%s %s", comment_line_str, cut_line);
                if (starts_with(s, pattern.buf + 1))
                        len = 0;
                else if ((p = strstr(s, pattern.buf))) {
			size_t newlen = p - s + 1;
			...

That is, if the buffer begins with the cut line (including the LF at
the end of the line, without any CR before it), we found it at the
beginning of the buffer at len==0, otherwise if we find a region of
memory that begins with LF, followed by the scissors, followed by LF
(again, without allowing CR before it), we have a match there.

That is reasonable as long as the user does not muck with the line
that we told them not to touch.  In this case, the editor is
corrupting the line without being instructed by the user so the user
cannot do much about it.

We could loosen the rule by doing something along the following line:

 - define the cut_line constant without LF at the end of the line.

 - teach wt_status_append_cut_line() to compensate for the lack of
   LF in cut_line.

 - teach wt_status_locate_end() to notice and ignore CR if it
   appears at the end of the cut_line.

I'll go offline at the end of this week, so I will stop here at
illustrating the above approach in the form of an untested patch
below.  I am sure I'd have many off by one errors in it, but it
should be sufficient to outline the idea.

 wt-status.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git c/wt-status.c w/wt-status.c
index 6a8c05d1cf..b54bc5de81 100644
--- c/wt-status.c
+++ w/wt-status.c
@@ -39,7 +39,7 @@
 #define UF_DELAY_WARNING_IN_MS (2 * 1000)
 
 static const char cut_line[] =
-"------------------------ >8 ------------------------\n";
+"------------------------ >8 ------------------------";
 
 static char default_wt_status_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
@@ -1095,15 +1095,28 @@ static void wt_longstatus_print_other(struct wt_status *s,
 	status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 }
 
+static int at_eol(const char *s, size_t len, size_t loc)
+{
+	if (len <= loc)
+		return 0;
+	if (s[loc] == '\r')
+		loc++;
+	if (len <= loc)
+		return 0;
+	return s[loc] == '\n';
+}
+
 size_t wt_status_locate_end(const char *s, size_t len)
 {
 	const char *p;
 	struct strbuf pattern = STRBUF_INIT;
 
-	strbuf_addf(&pattern, "\n%s %s", comment_line_str, cut_line);
-	if (starts_with(s, pattern.buf + 1))
+	strbuf_addf(&pattern, "%s %s", comment_line_str, cut_line);
+	if (starts_with(s, pattern.buf) && at_eol(s, len, pattern.len))
 		len = 0;
-	else if ((p = strstr(s, pattern.buf))) {
+	else if ((p = strstr(s, pattern.buf)) &&
+		 (s < p && p[-1] == '\n') &&
+		 at_eol(p, p - s, pattern.len)) {
 		size_t newlen = p - s + 1;
 		if (newlen < len)
 			len = newlen;
@@ -1116,7 +1129,7 @@ void wt_status_append_cut_line(struct strbuf *buf)
 {
 	const char *explanation = _("Do not modify or remove the line above.\nEverything below it will be ignored.");
 
-	strbuf_commented_addf(buf, comment_line_str, "%s", cut_line);
+	strbuf_commented_addf(buf, comment_line_str, "%s\n", cut_line);
 	strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_str);
 }