From 77d1a520fb5b8ad8cc86228023f16a44b75c05d1 Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Wed, 14 Mar 2012 19:24:08 +0100 Subject: diff: refactor the word-diff setup from builtin_diff_cmd Quite a chunk of builtin_diff_cmd deals with word-diff setup, defaults and such. This makes the function a bit hard to read, but is also asymmetric because the corresponding teardown lives in free_diff_words_data already. Refactor into a new function init_diff_words_data. For simplicity, also shuffle around some functions it depends on. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano --- diff.c | 119 ++++++++++++++++++++++++++++++++++------------------------------- 1 file changed, 63 insertions(+), 56 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 5388ded21..526f19805 100644 --- a/diff.c +++ b/diff.c @@ -987,6 +987,67 @@ static void diff_words_flush(struct emit_callback *ecbdata) diff_words_show(ecbdata->diff_words); } +static void diff_filespec_load_driver(struct diff_filespec *one) +{ + /* Use already-loaded driver */ + if (one->driver) + return; + + if (S_ISREG(one->mode)) + one->driver = userdiff_find_by_path(one->path); + + /* Fallback to default settings */ + if (!one->driver) + one->driver = userdiff_find_by_name("default"); +} + +static const char *userdiff_word_regex(struct diff_filespec *one) +{ + diff_filespec_load_driver(one); + return one->driver->word_regex; +} + +static void init_diff_words_data(struct emit_callback *ecbdata, + struct diff_options *o, + struct diff_filespec *one, + struct diff_filespec *two) +{ + int i; + + ecbdata->diff_words = + xcalloc(1, sizeof(struct diff_words_data)); + ecbdata->diff_words->type = o->word_diff; + ecbdata->diff_words->opt = o; + if (!o->word_regex) + o->word_regex = userdiff_word_regex(one); + if (!o->word_regex) + o->word_regex = userdiff_word_regex(two); + if (!o->word_regex) + o->word_regex = diff_word_regex_cfg; + if (o->word_regex) { + ecbdata->diff_words->word_regex = (regex_t *) + xmalloc(sizeof(regex_t)); + if (regcomp(ecbdata->diff_words->word_regex, + o->word_regex, + REG_EXTENDED | REG_NEWLINE)) + die ("Invalid regular expression: %s", + o->word_regex); + } + for (i = 0; i < ARRAY_SIZE(diff_words_styles); i++) { + if (o->word_diff == diff_words_styles[i].type) { + ecbdata->diff_words->style = + &diff_words_styles[i]; + break; + } + } + if (want_color(o->use_color)) { + struct diff_words_style *st = ecbdata->diff_words->style; + st->old.color = diff_get_color_opt(o, DIFF_FILE_OLD); + st->new.color = diff_get_color_opt(o, DIFF_FILE_NEW); + st->ctx.color = diff_get_color_opt(o, DIFF_PLAIN); + } +} + static void free_diff_words_data(struct emit_callback *ecbdata) { if (ecbdata->diff_words) { @@ -2016,20 +2077,6 @@ static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two, char *pre emit_binary_diff_body(file, two, one, prefix); } -static void diff_filespec_load_driver(struct diff_filespec *one) -{ - /* Use already-loaded driver */ - if (one->driver) - return; - - if (S_ISREG(one->mode)) - one->driver = userdiff_find_by_path(one->path); - - /* Fallback to default settings */ - if (!one->driver) - one->driver = userdiff_find_by_name("default"); -} - int diff_filespec_is_binary(struct diff_filespec *one) { if (one->is_binary == -1) { @@ -2055,12 +2102,6 @@ static const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespe return one->driver->funcname.pattern ? &one->driver->funcname : NULL; } -static const char *userdiff_word_regex(struct diff_filespec *one) -{ - diff_filespec_load_driver(one); - return one->driver->word_regex; -} - void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b) { if (!options->a_prefix) @@ -2247,42 +2288,8 @@ static void builtin_diff(const char *name_a, xecfg.ctxlen = strtoul(diffopts + 10, NULL, 10); else if (!prefixcmp(diffopts, "-u")) xecfg.ctxlen = strtoul(diffopts + 2, NULL, 10); - if (o->word_diff) { - int i; - - ecbdata.diff_words = - xcalloc(1, sizeof(struct diff_words_data)); - ecbdata.diff_words->type = o->word_diff; - ecbdata.diff_words->opt = o; - if (!o->word_regex) - o->word_regex = userdiff_word_regex(one); - if (!o->word_regex) - o->word_regex = userdiff_word_regex(two); - if (!o->word_regex) - o->word_regex = diff_word_regex_cfg; - if (o->word_regex) { - ecbdata.diff_words->word_regex = (regex_t *) - xmalloc(sizeof(regex_t)); - if (regcomp(ecbdata.diff_words->word_regex, - o->word_regex, - REG_EXTENDED | REG_NEWLINE)) - die ("Invalid regular expression: %s", - o->word_regex); - } - for (i = 0; i < ARRAY_SIZE(diff_words_styles); i++) { - if (o->word_diff == diff_words_styles[i].type) { - ecbdata.diff_words->style = - &diff_words_styles[i]; - break; - } - } - if (want_color(o->use_color)) { - struct diff_words_style *st = ecbdata.diff_words->style; - st->old.color = diff_get_color_opt(o, DIFF_FILE_OLD); - st->new.color = diff_get_color_opt(o, DIFF_FILE_NEW); - st->ctx.color = diff_get_color_opt(o, DIFF_PLAIN); - } - } + if (o->word_diff) + init_diff_words_data(&ecbdata, o, one, two); xdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata, &xpp, &xecfg); if (o->word_diff) -- cgit v1.2.1 From 6440d3417c1d51a20014d4b6fc6c59bacfa87dab Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Wed, 14 Mar 2012 19:24:09 +0100 Subject: diff: tweak a _copy_ of diff_options with word-diff When using word diff, the code sets the word_regex from various defaults if it was not set already. The problem is that it does this on the original diff_options, which will also be used in subsequent diffs. This means that when the word_regex is not given on the command line, only the first diff for which a setting for word_regex (either from attributes or diff.wordRegex) ever takes effect. This value then propagates to the rest of the diff runs and in particular prevents further attribute lookups. Fix the problem of changing diff state once and for all, by working with a _copy_ of the diff_options. Noticed-by: Johannes Sixt Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano --- diff.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 526f19805..349a61d58 100644 --- a/diff.c +++ b/diff.c @@ -1008,11 +1008,13 @@ static const char *userdiff_word_regex(struct diff_filespec *one) } static void init_diff_words_data(struct emit_callback *ecbdata, - struct diff_options *o, + struct diff_options *orig_opts, struct diff_filespec *one, struct diff_filespec *two) { int i; + struct diff_options *o = xmalloc(sizeof(struct diff_options)); + memcpy(o, orig_opts, sizeof(struct diff_options)); ecbdata->diff_words = xcalloc(1, sizeof(struct diff_words_data)); @@ -1052,6 +1054,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata) { if (ecbdata->diff_words) { diff_words_flush(ecbdata); + free (ecbdata->diff_words->opt); free (ecbdata->diff_words->minus.text.ptr); free (ecbdata->diff_words->minus.orig); free (ecbdata->diff_words->plus.text.ptr); -- cgit v1.2.1