{"thread":{"id":"13205","subject":"[PATCH] log-tree.c: Make log_tree_diff_flush() honor line_termination.","startedAt":"2008-04-22T02:18:48Z","lastAt":"2008-04-22T02:58:27Z","messageCount":2,"participants":["Govind Salinas","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"74898","messageId":"5d46db230804211918u1444a80cwe1e977d37c2eb257@mail.gmail.com","threadId":"13205","inReplyTo":null,"subject":"[PATCH] log-tree.c: Make log_tree_diff_flush() honor line_termination.","fromName":"Govind Salinas","fromEmail":"govind@sophiasuchtig.com","sentAt":"2008-04-22T02:18:48Z","receivedAt":"2008-04-22T02:18:48Z","isPatch":true,"sender":{"key":"govind@sophiasuchtig.com","avatar":null},"body":"Signed-off-by: Govind Salinas <blix@sophiasuchtig.com>\n---\nI sent this in a few weeks ago, but it was not eligible for inclusion on 1.5.5.\nThere was some discussion but I was never sure if the patch was acceptable\nto everyone.  I would like to know if this could be done for the next version?\n\n\n log-tree.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 5b29639..374b277 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -347,7 +347,7 @@ int log_tree_diff_flush(struct rev_info *opt)\n \t\t\tint pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;\n \t\t\tif ((pch & opt->diffopt.output_format) == pch)\n \t\t\t\tprintf(\"---\");\n-\t\t\tputchar('\\n');\n+\t\t\tputchar(opt->diffopt.line_termination);\n \t\t}\n \t}\n \tdiff_flush(&opt->diffopt);\n-- \n1.5.5.rc2.131.g3d2f\n"},{"id":"74900","messageId":"7vskxe8ujw.fsf@gitster.siamese.dyndns.org","threadId":"13205","inReplyTo":"5d46db230804211918u1444a80cwe1e977d37c2eb257@mail.gmail.com","subject":"Re: [PATCH] log-tree.c: Make log_tree_diff_flush() honor line_termination.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-22T02:58:27Z","receivedAt":"2008-04-22T02:58:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Govind Salinas\" <govind@sophiasuchtig.com> writes:\n\n[no justification here]\n\n> Signed-off-by: Govind Salinas <blix@sophiasuchtig.com>\n\n> diff --git a/log-tree.c b/log-tree.c\n> index 5b29639..374b277 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -347,7 +347,7 @@ int log_tree_diff_flush(struct rev_info *opt)\n>  \t\t\tint pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;\n>  \t\t\tif ((pch & opt->diffopt.output_format) == pch)\n>  \t\t\t\tprintf(\"---\");\n> -\t\t\tputchar('\\n');\n> +\t\t\tputchar(opt->diffopt.line_termination);\n>  \t\t}\n>  \t}\n>  \tdiff_flush(&opt->diffopt);\n\nWhat this \"\\n\" separates are the commit log message (potentially followed\nby the three-dash marker to signal \"the patch follows\") and the textual\ndiff (potentially with diffstat, which also is textual).\n\nLines in the textual diff part is always separated with \"\\n\".  The patch\nis line oriented by definition, and it does not make it any easier for the\ntools to grok even if you made it NUL terminated.  The log message when\ngiven by log-tree is typically indented by four spaces, so the beginning\nof diff/patch part which is not indented can be detected easily without\nthe help fro NUL termination.  In other words, I do not think the tool\ndownstream you are writing is helped much with this change.  While I can\nunderstand why you wanted to do it (i.e. \"being consistent\"), I do not\nthink the consistency buys us much in this particular case.\n\nHowever, the tool downstream other people have already written to read\nfrom the log-tree output already knows that there will be LF at this place\neven if they drive log-tree with a \"-z\" option, as that has been the way\nfrom the beginning.  I have a suspicion that tools like qgit may start\nbarfing with this change if they read from \"-z\" output.\n\nWhich makes the purist in me feel somewhat sad, but the pragmatist in me\nis not convinced until he is shown how this will help the downstream tools\nthat read from the output from log-tree, which you didn't do with zero\nline of a proposed commit log message.\n\nA possible defense for this patch is that it _could_ make the output\neasier to parse in the presense of a commit log message with a line that\nbegins with \"diff --git\" when log-tree is driven with --pretty=raw.\n"}]}