From: Johannes Schindelin Date: Sat, 17 Jan 2009 16:29:44 +0000 (+0100) Subject: color-words: change algorithm to allow for 0-character word boundaries X-Git-Tag: v1.6.2-rc0~89^2~6 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=2e5d2003b28820f88296e47a79eb440ca0295000;p=git.git 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 Signed-off-by: Junio C Hamano --- diff --git a/diff.c b/diff.c index c111eef13..37c886a81 100644 --- a/diff.c +++ b/diff.c @@ -319,8 +319,10 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one) struct diff_words_buffer { mmfile_t text; long alloc; - long current; /* output pointer */ - int suppressed_newline; + struct diff_words_orig { + const char *begin, *end; + } *orig; + int orig_nr, orig_alloc; }; static void diff_words_append(char *line, unsigned long len, @@ -335,80 +337,89 @@ static void diff_words_append(char *line, unsigned long len, struct diff_words_data { struct diff_words_buffer minus, plus; + const char *current_plus; FILE *file; }; -static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color, - int suppress_newline) +static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len) { - const char *ptr; - int eol = 0; + struct diff_words_data *diff_words = priv; + int minus_first, minus_len, plus_first, plus_len; + const char *minus_begin, *minus_end, *plus_begin, *plus_end; - if (len == 0) + if (line[0] != '@' || parse_hunk_header(line, len, + &minus_first, &minus_len, &plus_first, &plus_len)) return; - ptr = buffer->text.ptr + buffer->current; - buffer->current += len; + /* POSIX requires that first be decremented by one if len == 0... */ + if (minus_len) { + minus_begin = diff_words->minus.orig[minus_first].begin; + minus_end = + diff_words->minus.orig[minus_first + minus_len - 1].end; + } else + minus_begin = minus_end = + diff_words->minus.orig[minus_first].end; - if (ptr[len - 1] == '\n') { - eol = 1; - len--; - } + if (plus_len) { + plus_begin = diff_words->plus.orig[plus_first].begin; + plus_end = diff_words->plus.orig[plus_first + plus_len - 1].end; + } else + plus_begin = plus_end = diff_words->plus.orig[plus_first].end; - fputs(diff_get_color(1, color), file); - fwrite(ptr, len, 1, file); - fputs(diff_get_color(1, DIFF_RESET), file); + if (diff_words->current_plus != plus_begin) + fwrite(diff_words->current_plus, + plus_begin - diff_words->current_plus, 1, + diff_words->file); + if (minus_begin != minus_end) + color_fwrite_lines(diff_words->file, + diff_get_color(1, DIFF_FILE_OLD), + minus_end - minus_begin, minus_begin); + if (plus_begin != plus_end) + color_fwrite_lines(diff_words->file, + diff_get_color(1, DIFF_FILE_NEW), + plus_end - plus_begin, plus_begin); - if (eol) { - if (suppress_newline) - buffer->suppressed_newline = 1; - else - putc('\n', file); - } -} - -static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len) -{ - struct diff_words_data *diff_words = priv; - - if (diff_words->minus.suppressed_newline) { - if (line[0] != '+') - putc('\n', diff_words->file); - diff_words->minus.suppressed_newline = 0; - } - - len--; - switch (line[0]) { - case '-': - print_word(diff_words->file, - &diff_words->minus, len, DIFF_FILE_OLD, 1); - break; - case '+': - print_word(diff_words->file, - &diff_words->plus, len, DIFF_FILE_NEW, 0); - break; - case ' ': - print_word(diff_words->file, - &diff_words->plus, len, DIFF_PLAIN, 0); - diff_words->minus.current += len; - break; - } + diff_words->current_plus = plus_end; } /* - * This function splits the words in buffer->text, and stores the list with - * newline separator into out. + * This function splits the words in buffer->text, stores the list with + * newline separator into out, and saves the offsets of the original words + * in buffer->orig. */ static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out) { - int i; - out->size = buffer->text.size; - out->ptr = xmalloc(out->size); - memcpy(out->ptr, buffer->text.ptr, out->size); - for (i = 0; i < out->size; i++) - if (isspace(out->ptr[i])) - out->ptr[i] = '\n'; - buffer->current = 0; + int i, j; + + out->size = 0; + out->ptr = xmalloc(buffer->text.size); + + /* fake an empty "0th" word */ + ALLOC_GROW(buffer->orig, 1, buffer->orig_alloc); + buffer->orig[0].begin = buffer->orig[0].end = buffer->text.ptr; + buffer->orig_nr = 1; + + for (i = 0; i < buffer->text.size; i++) { + if (isspace(buffer->text.ptr[i])) + continue; + for (j = i + 1; j < buffer->text.size && + !isspace(buffer->text.ptr[j]); j++) + ; /* find the end of the word */ + + /* store original boundaries */ + ALLOC_GROW(buffer->orig, buffer->orig_nr + 1, + buffer->orig_alloc); + buffer->orig[buffer->orig_nr].begin = buffer->text.ptr + i; + buffer->orig[buffer->orig_nr].end = buffer->text.ptr + j; + buffer->orig_nr++; + + /* store one word */ + memcpy(out->ptr + out->size, buffer->text.ptr + i, j - i); + out->ptr[out->size + j - i] = '\n'; + out->size += j - i + 1; + + i = j - 1; + } } /* this executes the word diff on the accumulated buffers */ @@ -419,22 +430,34 @@ static void diff_words_show(struct diff_words_data *diff_words) xdemitcb_t ecb; mmfile_t minus, plus; + /* special case: only removal */ + if (!diff_words->plus.text.size) { + color_fwrite_lines(diff_words->file, + diff_get_color(1, DIFF_FILE_OLD), + diff_words->minus.text.size, diff_words->minus.text.ptr); + diff_words->minus.text.size = 0; + return; + } + + diff_words->current_plus = diff_words->plus.text.ptr; + memset(&xpp, 0, sizeof(xpp)); memset(&xecfg, 0, sizeof(xecfg)); diff_words_fill(&diff_words->minus, &minus); diff_words_fill(&diff_words->plus, &plus); xpp.flags = XDF_NEED_MINIMAL; - xecfg.ctxlen = diff_words->minus.alloc + diff_words->plus.alloc; + xecfg.ctxlen = 0; xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words, &xpp, &xecfg, &ecb); free(minus.ptr); free(plus.ptr); + if (diff_words->current_plus != diff_words->plus.text.ptr + + diff_words->plus.text.size) + fwrite(diff_words->current_plus, + diff_words->plus.text.ptr + diff_words->plus.text.size + - diff_words->current_plus, 1, + diff_words->file); diff_words->minus.text.size = diff_words->plus.text.size = 0; - - if (diff_words->minus.suppressed_newline) { - putc('\n', diff_words->file); - diff_words->minus.suppressed_newline = 0; - } } 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) diff_words_show(ecbdata->diff_words); free (ecbdata->diff_words->minus.text.ptr); + free (ecbdata->diff_words->minus.orig); free (ecbdata->diff_words->plus.text.ptr); + free (ecbdata->diff_words->plus.orig); free(ecbdata->diff_words); ecbdata->diff_words = NULL; } diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh new file mode 100755 index 000000000..b22195f8b --- /dev/null +++ b/t/t4034-diff-words.sh @@ -0,0 +1,66 @@ +#!/bin/sh + +test_description='word diff colors' + +. ./test-lib.sh + +test_expect_success setup ' + + git config diff.color.old red + git config diff.color.new green + +' + +decrypt_color () { + sed \ + -e 's/.\[1m//g' \ + -e 's/.\[31m//g' \ + -e 's/.\[32m//g' \ + -e 's/.\[36m//g' \ + -e 's/.\[m//g' +} + +word_diff () { + test_must_fail git diff --no-index "$@" pre post > output && + decrypt_color < output > output.decrypted && + test_cmp expect output.decrypted +} + +cat > pre <<\EOF +h(4) + +a = b + c +EOF + +cat > post <<\EOF +h(4),hh[44] + +a = b + c + +aa = a + +aeff = aeff * ( aaa ) +EOF + +cat > expect <<\EOF +diff --git a/pre b/post +index 330b04f..5ed8eff 100644 +--- a/pre ++++ b/post +@@ -1,3 +1,7 @@ +h(4)h(4),hh[44] + +a = b + c + +aa = a + +aeff = aeff * ( aaa ) +EOF + +test_expect_success 'word diff with runs of whitespace' ' + + word_diff --color-words + +' + +test_done