color-words: change algorithm to allow for 0-character word boundaries
Up until now, the color-words code assumed that word boundaries are identical to white space characters. Therefore, it could get away with a very simple scheme: it copied the hunks, substituted newlines for each white space character, called libxdiff with the processed text, and then identified the text to output by the offsets (which agreed since the original text had the same length). This code was ugly, for a number of reasons: - it was impossible to introduce 0-character word boundaries, - we had to print everything word by word, and - the code needed extra special handling of newlines in the removed part. Fix all of these issues by processing the text such that - we build word lists, separated by newlines, - we remember the original offsets for every word, and - after calling libxdiff on the wordlists, we parse the hunk headers, and find the corresponding offsets, and then - we print the removed/added parts in one go. The pre and post samples in the test were provided by Santi Béjar. Note that there is some strange special handling of hunk headers where one line range is 0 due to POSIX: in this case, the start is one too low. In other words a hunk header '@@ -1,0 +2 @@' actually means that the line must be added after the _second_ line of the pre text, _not_ the first. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin committed
Jan 17, 2009 at 17:29 UTC
2e5d2003b28820f88296e47a79eb440ca0295000
2 files changed
+157
-66
diff.c
+91
-66
@@ -319,8 +319,10 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
319
struct diff_words_buffer {
320
mmfile_t text;
321
long alloc;
322
- long current; /* output pointer */
323
- int suppressed_newline;
322
+ struct diff_words_orig {
323
+ const char *begin, *end;
324
+ } *orig;
325
+ int orig_nr, orig_alloc;
326
};
327
328
static void diff_words_append(char *line, unsigned long len,
@@ -335,80 +337,89 @@ static void diff_words_append(char *line, unsigned long len,
337
338
struct diff_words_data {
339
struct diff_words_buffer minus, plus;
340
+ const char *current_plus;
341
FILE *file;
342
};
343
341
-static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,
342
- int suppress_newline)
344
+static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
345
{
344
- const char *ptr;
345
- int eol = 0;
346
+ struct diff_words_data *diff_words = priv;
347
+ int minus_first, minus_len, plus_first, plus_len;
348
+ const char *minus_begin, *minus_end, *plus_begin, *plus_end;
349
347
- if (len == 0)
350
+ if (line[0] != '@' || parse_hunk_header(line, len,
351
+ &minus_first, &minus_len, &plus_first, &plus_len))
352
return;
353
350
- ptr = buffer->text.ptr + buffer->current;
351
- buffer->current += len;
354
+ /* POSIX requires that first be decremented by one if len == 0... */
355
+ if (minus_len) {
356
+ minus_begin = diff_words->minus.orig[minus_first].begin;
357
+ minus_end =
358
+ diff_words->minus.orig[minus_first + minus_len - 1].end;
359
+ } else
360
+ minus_begin = minus_end =
361
+ diff_words->minus.orig[minus_first].end;
362
353
- if (ptr[len - 1] == '\n') {
354
- eol = 1;
355
- len--;
356
- }
363
+ if (plus_len) {
364
+ plus_begin = diff_words->plus.orig[plus_first].begin;
365
+ plus_end = diff_words->plus.orig[plus_first + plus_len - 1].end;
366
+ } else
367
+ plus_begin = plus_end = diff_words->plus.orig[plus_first].end;
368
358
- fputs(diff_get_color(1, color), file);
359
- fwrite(ptr, len, 1, file);
360
- fputs(diff_get_color(1, DIFF_RESET), file);
369
+ if (diff_words->current_plus != plus_begin)
370
+ fwrite(diff_words->current_plus,
371
+ plus_begin - diff_words->current_plus, 1,
372
+ diff_words->file);
373
+ if (minus_begin != minus_end)
374
+ color_fwrite_lines(diff_words->file,
375
+ diff_get_color(1, DIFF_FILE_OLD),
376
+ minus_end - minus_begin, minus_begin);
377
+ if (plus_begin != plus_end)
378
+ color_fwrite_lines(diff_words->file,
379
+ diff_get_color(1, DIFF_FILE_NEW),
380
+ plus_end - plus_begin, plus_begin);
381
362
- if (eol) {
363
- if (suppress_newline)
364
- buffer->suppressed_newline = 1;
365
- else
366
- putc('\n', file);
367
- }
368
-}
369
-
370
-static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
371
-{
372
- struct diff_words_data *diff_words = priv;
373
-
374
- if (diff_words->minus.suppressed_newline) {
375
- if (line[0] != '+')
376
- putc('\n', diff_words->file);
377
- diff_words->minus.suppressed_newline = 0;
378
- }
379
-
380
- len--;
381
- switch (line[0]) {
382
- case '-':
383
- print_word(diff_words->file,
384
- &diff_words->minus, len, DIFF_FILE_OLD, 1);
385
- break;
386
- case '+':
387
- print_word(diff_words->file,
388
- &diff_words->plus, len, DIFF_FILE_NEW, 0);
389
- break;
390
- case ' ':
391
- print_word(diff_words->file,
392
- &diff_words->plus, len, DIFF_PLAIN, 0);
393
- diff_words->minus.current += len;
394
- break;
395
- }
382
+ diff_words->current_plus = plus_end;
383
}
384
385
/*
399
- * This function splits the words in buffer->text, and stores the list with
400
- * newline separator into out.
386
+ * This function splits the words in buffer->text, stores the list with
387
+ * newline separator into out, and saves the offsets of the original words
388
+ * in buffer->orig.
389
*/
390
static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out)
391
{
404
- int i;
405
- out->size = buffer->text.size;
406
- out->ptr = xmalloc(out->size);
407
- memcpy(out->ptr, buffer->text.ptr, out->size);
408
- for (i = 0; i < out->size; i++)
409
- if (isspace(out->ptr[i]))
410
- out->ptr[i] = '\n';
411
- buffer->current = 0;
392
+ int i, j;
393
+
394
+ out->size = 0;
395
+ out->ptr = xmalloc(buffer->text.size);
396
+
397
+ /* fake an empty "0th" word */
398
+ ALLOC_GROW(buffer->orig, 1, buffer->orig_alloc);
399
+ buffer->orig[0].begin = buffer->orig[0].end = buffer->text.ptr;
400
+ buffer->orig_nr = 1;
401
+
402
+ for (i = 0; i < buffer->text.size; i++) {
403
+ if (isspace(buffer->text.ptr[i]))
404
+ continue;
405
+ for (j = i + 1; j < buffer->text.size &&
406
+ !isspace(buffer->text.ptr[j]); j++)
407
+ ; /* find the end of the word */
408
+
409
+ /* store original boundaries */
410
+ ALLOC_GROW(buffer->orig, buffer->orig_nr + 1,
411
+ buffer->orig_alloc);
412
+ buffer->orig[buffer->orig_nr].begin = buffer->text.ptr + i;
413
+ buffer->orig[buffer->orig_nr].end = buffer->text.ptr + j;
414
+ buffer->orig_nr++;
415
+
416
+ /* store one word */
417
+ memcpy(out->ptr + out->size, buffer->text.ptr + i, j - i);
418
+ out->ptr[out->size + j - i] = '\n';
419
+ out->size += j - i + 1;
420
+
421
+ i = j - 1;
422
+ }
423
}
424
425
/* this executes the word diff on the accumulated buffers */
@@ -419,22 +430,34 @@ static void diff_words_show(struct diff_words_data *diff_words)
430
xdemitcb_t ecb;
431
mmfile_t minus, plus;
432
433
+ /* special case: only removal */
434
+ if (!diff_words->plus.text.size) {
435
+ color_fwrite_lines(diff_words->file,
436
+ diff_get_color(1, DIFF_FILE_OLD),
437
+ diff_words->minus.text.size, diff_words->minus.text.ptr);
438
+ diff_words->minus.text.size = 0;
439
+ return;
440
+ }
441
+
442
+ diff_words->current_plus = diff_words->plus.text.ptr;
443
+
444
memset(&xpp, 0, sizeof(xpp));
445
memset(&xecfg, 0, sizeof(xecfg));
446
diff_words_fill(&diff_words->minus, &minus);
447
diff_words_fill(&diff_words->plus, &plus);
448
xpp.flags = XDF_NEED_MINIMAL;
427
- xecfg.ctxlen = diff_words->minus.alloc + diff_words->plus.alloc;
449
+ xecfg.ctxlen = 0;
450
xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,
451
&xpp, &xecfg, &ecb);
452
free(minus.ptr);
453
free(plus.ptr);
454
+ if (diff_words->current_plus != diff_words->plus.text.ptr +
455
+ diff_words->plus.text.size)
456
+ fwrite(diff_words->current_plus,
457
+ diff_words->plus.text.ptr + diff_words->plus.text.size
458
+ - diff_words->current_plus, 1,
459
+ diff_words->file);
460
diff_words->minus.text.size = diff_words->plus.text.size = 0;
433
-
434
- if (diff_words->minus.suppressed_newline) {
435
- putc('\n', diff_words->file);
436
- diff_words->minus.suppressed_newline = 0;
437
- }
461
}
462
463
typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
@@ -458,7 +481,9 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
481
diff_words_show(ecbdata->diff_words);
482
483
free (ecbdata->diff_words->minus.text.ptr);
484
+ free (ecbdata->diff_words->minus.orig);
485
free (ecbdata->diff_words->plus.text.ptr);
486
+ free (ecbdata->diff_words->plus.orig);
487
free(ecbdata->diff_words);
488
ecbdata->diff_words = NULL;
489
}
t/t4034-diff-words.sh
new
+66
@@ -0,0 +1,66 @@
1
+#!/bin/sh
2
+
3
+test_description='word diff colors'
4
+
5
+. ./test-lib.sh
6
+
7
+test_expect_success setup '
8
+
9
+ git config diff.color.old red
10
+ git config diff.color.new green
11
+
12
+'
13
+
14
+decrypt_color () {
15
+ sed \
16
+ -e 's/.\[1m/<WHITE>/g' \
17
+ -e 's/.\[31m/<RED>/g' \
18
+ -e 's/.\[32m/<GREEN>/g' \
19
+ -e 's/.\[36m/<BROWN>/g' \
20
+ -e 's/.\[m/<RESET>/g'
21
+}
22
+
23
+word_diff () {
24
+ test_must_fail git diff --no-index "$@" pre post > output &&
25
+ decrypt_color < output > output.decrypted &&
26
+ test_cmp expect output.decrypted
27
+}
28
+
29
+cat > pre <<\EOF
30
+h(4)
31
+
32
+a = b + c
33
+EOF
34
+
35
+cat > post <<\EOF
36
+h(4),hh[44]
37
+
38
+a = b + c
39
+
40
+aa = a
41
+
42
+aeff = aeff * ( aaa )
43
+EOF
44
+
45
+cat > expect <<\EOF
46
+<WHITE>diff --git a/pre b/post<RESET>
47
+<WHITE>index 330b04f..5ed8eff 100644<RESET>
48
+<WHITE>--- a/pre<RESET>
49
+<WHITE>+++ b/post<RESET>
50
+<BROWN>@@ -1,3 +1,7 @@<RESET>
51
+<RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>
52
+<RESET>
53
+a = b + c<RESET>
54
+
55
+<GREEN>aa = a<RESET>
56
+
57
+<GREEN>aeff = aeff * ( aaa )<RESET>
58
+EOF
59
+
60
+test_expect_success 'word diff with runs of whitespace' '
61
+
62
+ word_diff --color-words
63
+
64
+'
65
+
66
+test_done