Jeff King <peff@xxxxxxxx> writes:
> On Thu, Aug 02, 2012 at 10:11:02PM +0100, Adam Butcher wrote:
>
>> From 01730a741cc5fd7d0a5d8bd0d3df80d12c81fe48 Mon Sep 17 00:00:00 2001
>> From: Adam Butcher <dev.lists@xxxxxxxxxxxxxxx>
>> Date: Wed, 1 Aug 2012 22:25:09 +0100
>> Subject: [PATCH] Fix 'No newline...' annotation in rewrite diffs.
>
> You can drop these lines from the email body; they are redundant with
> what's in your actual header.
s/can/should/ actually, for readability.
>> When operating in --break-rewrites (-B) mode on a file with no newline
>> terminator (and assuming --break-rewrites determines that the diff
>> _is_ a rewrite), git diff previously concatenated the indicator comment
>> '\ No newline at end of file' directly to the terminating line rather
>> than on a line of its own. The resulting diff is broken; claiming
>> that the last line actually contains the indicator text. Without -B
>> there is no problem with the same files.
>>
>> This patch fixes the former case by inserting a newline into the
>> output prior to emitting the indicator comment.
>
> Makes sense.
>
>> Potential issue: Currently this emits an ASCII 10 newline character
>> only. I'm not sure whether this will be okay on all platforms; it
>> seems to work fine on Windows and GNU at least.
>
> This should not be a problem. Git always outputs newlines; it is stdio
> who might munge it into CRLF if need be (and your patch uses putc, so we
> should be fine).
>
>> A couple of tests have been added to the rewrite suite to confirm that
>> the indicator comment is generated on its own line in both plain diff
>> and rewrite mode. The latter test fails if the functional part of
>> this patch (i.e. diff.c) is reverted.
>
> Yay, tests.
>
>> ---
Sign-off needed.
>> diff.c | 1 +
>> t/t4022-diff-rewrite.sh | 27 +++++++++++++++++++++++++++
>> 2 files changed, 28 insertions(+)
>>
>> diff --git a/diff.c b/diff.c
>> index 95706a5..77d4e84 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -574,6 +574,7 @@ static void emit_rewrite_lines(struct
>> emit_callback *ecb,
>
> Your patch is line-wrapped and cannot be applied as-is (try turning off
> "flowed text" in your MUA).
>
>> if (!endp) {
>> const char *plain = diff_get_color(ecb->color_diff,
>> DIFF_PLAIN);
>> + putc('\n', ecb->opt->file);
>> emit_line_0(ecb->opt, plain, reset, '\\',
>> nneof, strlen(nneof));
>> }
>
> Looks correct. I was curious how the regular (non-rewrite) code path did
> this, and it just sticks the "\n" as part of the nneof string. However,
> we would not want that here, because each line should have its own
> color markers.
>
>> +# create a file containing numbers with no newline at
>> +# the end and modify it such that the starting 10 lines
>> +# are unchanged, the next 101 are rewritten and the last
>> +# line differs only in that in is terminated by a newline.
>> +seq 1 10 > seq
>> +seq 100 +1 200 >> seq
>> +printf 201 >> seq
>> +(git add seq; git commit seq -m seq) >/dev/null
>> +seq 1 10 > seq
>> +seq 300 -1 200 >> seq
>
> Seq is (unfortunately) not portable. I usually use a perl snippet
> instead, like:
>
> perl -le 'print for (1..10)'
>
> Though I think we are adjusting that to use $PERL_PATH these days.
t/perf/perf-lib.sh and t/t5551-http-fetch.sh seem to use "seq";
perhaps we should replace them, then.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html