[PATCH] log-tree.c: Make log_tree_diff_flush() honor line_termination.

[PATCH] log-tree.c: Make log_tree_diff_flush() honor line_termination.

[Govind Salinas]

Signed-off-by: Govind Salinas <blix@sophiasuchtig.com>
---
I sent this in a few weeks ago, but it was not eligible for inclusion on 1.5.5.
There was some discussion but I was never sure if the patch was acceptable
to everyone.  I would like to know if this could be done for the next version?


 log-tree.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 5b29639..374b277 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -347,7 +347,7 @@ int log_tree_diff_flush(struct rev_info *opt)
 			int pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;
 			if ((pch & opt->diffopt.output_format) == pch)
 				printf("---");
-			putchar('\n');
+			putchar(opt->diffopt.line_termination);
 		}
 	}
 	diff_flush(&opt->diffopt);
-- 
1.5.5.rc2.131.g3d2f

Re: [PATCH] log-tree.c: Make log_tree_diff_flush() honor line_termination.

[Junio C Hamano]

"Govind Salinas" <govind@sophiasuchtig.com> writes:

[no justification here]

> Signed-off-by: Govind Salinas <blix@sophiasuchtig.com>

> diff --git a/log-tree.c b/log-tree.c
> index 5b29639..374b277 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -347,7 +347,7 @@ int log_tree_diff_flush(struct rev_info *opt)
>  			int pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;
>  			if ((pch & opt->diffopt.output_format) == pch)
>  				printf("---");
> -			putchar('\n');
> +			putchar(opt->diffopt.line_termination);
>  		}
>  	}
>  	diff_flush(&opt->diffopt);

What this "\n" separates are the commit log message (potentially followed
by the three-dash marker to signal "the patch follows") and the textual
diff (potentially with diffstat, which also is textual).

Lines in the textual diff part is always separated with "\n".  The patch
is line oriented by definition, and it does not make it any easier for the
tools to grok even if you made it NUL terminated.  The log message when
given by log-tree is typically indented by four spaces, so the beginning
of diff/patch part which is not indented can be detected easily without
the help fro NUL termination.  In other words, I do not think the tool
downstream you are writing is helped much with this change.  While I can
understand why you wanted to do it (i.e. "being consistent"), I do not
think the consistency buys us much in this particular case.

However, the tool downstream other people have already written to read
from the log-tree output already knows that there will be LF at this place
even if they drive log-tree with a "-z" option, as that has been the way
from the beginning.  I have a suspicion that tools like qgit may start
barfing with this change if they read from "-z" output.

Which makes the purist in me feel somewhat sad, but the pragmatist in me
is not convinced until he is shown how this will help the downstream tools
that read from the output from log-tree, which you didn't do with zero
line of a proposed commit log message.

A possible defense for this patch is that it _could_ make the output
easier to parse in the presense of a commit log message with a line that
begins with "diff --git" when log-tree is driven with --pretty=raw.