On Mon, May 15, 2017 at 12:09 PM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
>
>
> On 05/13/2017 09:01 PM, Stefan Beller wrote:
>>
>> In a later patch, I want to propose an option to detect&color
>> moved lines in a diff, which cannot be done in a one-pass over
>> the diff. Instead we need to go over the whole diff twice,
>> because we cannot detect the first line of the two corresponding
>> lines (+ and -) that got moved.
>>
>> So to prepare the diff machinery for two pass algorithms
>> (i.e. buffer it all up and then operate on the result),
>> move all emissions to places, such that the only emitting
>> function is emit_line_0.
>>
>> This covers emit_rewrite_lines.
>>
>> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
>> ---
>> diff.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/diff.c b/diff.c
>> index e4b46fee4f..369c804f03 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -748,7 +748,7 @@ static void emit_rewrite_lines(struct emit_callback
>> *ecb,
>> if (!endp) {
>> const char *context = diff_get_color(ecb->color_diff,
>> DIFF_CONTEXT);
>> - putc('\n', ecb->opt->file);
>> + emit_line(ecb->opt, NULL, NULL, "\n", 1);
>
>
> This outputs diff_line_prefix(ecb->opt) - is that OK?
It shows this area is not covered well by our test suite.
My first reaction was that this is not ok, but we'd have
to inspect the situation. It was introduced in
35e2d03c2c (Fix '\ No newline...' annotation in rewrite diffs,
2012-08-04).
And looking at the code of the function I think this is broken.
I wonder what the best way forward is for this patch series here,
as we'd need to buffer the last line. That should be fine as it is
a corner case, maybe:
diff --git a/diff.c b/diff.c
index 0f10736ee6..f46e52135d 100644
--- a/diff.c
+++ b/diff.c
@@ -1011,15 +1011,27 @@ static void add_line_count(struct strbuf *out,
int count)
static void emit_rewrite_lines(struct emit_callback *ecb,
int prefix, const char *data, int size)
{
- const char *endp = NULL;
- static const char *nneof = " No newline at end of file\n";
+ static const char *nneof = "\\ No newline at end of file\n";
const char *reset = diff_get_color(ecb->color_diff, DIFF_RESET);
+ struct strbuf sb = STRBUF_INIT;
while (0 < size) {
int len;
endp = memchr(data, '\n', size);
- len = endp ? (endp - data + 1) : size;
+ if (endp)
+ len = endp - data + 1;
+ else {
+ /* last line has no \n */
+ while (0 < size) {
+ strbuf_addch(&sb, *data);
+ size -= len;
+ data += len;
+ }
+ strbuf_addch(&sb, '\n');
+ data = sb.buf;
+ len = sb.len;
+ }
if (prefix != '+') {
ecb->lno_in_preimage++;
emit_del_line(reset, ecb, data, len);
@@ -1030,12 +1042,12 @@ static void emit_rewrite_lines(struct
emit_callback *ecb,
size -= len;
data += len;
}
- if (!endp) {
+ if (sb.len) {
const char *context = diff_get_color(ecb->color_diff,
DIFF_CONTEXT);
- emit_line(ecb->opt, NULL, NULL, "\n", 1);
- emit_line_0(ecb->opt, context, reset, '\\',
+ emit_line(ecb->opt, context, reset,
nneof, strlen(nneof));
+ strbuf_release(&sb);
}
}