# git log -z still outputting newlines? 17 messages from 2012-04-30 to 2012-05-01. Participants: Randal L. Schwartz, Andreas Schwab, Jan Krüger, Thomas Rast, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/30369 ## Randal L. Schwartz, 2012-04-30 17:58 Subject: git log -z still outputting newlines? Message-ID: <86ty01qez7.fsf@red.stonehenge.com> URL: https://gitlist.dev/e/86ty01qez7.fsf%40red.stonehenge.com ``` $ git log -z --format='%cE' -5 | od -c 0000000 g i t s t e r @ p o b o x . c o 0000020 m \n g i t s t e r @ p o b o x . 0000040 c o m \n g i t s t e r @ p o b o 0000060 x . c o m \n g i t s t e r @ p o 0000100 b o x . c o m \n g i t s t e r @ 0000120 p o b o x . c o m \n 0000132 Why are all those newlines in there? Bug? Misfeature? Feature? If feature, how do I ensure \0 in my output? If I add %x00, I get both \0 *and* \n in output. :( -- Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095 Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc. See http://methodsandmessages.posterous.com/ for Smalltalk discussion ``` ## Andreas Schwab, 2012-04-30 18:34 Subject: Re: git log -z still outputting newlines? Message-ID: URL: https://gitlist.dev/e/m2pqaprrup.fsf%40igel.home In-Reply-To: <86ty01qez7.fsf@red.stonehenge.com> ``` merlyn@stonehenge.com (Randal L. Schwartz) writes: > $ git log -z --format='%cE' -5 | od -c > 0000000 g i t s t e r @ p o b o x . c o > 0000020 m \n g i t s t e r @ p o b o x . > 0000040 c o m \n g i t s t e r @ p o b o > 0000060 x . c o m \n g i t s t e r @ p o > 0000100 b o x . c o m \n g i t s t e r @ > 0000120 p o b o x . c o m \n > 0000132 > > Why are all those newlines in there? Bug? Misfeature? Feature? If > feature, how do I ensure \0 in my output? If I add %x00, I get both \0 > *and* \n in output. :( --format=format:%cE respects the -z option. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ``` ## Jan Krüger, 2012-04-30 18:45 Subject: [PATCH] log-tree: use custom line terminator in line termination mode Message-ID: <1335811555-23564-1-git-send-email-jk@jk.gs> URL: https://gitlist.dev/e/1335811555-23564-1-git-send-email-jk%40jk.gs In-Reply-To: <86ty01qez7.fsf@red.stonehenge.com> ``` When using a custom format in line termination mode (as opposed to line separation mode), the configured line terminator is not used, so things like "git log --pretty=tformat:%H -z" do not work properly. Make it use the line terminator the user ordered. Signed-off-by: Jan Krüger --- log-tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/log-tree.c b/log-tree.c index 34c49e7..44f0268 100644 --- a/log-tree.c +++ b/log-tree.c @@ -682,7 +682,7 @@ void show_log(struct rev_info *opt) if (opt->use_terminator) { if (!opt->missing_newline) graph_show_padding(opt->graph); - putchar('\n'); + putchar(opt->diffopt.line_termination); } strbuf_release(&msgbuf); -- 1.7.10 ``` ## Thomas Rast, 2012-04-30 19:02 Subject: Re: git log -z still outputting newlines? Message-ID: <87zk9toxes.fsf@thomas.inf.ethz.ch> URL: https://gitlist.dev/e/87zk9toxes.fsf%40thomas.inf.ethz.ch In-Reply-To: ``` Andreas Schwab writes: > merlyn@stonehenge.com (Randal L. Schwartz) writes: > >> $ git log -z --format='%cE' -5 | od -c >> 0000000 g i t s t e r @ p o b o x . c o >> 0000020 m \n g i t s t e r @ p o b o x . >> 0000040 c o m \n g i t s t e r @ p o b o >> 0000060 x . c o m \n g i t s t e r @ p o >> 0000100 b o x . c o m \n g i t s t e r @ >> 0000120 p o b o x . c o m \n >> 0000132 >> >> Why are all those newlines in there? Bug? Misfeature? Feature? If >> feature, how do I ensure \0 in my output? If I add %x00, I get both \0 >> *and* \n in output. :( > > --format=format:%cE respects the -z option. The underlying problem is apparently that --format=%cE triggers the format-guessing logic, which assumes you meant --pretty=tformat:%cE instead of --pretty=format:%cE. It's probably a bug that --pretty=tformat:%cE does not use \0 here. After all the manual states · tformat: The tformat: format works exactly like format:, except that it provides "terminator" semantics instead of "separator" semantics. Fixing it may be as easy as the patch below, but I haven't spent much time on it. diff --git i/log-tree.c w/log-tree.c index 34c49e7..44f0268 100644 --- i/log-tree.c +++ w/log-tree.c @@ -682,7 +682,7 @@ void show_log(struct rev_info *opt) if (opt->use_terminator) { if (!opt->missing_newline) graph_show_padding(opt->graph); - putchar('\n'); + putchar(opt->diffopt.line_termination); } strbuf_release(&msgbuf); -- Thomas Rast trast@{inf,student}.ethz.ch ``` ## Junio C Hamano, 2012-04-30 19:36 Subject: Re: [PATCH] log-tree: use custom line terminator in line termination mode Message-ID: <7vaa1tf1w8.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vaa1tf1w8.fsf%40alter.siamese.dyndns.org In-Reply-To: <1335811555-23564-1-git-send-email-jk@jk.gs> ``` Jan Krüger writes: > When using a custom format in line termination mode (as opposed to line > separation mode), the configured line terminator is not used, so things > like "git log --pretty=tformat:%H -z" do not work properly. > > Make it use the line terminator the user ordered. > > Signed-off-by: Jan Krüger > --- > log-tree.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/log-tree.c b/log-tree.c > index 34c49e7..44f0268 100644 > --- a/log-tree.c > +++ b/log-tree.c > @@ -682,7 +682,7 @@ void show_log(struct rev_info *opt) > if (opt->use_terminator) { > if (!opt->missing_newline) > graph_show_padding(opt->graph); > - putchar('\n'); > + putchar(opt->diffopt.line_termination); > } > > strbuf_release(&msgbuf); Looks sensible. Perhaps we would want to add a test? ``` ## Jan Krüger, 2012-04-30 20:28 Subject: [PATCH v2] log-tree: use custom line terminator in line termination mode Message-ID: <1335817705-24718-1-git-send-email-jk@jk.gs> URL: https://gitlist.dev/e/1335817705-24718-1-git-send-email-jk%40jk.gs In-Reply-To: <7vaa1tf1w8.fsf@alter.siamese.dyndns.org> ``` When using a custom format in line termination mode (as opposed to line separation mode), the configured line terminator is not used, so things like "git log --pretty=tformat:%H -z" do not work properly. Make it use the line terminator the user ordered. Signed-off-by: Jan Krüger --- Here are two simple tests, for both format: and tformat: with -z. log-tree.c | 2 +- t/t4205-log-pretty-formats.sh | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/log-tree.c b/log-tree.c index 34c49e7..44f0268 100644 --- a/log-tree.c +++ b/log-tree.c @@ -682,7 +682,7 @@ void show_log(struct rev_info *opt) if (opt->use_terminator) { if (!opt->missing_newline) graph_show_padding(opt->graph); - putchar('\n'); + putchar(opt->diffopt.line_termination); } strbuf_release(&msgbuf); diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 2ae9faa..03a73ba 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -71,4 +71,16 @@ test_expect_success 'alias loop' ' test_must_fail git log --pretty=test-foo ' +printf "add bar\0initial" > expected +test_expect_success 'NUL separation' ' + git log -z --pretty="format:%s" >actual && + test_cmp expected actual +' + +printf "add bar\0initial\0" > expected +test_expect_success 'NUL termination' ' + git log -z --pretty="tformat:%s" >actual && + test_cmp expected actual +' + test_done -- 1.7.10.406.g0017 ``` ## Junio C Hamano, 2012-04-30 22:58 Subject: Re: [PATCH v2] log-tree: use custom line terminator in line termination mode Message-ID: <7vd36oesjr.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd36oesjr.fsf%40alter.siamese.dyndns.org In-Reply-To: <1335817705-24718-1-git-send-email-jk@jk.gs> ``` Jan Krüger writes: > When using a custom format in line termination mode (as opposed to line > separation mode), the configured line terminator is not used, so things > like "git log --pretty=tformat:%H -z" do not work properly. > > Make it use the line terminator the user ordered. > > Signed-off-by: Jan Krüger > --- > Here are two simple tests, for both format: and tformat: with -z. Thanks for being thorough. Very much appreciated. Having said that, are we sure that printf "add bar\0initial" works per specification, or merely works by accident in some implementation? In C, we have to write this as printf("add bar%cinitial", 0), and the above makes my stomach feel a bit queasy. Admittedly we have "printf "\0\0" in t6024 and we haven't seen anybody complain for the past 6 years, so perhaps I shouldn't be worried too much about this. > diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh > index 2ae9faa..03a73ba 100755 > --- a/t/t4205-log-pretty-formats.sh > +++ b/t/t4205-log-pretty-formats.sh > @@ -71,4 +71,16 @@ test_expect_success 'alias loop' ' > test_must_fail git log --pretty=test-foo > ' > > +printf "add bar\0initial" > expected > +test_expect_success 'NUL separation' ' > + git log -z --pretty="format:%s" >actual && > + test_cmp expected actual > +' > + > +printf "add bar\0initial\0" > expected > +test_expect_success 'NUL termination' ' > + git log -z --pretty="tformat:%s" >actual && > + test_cmp expected actual > +' > + > test_done ``` ## Jan Krüger, 2012-04-30 23:28 Subject: Re: [PATCH v2] log-tree: use custom line terminator in line termination mode Message-ID: <4F9F201C.40509@jk.gs> URL: https://gitlist.dev/e/4F9F201C.40509%40jk.gs In-Reply-To: <7vd36oesjr.fsf@alter.siamese.dyndns.org> ``` On 05/01/2012 12:58 AM, Junio C Hamano wrote: > Having said that, are we sure that > > printf "add bar\0initial" > > works per specification, or merely works by accident in some > implementation? I don't know for certain. In these cases I go by what dash (Debian's sh implementation) does. To quote its manpage: "Only features designated by POSIX, plus a few Berkeley extensions, are being incorporated into this shell." It does support this syntax for printf. That's enough for me personally. Is it enough for everyone? I don't know. -Jan ``` ## Andreas Schwab, 2012-05-01 00:28 Subject: Re: [PATCH v2] log-tree: use custom line terminator in line termination mode Message-ID: URL: https://gitlist.dev/e/m2ehr4sq1q.fsf%40igel.home In-Reply-To: <7vd36oesjr.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: > Having said that, are we sure that > > printf "add bar\0initial" > > works per specification, or merely works by accident in some > implementation? Since the backslash is not followed by $ ` " \ it is not special to the shell. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ``` ## Junio C Hamano, 2012-05-01 00:43 Subject: Re: [PATCH v2] log-tree: use custom line terminator in line termination mode Message-ID: <7v62cgenn4.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v62cgenn4.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Andreas Schwab writes: > Junio C Hamano writes: > >> Having said that, are we sure that >> >> printf "add bar\0initial" >> >> works per specification, or merely works by accident in some >> implementation? > > Since the backslash is not followed by $ ` " \ it is not > special to the shell. Yeah, but I wasn't worried about what shell does in the first place. I was worried about what printf(1) does. ``` ## Randal L. Schwartz, 2012-05-01 02:51 Subject: Re: [PATCH v2] log-tree: use custom line terminator in line termination mode Message-ID: <86ehr4r4u0.fsf@red.stonehenge.com> URL: https://gitlist.dev/e/86ehr4r4u0.fsf%40red.stonehenge.com In-Reply-To: <7vd36oesjr.fsf@alter.siamese.dyndns.org> ``` >>>>> "Junio" == Junio C Hamano writes: Junio> Having said that, are we sure that Junio> printf "add bar\0initial" Junio> works per specification, or merely works by accident in some Junio> implementation? >From the POSIX spec (http://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html): In addition to the escape sequences shown in XBD File Format Notation ( '\\' , '\a' , '\b' , '\f' , '\n' , '\r' , '\t' , '\v' ), "\ddd" , where ddd is a one, two, or three-digit octal number, shall be written as a byte with the numeric value specified by the octal number. Looks pretty intentional to me. \0 is a nul. -- Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095 Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc. See http://methodsandmessages.posterous.com/ for Smalltalk discussion ``` ## Junio C Hamano, 2012-05-01 04:27 Subject: Re: [PATCH v2] log-tree: use custom line terminator in line termination mode Message-ID: <7vhaw0cyql.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vhaw0cyql.fsf%40alter.siamese.dyndns.org In-Reply-To: <86ehr4r4u0.fsf@red.stonehenge.com> ``` merlyn@stonehenge.com (Randal L. Schwartz) writes: >>>>>> "Junio" == Junio C Hamano writes: > > Junio> Having said that, are we sure that > > Junio> printf "add bar\0initial" > > > Junio> works per specification, or merely works by accident in some > Junio> implementation? > > From the POSIX spec > (http://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html): > > In addition to the escape sequences shown in XBD File Format Notation ( > '\\' , '\a' , '\b' , '\f' , '\n' , '\r' , '\t' , '\v' ), "\ddd" , where > ddd is a one, two, or three-digit octal number, shall be written as a > byte with the numeric value specified by the octal number. > > Looks pretty intentional to me. \0 is a nul. I was staring at the same passage, but somehow it didn't "click" for me that the above is _not_ saying that the NUL would terminate the string. So I was worried too much and needlessly. Thanks. ``` ## Andreas Schwab, 2012-05-01 07:29 Subject: Re: [PATCH v2] log-tree: use custom line terminator in line termination mode Message-ID: URL: https://gitlist.dev/e/m2mx5snytp.fsf%40linux-m68k.org In-Reply-To: <7v62cgenn4.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: > Andreas Schwab writes: > >> Junio C Hamano writes: >> >>> Having said that, are we sure that >>> >>> printf "add bar\0initial" >>> >>> works per specification, or merely works by accident in some >>> implementation? >> >> Since the backslash is not followed by $ ` " \ it is not >> special to the shell. > > Yeah, but I wasn't worried about what shell does in the first place. I was > worried about what printf(1) does. Oh, I was confused becaused you mentioned the C case where the \NNN interpretation is done by the compiler, not printf(3). Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ``` ## Jeff King, 2012-05-01 08:56 Subject: Re: [PATCH] log-tree: use custom line terminator in line termination mode Message-ID: <20120501085649.GD4998@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120501085649.GD4998%40sigill.intra.peff.net In-Reply-To: <7vaa1tf1w8.fsf@alter.siamese.dyndns.org> ``` On Mon, Apr 30, 2012 at 12:36:07PM -0700, Junio C Hamano wrote: > Jan Krüger writes: > > > When using a custom format in line termination mode (as opposed to line > > separation mode), the configured line terminator is not used, so things > > like "git log --pretty=tformat:%H -z" do not work properly. > > > > Make it use the line terminator the user ordered. > > > > Signed-off-by: Jan Krüger > > --- > > log-tree.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/log-tree.c b/log-tree.c > > index 34c49e7..44f0268 100644 > > --- a/log-tree.c > > +++ b/log-tree.c > > @@ -682,7 +682,7 @@ void show_log(struct rev_info *opt) > > if (opt->use_terminator) { > > if (!opt->missing_newline) > > graph_show_padding(opt->graph); > > - putchar('\n'); > > + putchar(opt->diffopt.line_termination); > > } > > > > strbuf_release(&msgbuf); > > Looks sensible. Perhaps we would want to add a test? Hmm. This came up before, and the issue is (or can be) slightly more complex: http://thread.gmane.org/gmane.comp.version-control.git/122478/focus=122568 -Peff ``` ## Junio C Hamano, 2012-05-01 16:12 Subject: Re: [PATCH] log-tree: use custom line terminator in line termination mode Message-ID: <7v1un3c23p.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v1un3c23p.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120501085649.GD4998@sigill.intra.peff.net> ``` Jeff King writes: > On Mon, Apr 30, 2012 at 12:36:07PM -0700, Junio C Hamano wrote: > >> Jan Krüger writes: >> >> > When using a custom format in line termination mode (as opposed to line >> > separation mode), the configured line terminator is not used, so things >> > like "git log --pretty=tformat:%H -z" do not work properly. >> > >> > Make it use the line terminator the user ordered. >> > >> > Signed-off-by: Jan Krüger >> > --- >> > log-tree.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/log-tree.c b/log-tree.c >> > index 34c49e7..44f0268 100644 >> > --- a/log-tree.c >> > +++ b/log-tree.c >> > @@ -682,7 +682,7 @@ void show_log(struct rev_info *opt) >> > if (opt->use_terminator) { >> > if (!opt->missing_newline) >> > graph_show_padding(opt->graph); >> > - putchar('\n'); >> > + putchar(opt->diffopt.line_termination); >> > } >> > >> > strbuf_release(&msgbuf); >> >> Looks sensible. Perhaps we would want to add a test? > > Hmm. This came up before, and the issue is (or can be) slightly more > complex: > > http://thread.gmane.org/gmane.comp.version-control.git/122478/focus=122568 Yeah, the test given completely forgets about "log -p" case, as you said in the above: But we can't just modify that to use the specified line terminator, because sometimes it is acting as a separator between commit message and diff, and sometimes it is acting as the terminator of the whole record. So the patch is not quite right for the "log -p -z" (or "log --stat -z") case. The correct output would have NUL after each commit, so "-z --format=%s" would have a single-liner subject with the line-terminating LF replaced with NUL, and "-p/--stat -z --format=%s" would have a single-liner subject with its line-terminating LF, followed by the diff/diffstat in which the terminating LF of the last line is replaced with NUL, but to be consistent with what "-p/--stat -z --pretty=format:%s" does, I think it is OK to append NUL to the diff/diffstat part instead of replacing its last LF with NUL. ``` ## Junio C Hamano, 2012-05-01 17:06 Subject: Re: [PATCH] log-tree: use custom line terminator in line termination mode Message-ID: <7vobq7al12.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vobq7al12.fsf%40alter.siamese.dyndns.org In-Reply-To: <7v1un3c23p.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: > Jeff King writes: > >> Hmm. This came up before, and the issue is (or can be) slightly more >> complex: >> >> http://thread.gmane.org/gmane.comp.version-control.git/122478/focus=122568 > > Yeah, the test given completely forgets about "log -p" case, as you said > in the above: > > But we can't just modify that to use the specified line terminator, > because sometimes it is acting as a separator between commit message and > diff, and sometimes it is acting as the terminator of the whole record. > > So the patch is not quite right for the "log -p -z" (or "log --stat -z") > case. > > The correct output would have NUL after each commit, so "-z --format=%s" > would have a single-liner subject with the line-terminating LF replaced > with NUL, and "-p/--stat -z --format=%s" would have a single-liner subject > with its line-terminating LF, followed by the diff/diffstat in which the > terminating LF of the last line is replaced with NUL, but to be consistent > with what "-p/--stat -z --pretty=format:%s" does, I think it is OK to > append NUL to the diff/diffstat part instead of replacing its last LF with > NUL. In other words, this test on top (the last one only demonstrates the breakage). t/t4205-log-pretty-formats.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index f00e446..4afd778 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -83,4 +83,20 @@ test_expect_success 'NUL termination' ' test_cmp expected actual ' +test_expect_success 'NUL separation with --stat' ' + stat0_part=$(git diff --stat HEAD^ HEAD) && + stat1_part=$(git diff --stat --root HEAD^) && + printf "add bar\n$stat0_part\n\0initial\n$stat1_part\n" >expected && + git log -z --stat --pretty="format:%s" >actual && + test_cmp expected actual +' + +test_expect_failure 'NUL termination with --stat' ' + stat0_part=$(git diff --stat HEAD^ HEAD) && + stat1_part=$(git diff --stat --root HEAD^) && + printf "add bar\n$stat0_part\n\0initial\n$stat1_part\n\0" >expected && + git log -z --stat --pretty="tformat:%s" >actual && + test_cmp expected actual +' + test_done ``` ## Junio C Hamano, 2012-05-01 20:03 Subject: Re: [PATCH] log-tree: use custom line terminator in line termination mode Message-ID: <7vmx5r8y9i.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vmx5r8y9i.fsf%40alter.siamese.dyndns.org In-Reply-To: <7vobq7al12.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: >> The correct output would have NUL after each commit, so "-z --format=%s" >> would have a single-liner subject with the line-terminating LF replaced >> with NUL, and "-p/--stat -z --format=%s" would have a single-liner subject >> with its line-terminating LF, followed by the diff/diffstat in which the >> terminating LF of the last line is replaced with NUL, but to be consistent >> with what "-p/--stat -z --pretty=format:%s" does, I think it is OK to >> append NUL to the diff/diffstat part instead of replacing its last LF with >> NUL. > > In other words, this test on top (the last one only demonstrates the > breakage). Just a short hint if anybody wants to take a stab at it while I am deeply in today's integration cycle. The code for "separator" semantics was a well tested code when "terminator" semantics was bolted on, and operated like this: for each record do if we have shown a record already then show the termination character fi show the record done The only difference between the two semantics is if we append the termination character after all the above is done, so the code should be structured that way, but that may not be how we currently do it. So the proper way to add the "terminator" semantics ought to be: if we have shown any record in the loop && opt->use_terminator then show the termination character fi at the end of the above loop. If we see opt->use_terminator anywhere else in the existing code, it is an indication of a bug. There is one small glitch. If a record is _not_ terminated with a newline, e.g. "log --pretty=format:%s" without --stat/-p, "separator" semantics will end up giving an incomplete line at the end. Most of the time we will be piping out output to the pager so it may not be a problem in the real life, but it would be nicer to also terminate such an output. So the resulting logic should look something like: for each record do if we have shown a record already then show the termination character fi show the record remember if the record ended with a LF done if we have shown any record in the loop && (opt->use_terminator || (opt->diffopt.line_termination == '\n' && the last record did not end with a LF)) then show the termination character fi ```