From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got diff chomps trailing \r
To: Christian Weisgerber <naddy@mips.inka.de>
Cc: gameoftrees@openbsd.org
Date: Thu, 11 May 2023 19:28:15 +0200

On Thu, May 11, 2023 at 04:54:39PM +0200, Christian Weisgerber wrote:
> Stefan Sperling:
> 
> > As a first step it would be nice to have is a regression test.
> 
> There already is test_diff_crlf(), but it confirms that the trailing
> carriage return is omitted.
> 
> Here's a tweak to make it actually test the desired behavior.
> Yes, the sed 'l' command is in POSIX.

Thanks! Below patch makes your test pass. ok? 

I would first commit the fix to diff.git (where tests are still passing
with this patch in place), then sync it over along with your test change.

diff /home/stsp/src/got
commit - bfcdc9e9816dc81c90ce2db28ad9b602ec06b95a
path + /home/stsp/src/got
blob - 8b2477047aa2fbba35eb20ce030a857b1d0c4664
file + lib/diff_output.c
--- lib/diff_output.c
+++ lib/diff_output.c
@@ -74,13 +74,13 @@ diff_output_lines(struct diff_output_info *outinfo, FI
 
 	foreach_diff_atom(atom, start_atom, count) {
 		off_t outlen = 0;
-		int i, ch, nbuf = 0;
+		int i, ch, nbuf = 0, crlf = 0;
 		unsigned int len = atom->len;
-		unsigned char buf[DIFF_OUTPUT_BUF_SIZE + 1 /* '\n' */];
+		unsigned char buf[DIFF_OUTPUT_BUF_SIZE + 2 /* '\r\n' */];
 		size_t n;
 
 		n = strlcpy(buf, prefix, sizeof(buf));
-		if (n >= DIFF_OUTPUT_BUF_SIZE) /* leave room for '\n' */
+		if (n >= DIFF_OUTPUT_BUF_SIZE) /* leave room for '\r\n' */
 			return ENOBUFS;
 		nbuf += n;
 
@@ -94,8 +94,10 @@ diff_output_lines(struct diff_output_info *outinfo, FI
 				rc = get_atom_byte(&ch, atom, len - 1);
 				if (rc)
 					return rc;
-				if (ch == '\r')
+				if (ch == '\r') {
 					len--;
+					crlf = 1;
+				}
 			}
 		}
 
@@ -112,6 +114,8 @@ diff_output_lines(struct diff_output_info *outinfo, FI
 			}
 			buf[nbuf++] = ch;
 		}
+		if (crlf)
+			buf[nbuf++] = '\r';
 		buf[nbuf++] = '\n';
 		rc = fwrite(buf, 1, nbuf, dest);
 		if (rc != nbuf)
blob - 74c5c3277048c41e7d957d274e2fd708fbd7e022
file + regress/cmdline/diff.sh
--- regress/cmdline/diff.sh
+++ regress/cmdline/diff.sh
@@ -1308,7 +1308,7 @@ test_diff_crlf() {
 		return 1
 	fi
 
-	printf 'test\r\n' > $testroot/wt/crlf
+	printf 'one\r\ntwo\r\nthree\r\n' > $testroot/wt/crlf
 	(cd $testroot/wt && got add crlf && got commit -m +crlf) >/dev/null
 	ret=$?
 	if [ $ret -ne 0 ]; then
@@ -1316,14 +1316,16 @@ test_diff_crlf() {
 		return 1
 	fi
 
-	printf 'test 2\r\n' > $testroot/wt/crlf
-	(cd $testroot/wt && got diff | sed -n '/^---/,$p' > $testroot/stdout)
-	cat <<EOF > $testroot/stdout.expected
---- crlf
-+++ crlf
-@@ -1 +1 @@
--test
-+test 2
+	printf 'one\r\ntwain\r\nthree\r\n' > $testroot/wt/crlf
+	(cd $testroot/wt && got diff | sed -n '/^---/,$l' > $testroot/stdout)
+	cat <<\EOF > $testroot/stdout.expected
+--- crlf$
++++ crlf$
+@@ -1,3 +1,3 @@$
+ one\r$
+-two\r$
++twain\r$
+ three\r$
 EOF
 
 	cmp -s $testroot/stdout.expected $testroot/stdout