* [PATCH 0/2] gitk --color-words [not found] <cover.1269996525.git.trast@student.ethz> @ 2010-04-04 13:46 ` Thomas Rast 2010-04-04 13:46 ` [PATCH 1/2] diff: add --word-diff option that generalizes --color-words Thomas Rast 2010-04-04 13:46 ` [PATCH 2/2] gitk: add the equivalent of diff --color-words Thomas Rast 0 siblings, 2 replies; 30+ messages in thread From: Thomas Rast @ 2010-04-04 13:46 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader Miles Bader wrote: > Junio C Hamano <gitster@pobox.com> writes: > > How readable can you make this for human consumption while still keeping > > it machine readable? The answer could be it already is human readble. > > > > Two reasons I ask the above question are that I find the feature quite > > interesting, and would want to see if it can be also fed to humans, and > > that the combination of this new option and the existing --color-words is > > misnamed. > > There's the format used by the "wdiff" program, which is more like > traditional diff output in that it doesn't use color, but is human > friendly, and also seems to be somewhat machine-parseable: > > $ echo 'This is a test' > /tmp/a > $ echo 'This is funky test' > /tmp/b > $ wdiff /tmp/a /tmp/b > This is [-a-] {+funky+} test > > [I say "somewhat" because wdiff itself doesn't appear to escape potentially > ambiguous content, e.g., if there's actually a "{+" in the file....] Junio C Hamano wrote: > If you call this --word-diff, then it would become more clear that > --color-words perhaps should have been called --word-diff=color or > something. Excellent ideas! I don't have anything to add ;-) This makes [1/2] a rather new patch though, I moved the whole prefix/suffix handing further out to accommodate different styles. There's a little change in [2/2] apart from the obvious option renaming, too: it interprets --color-words and --word-diff as an initial setting for the checkbox. Thomas Rast (2): diff: add --word-diff option that generalizes --color-words gitk: add the equivalent of diff --color-words ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/2] diff: add --word-diff option that generalizes --color-words 2010-04-04 13:46 ` [PATCH 0/2] gitk --color-words Thomas Rast @ 2010-04-04 13:46 ` Thomas Rast 2010-04-05 2:06 ` Junio C Hamano 2010-04-04 13:46 ` [PATCH 2/2] gitk: add the equivalent of diff --color-words Thomas Rast 1 sibling, 1 reply; 30+ messages in thread From: Thomas Rast @ 2010-04-04 13:46 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader This teaches the --color-words engine a more general interface that supports two new modes: * --word-diff=plain, inspired by the 'wdiff' utility (most similar to 'wdiff -n <old> <new>'): uses delimiters [-removed-] and {+added+} * --word-diff=porcelain, which generates an ad-hoc machine readable format: - each diff unit is prefixed by [-+ ] and terminated by newline as in unified diff - newlines in the input are output as a line consisting only of a tilde '~' --color-words becomes a synonym for --word-diff=color. Also adds some compatibility/convenience options. Thanks to Junio C Hamano and Miles Bader for good ideas. Signed-off-by: Thomas Rast <trast@student.ethz.ch> --- Documentation/diff-options.txt | 39 +++++++++- Documentation/gitattributes.txt | 2 +- color.c | 28 ------- color.h | 1 - diff.c | 148 ++++++++++++++++++++++++++++++++++----- diff.h | 11 +++- t/t4034-diff-words.sh | 70 ++++++++++++++++++ 7 files changed, 245 insertions(+), 54 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 60e922e..e51fc9a 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -126,11 +126,38 @@ any of those replacements occurred. gives the default to color output. Same as `--color=never`. ---color-words[=<regex>]:: - Show colored word diff, i.e., color words which have changed. - By default, words are separated by whitespace. +--word-diff[=<mode>]:: + Show a word diff, using the <mode> to delimit changed words. + By default, words are delimited by whitespace; see + `--word-diff-regex` below. The <mode> defaults to 'auto', and + must be one of: ++ +-- +color:: + Highlight changed words with colors. Implies `--color`. +plain:: + Show words as `[-removed-]` and `{+added+}`. Makes no + attempts to escape the delimiters if they appear in the input, + so the output may be ambiguous. +porcelain:: + Use a special line-based format intended for script + consumption. Added/removed/unchanged runs are printed in the + usual unified diff format, starting with a `+`/`-`/` ` + character at the beginning of the line and extending to the + end of the line. Newlines in the input are represented by a + tilde `~` on a line of its own. +auto:: + 'color' if color is enabled, 'plain' otherwise. +none:: + Disable word diff again. +-- + +--word-diff-regex=<regex>:: + Use <regex> to decide what a word is, instead of considering + runs of non-whitespace to be a word. Also implies + `--word-diff=auto` unless it was already enabled. + -When a <regex> is specified, every non-overlapping match of the +Every non-overlapping match of the <regex> is considered a word. Anything between these matches is considered whitespace and ignored(!) for the purposes of finding differences. You may want to append `|[^[:space:]]` to your regular @@ -142,6 +169,10 @@ The regex can also be set via a diff driver or configuration option, see linkgit:gitattributes[1] or linkgit:git-config[1]. Giving it explicitly overrides any diff driver or configuration setting. Diff drivers override configuration settings. + +--color-words[=<regex>]:: + Equivalent to `--word-diff=color` plus (if a regex was + specified) `--word-diff-regex=<regex>`. endif::git-format-patch[] --no-renames:: diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index d892e64..7554fcd 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -360,7 +360,7 @@ patterns are available: Customizing word diff ^^^^^^^^^^^^^^^^^^^^^ -You can customize the rules that `git diff --color-words` uses to +You can customize the rules that `git diff --word-diff` uses to split words in a line, by specifying an appropriate regular expression in the "diff.*.wordRegex" configuration variable. For example, in TeX a backslash followed by a sequence of letters forms a command, but diff --git a/color.c b/color.c index bcf4e2c..1b00554 100644 --- a/color.c +++ b/color.c @@ -211,31 +211,3 @@ int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...) va_end(args); return r; } - -/* - * This function splits the buffer by newlines and colors the lines individually. - * - * Returns 0 on success. - */ -int color_fwrite_lines(FILE *fp, const char *color, - size_t count, const char *buf) -{ - if (!*color) - return fwrite(buf, count, 1, fp) != 1; - while (count) { - char *p = memchr(buf, '\n', count); - if (p != buf && (fputs(color, fp) < 0 || - fwrite(buf, p ? p - buf : count, 1, fp) != 1 || - fputs(GIT_COLOR_RESET, fp) < 0)) - return -1; - if (!p) - return 0; - if (fputc('\n', fp) < 0) - return -1; - count -= p + 1 - buf; - buf = p + 1; - } - return 0; -} - - diff --git a/color.h b/color.h index 5c264b0..03ca064 100644 --- a/color.h +++ b/color.h @@ -61,6 +61,5 @@ int color_fprintf(FILE *fp, const char *color, const char *fmt, ...); __attribute__((format (printf, 3, 4))) int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...); -int color_fwrite_lines(FILE *fp, const char *color, size_t count, const char *buf); #endif /* COLOR_H */ diff --git a/diff.c b/diff.c index f5d93e9..dca310e 100644 --- a/diff.c +++ b/diff.c @@ -572,16 +572,60 @@ static void diff_words_append(char *line, unsigned long len, buffer->text.ptr[buffer->text.size] = '\0'; } +struct diff_words_style +{ + enum diff_words_type type; + int is_color; + const char *new_prefix; + const char *new_suffix; + const char *old_prefix; + const char *old_suffix; + const char *ctx_prefix; + const char *ctx_suffix; + const char *newline; +}; + +struct diff_words_style diff_words_styles[] = { + { DIFF_WORDS_PORCELAIN, 0, "+", "\n", "-", "\n", " ", "\n", "~\n" }, + { DIFF_WORDS_PLAIN, 0, "{+", "+}", "[-", "-]", "", "", "\n" }, + { DIFF_WORDS_COLOR, 1, NULL, NULL, NULL, NULL, NULL, NULL, "\n" } +}; + struct diff_words_data { struct diff_words_buffer minus, plus; const char *current_plus; FILE *file; regex_t *word_regex; + enum diff_words_type type; + struct diff_words_style *style; }; +static int fn_out_diff_words_write_helper(FILE *fp, + const char *prefix, + const char *suffix, + const char *newline, + size_t count, const char *buf) +{ + while (count) { + char *p = memchr(buf, '\n', count); + if (p != buf && (fputs(prefix, fp) < 0 || + fwrite(buf, p ? p - buf : count, 1, fp) != 1 || + fputs(suffix, fp) < 0)) + return -1; + if (!p) + return 0; + if (fputs(newline, fp) < 0) + return -1; + count -= p + 1 - buf; + buf = p + 1; + } + return 0; +} + static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len) { struct diff_words_data *diff_words = priv; + struct diff_words_style *style = diff_words->style; int minus_first, minus_len, plus_first, plus_len; const char *minus_begin, *minus_end, *plus_begin, *plus_end; @@ -605,16 +649,20 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len) plus_begin = plus_end = diff_words->plus.orig[plus_first].end; if (diff_words->current_plus != plus_begin) - fwrite(diff_words->current_plus, - plus_begin - diff_words->current_plus, 1, - diff_words->file); + fn_out_diff_words_write_helper(diff_words->file, + style->ctx_prefix, style->ctx_suffix, + style->newline, + plus_begin - diff_words->current_plus, + diff_words->current_plus); if (minus_begin != minus_end) - color_fwrite_lines(diff_words->file, - diff_get_color(1, DIFF_FILE_OLD), + fn_out_diff_words_write_helper(diff_words->file, + style->old_prefix, style->old_suffix, + style->newline, minus_end - minus_begin, minus_begin); if (plus_begin != plus_end) - color_fwrite_lines(diff_words->file, - diff_get_color(1, DIFF_FILE_NEW), + fn_out_diff_words_write_helper(diff_words->file, + style->new_prefix, style->new_suffix, + style->newline, plus_end - plus_begin, plus_begin); diff_words->current_plus = plus_end; @@ -697,11 +745,13 @@ static void diff_words_show(struct diff_words_data *diff_words) xdemitconf_t xecfg; xdemitcb_t ecb; mmfile_t minus, plus; + struct diff_words_style *style = diff_words->style; /* special case: only removal */ if (!diff_words->plus.text.size) { - color_fwrite_lines(diff_words->file, - diff_get_color(1, DIFF_FILE_OLD), + fn_out_diff_words_write_helper(diff_words->file, + style->old_prefix, style->old_suffix, + style->newline, diff_words->minus.text.size, diff_words->minus.text.ptr); diff_words->minus.text.size = 0; return; @@ -722,10 +772,11 @@ static void diff_words_show(struct diff_words_data *diff_words) free(plus.ptr); if (diff_words->current_plus != diff_words->plus.text.ptr + diff_words->plus.text.size) - fwrite(diff_words->current_plus, + fn_out_diff_words_write_helper(diff_words->file, + style->ctx_prefix, style->ctx_suffix, + style->newline, diff_words->plus.text.ptr + diff_words->plus.text.size - - diff_words->current_plus, 1, - diff_words->file); + - diff_words->current_plus, diff_words->current_plus); diff_words->minus.text.size = diff_words->plus.text.size = 0; } @@ -837,6 +888,9 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) if (len < 1) { emit_line(ecbdata->file, reset, reset, line, len); + if (ecbdata->diff_words + && ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) + fputs("~\n", ecbdata->file); return; } @@ -851,9 +905,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) return; } diff_words_flush(ecbdata); - line++; - len--; - emit_line(ecbdata->file, plain, reset, line, len); + if (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) { + emit_line(ecbdata->file, plain, reset, line, len); + fputs("~\n", ecbdata->file); + } else { + /* don't print the prefix character */ + emit_line(ecbdata->file, plain, reset, line+1, len-1); + } return; } @@ -1755,10 +1813,19 @@ 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 (DIFF_OPT_TST(o, COLOR_DIFF_WORDS)) { + if (o->word_diff) { + int i; + + if (o->word_diff == DIFF_WORDS_AUTO) { + if (DIFF_OPT_TST(o, COLOR_DIFF)) + o->word_diff = DIFF_WORDS_COLOR; + else + o->word_diff = DIFF_WORDS_PLAIN; + } ecbdata.diff_words = xcalloc(1, sizeof(struct diff_words_data)); ecbdata.diff_words->file = o->file; + ecbdata.diff_words->type = o->word_diff; if (!o->word_regex) o->word_regex = userdiff_word_regex(one); if (!o->word_regex) @@ -1774,10 +1841,27 @@ static void builtin_diff(const char *name_a, 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 (ecbdata.diff_words->style->is_color) { + struct diff_words_style *s + = ecbdata.diff_words->style; + s->new_prefix = diff_get_color_opt(o, DIFF_FILE_NEW); + s->old_prefix = diff_get_color_opt(o, DIFF_FILE_OLD); + s->ctx_prefix = diff_get_color_opt(o, DIFF_PLAIN); + s->new_suffix = s->old_suffix + = diff_get_color_opt(o, DIFF_RESET); + s->ctx_suffix = ""; + } } xdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata, &xpp, &xecfg, &ecb); - if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS)) + if (o->word_diff) free_diff_words_data(&ecbdata); if (textconv_one) free(mf1.ptr); @@ -2845,13 +2929,39 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) DIFF_OPT_CLR(options, COLOR_DIFF); else if (!strcmp(arg, "--color-words")) { DIFF_OPT_SET(options, COLOR_DIFF); - DIFF_OPT_SET(options, COLOR_DIFF_WORDS); + options->word_diff = DIFF_WORDS_COLOR; } else if (!prefixcmp(arg, "--color-words=")) { DIFF_OPT_SET(options, COLOR_DIFF); - DIFF_OPT_SET(options, COLOR_DIFF_WORDS); + options->word_diff = DIFF_WORDS_COLOR; options->word_regex = arg + 14; } + else if (!strcmp(arg, "--word-diff")) { + if (options->word_diff == DIFF_WORDS_NONE) + options->word_diff = DIFF_WORDS_AUTO; + } + else if (!prefixcmp(arg, "--word-diff=")) { + const char *type = arg + 12; + if (!strcmp(type, "auto")) + options->word_diff = DIFF_WORDS_AUTO; + else if (!strcmp(type, "plain")) + options->word_diff = DIFF_WORDS_PLAIN; + else if (!strcmp(type, "color")) { + DIFF_OPT_SET(options, COLOR_DIFF); + options->word_diff = DIFF_WORDS_COLOR; + } + else if (!strcmp(type, "porcelain")) + options->word_diff = DIFF_WORDS_PORCELAIN; + else if (!strcmp(type, "none")) + options->word_diff = DIFF_WORDS_NONE; + else + die("bad --word-diff argument: %s", type); + } + else if (!prefixcmp(arg, "--word-diff-regex=")) { + if (options->word_diff == DIFF_WORDS_NONE) + options->word_diff = DIFF_WORDS_AUTO; + options->word_regex = arg + 18; + } else if (!strcmp(arg, "--exit-code")) DIFF_OPT_SET(options, EXIT_WITH_STATUS); else if (!strcmp(arg, "--quiet")) diff --git a/diff.h b/diff.h index 6a71013..7c8e3ff 100644 --- a/diff.h +++ b/diff.h @@ -54,7 +54,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q, #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6) #define DIFF_OPT_FOLLOW_RENAMES (1 << 7) #define DIFF_OPT_COLOR_DIFF (1 << 8) -#define DIFF_OPT_COLOR_DIFF_WORDS (1 << 9) +/* (1 << 9) unused */ #define DIFF_OPT_HAS_CHANGES (1 << 10) #define DIFF_OPT_QUICK (1 << 11) #define DIFF_OPT_NO_INDEX (1 << 12) @@ -79,6 +79,14 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q, #define DIFF_XDL_SET(opts, flag) ((opts)->xdl_opts |= XDF_##flag) #define DIFF_XDL_CLR(opts, flag) ((opts)->xdl_opts &= ~XDF_##flag) +enum diff_words_type { + DIFF_WORDS_NONE = 0, + DIFF_WORDS_AUTO, + DIFF_WORDS_PORCELAIN, + DIFF_WORDS_PLAIN, + DIFF_WORDS_COLOR +}; + struct diff_options { const char *filter; const char *orderfile; @@ -108,6 +116,7 @@ struct diff_options { int stat_width; int stat_name_width; const char *word_regex; + enum diff_words_type word_diff; /* this is set by diffcore for DIFF_FORMAT_PATCH */ int found_changes; diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh index 2e2e103..764368d 100755 --- a/t/t4034-diff-words.sh +++ b/t/t4034-diff-words.sh @@ -55,6 +55,72 @@ test_expect_success 'word diff with runs of whitespace' ' ' +test_expect_success '--word-diff=color' ' + + word_diff --word-diff=color + +' + +test_expect_success '--color --word-diff=color' ' + + word_diff --color --word-diff=auto + +' + +sed 's/#.*$//' > 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] +~ + # significant space +~ + a = b + c +~ +~ ++aa = a +~ +~ ++aeff = aeff * ( aaa ) +~ +EOF + +test_expect_success '--word-diff=porcelain' ' + + word_diff --word-diff=porcelain + +' + +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=plain' ' + + word_diff --word-diff=plain + +' + +test_expect_success '--no-color --word-diff=color' ' + + word_diff --no-color --word-diff=auto + +' + cat > expect <<\EOF <WHITE>diff --git a/pre b/post<RESET> <WHITE>index 330b04f..5ed8eff 100644<RESET> @@ -143,6 +209,10 @@ test_expect_success 'command-line overrides config' ' word_diff --color-words="[a-z]+" ' +test_expect_success 'command-line overrides config: --word-diff-regex' ' + word_diff --color --word-diff-regex="[a-z]+" +' + cp expect.non-whitespace-is-word expect test_expect_success '.gitattributes override config' ' -- 1.7.0.3.521.ga1e9e ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] diff: add --word-diff option that generalizes --color-words 2010-04-04 13:46 ` [PATCH 1/2] diff: add --word-diff option that generalizes --color-words Thomas Rast @ 2010-04-05 2:06 ` Junio C Hamano 2010-04-05 10:20 ` Thomas Rast 0 siblings, 1 reply; 30+ messages in thread From: Junio C Hamano @ 2010-04-05 2:06 UTC (permalink / raw) To: Thomas Rast Cc: git, Johannes Schindelin, Eelis van der Weegen, Paul Mackerras, Miles Bader Thomas Rast <trast@student.ethz.ch> writes: > This teaches the --color-words engine a more general interface that > supports two new modes: > > * --word-diff=plain, inspired by the 'wdiff' utility (most similar to > 'wdiff -n <old> <new>'): uses delimiters [-removed-] and {+added+} > > * --word-diff=porcelain, which generates an ad-hoc machine readable > format: > - each diff unit is prefixed by [-+ ] and terminated by newline as > in unified diff > - newlines in the input are output as a line consisting only of a > tilde '~' Thanks. This was a fun feature to look at. I think it is a bug that "git show --word-diff" gives the colored format output when I have "color.ui" configuration. Even though I may have "color.ui = auto" in the configuration, I am telling the command to do a --word-diff, not --color-words, from the command line, and that should override color.ui settings. So I think a request for --word-diff that is not --word-diff=color should never use the --color-words _unless_ there is --color on the command line: git show --word-diff git show --word-diff=plain git show --word-diff=porcelain All of the above may paint hunk headers and metainfo the usual way when I have "color.ui" set, but I do not want them to be painting the diff part like --color-words does. I am fine if "porcelain" did not to paint the metainfo, but I see this feature as three different output types of how word diff is presented, so in that sense, it probably is better to force scripts to explicitly ask for no-color, i.e. git show --no-color --word-diff=porcelain if they want to read and interpret metainfo. When the command line asks for color explicitly, then we should see the good old --color-words: git show --color-words git show --word-diff=color I am not sure what the following two should do. One could argue that these default to --color-words; or --color should apply only to the coloring of metainfo but not the diff part: git show --color --word-diff git show --word-diff --color I am slightly in favor of doing the same as --color-words, but people may have different opinions. The following of course should show the plain word diff but the metainfo and friends colored: git show --word-diff=plain --color git show --color --word-diff=plain It might be just the matter of defaulting --word-diff without "=<type>" not to "auto" but to "plain". I haven't looked at the code closely yet. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] diff: add --word-diff option that generalizes --color-words 2010-04-05 2:06 ` Junio C Hamano @ 2010-04-05 10:20 ` Thomas Rast 2010-04-05 18:46 ` Junio C Hamano 0 siblings, 1 reply; 30+ messages in thread From: Thomas Rast @ 2010-04-05 10:20 UTC (permalink / raw) To: Junio C Hamano Cc: git, Johannes Schindelin, Eelis van der Weegen, Paul Mackerras, Miles Bader Junio C Hamano wrote: > > I think it is a bug that "git show --word-diff" gives the colored format > output when I have "color.ui" configuration. > > Even though I may have "color.ui = auto" in the configuration, I am > telling the command to do a --word-diff, not --color-words, from the > command line, and that should override color.ui settings. I don't really see why the user would forfeit the convenience and readability of a color-words diff when the terminal supports colors. Granted, this is probably the first instance where the colors actually change the output format instead of just making easier to read, but still? But: > It might be just the matter of defaulting --word-diff without "=<type>" > not to "auto" but to "plain". I haven't looked at the code closely yet. Yes. -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] diff: add --word-diff option that generalizes --color-words 2010-04-05 10:20 ` Thomas Rast @ 2010-04-05 18:46 ` Junio C Hamano 2010-04-05 18:53 ` Miles Bader 2010-04-06 9:20 ` [PATCH 1/2] diff: add --word-diff option that generalizes --color-words Thomas Rast 0 siblings, 2 replies; 30+ messages in thread From: Junio C Hamano @ 2010-04-05 18:46 UTC (permalink / raw) To: Thomas Rast Cc: git, Johannes Schindelin, Eelis van der Weegen, Paul Mackerras, Miles Bader Thomas Rast <trast@student.ethz.ch> writes: >> I think it is a bug that "git show --word-diff" gives the colored format >> output when I have "color.ui" configuration. >> >> Even though I may have "color.ui = auto" in the configuration, I am >> telling the command to do a --word-diff, not --color-words, from the >> command line, and that should override color.ui settings. > > I don't really see why the user would forfeit the convenience and > readability of a color-words diff when the terminal supports colors. There is one difference between other uses of colors and color-words, but I can imagine that ordinary people may not have even realized nor thought about it. To people who are somewhat but not completely color-challenged (like myself), it still helps to paint hunk headers in a color that is different from the body text to make the boundary of each hunk more visible. Even without the perception of exact color/hue, the contrast alone helps in that case. The body of the diff is painted in deleted and added colors; the color is used only as a way to additionally enhance the output, while still keeping the plus/minus/SP at the leftmost column. Even to somebody who is completely color challenged, the necessary information is still there. Compare --color-words with these. The output uses color as the _only_ way to say which words appear before and after. The contrast among the three colors used for the plain body, deleted and added text may still help, or it may not, to a partially color challenged person. I also happen to see "--word-diff=color" more as just a customization to the "plain" output formatting to say "use these byte sequences that happen to be ANSI color codes, instead of '{+', '+}', etc." than as a part of what the ui.color switch wants to specify, which is "are various parts of the output colored to further help distinguishing them visually?" So color me somewhat biased, but there is a case where the suggestion in the message you are responding to makes a difference. I also think --color --word-diff=plain could show "{+new+}" in green and "[-old-]" in red and that may make things more consistent. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] diff: add --word-diff option that generalizes --color-words 2010-04-05 18:46 ` Junio C Hamano @ 2010-04-05 18:53 ` Miles Bader 2010-04-12 13:07 ` [PATCH v3 0/2] gitk --color-words Thomas Rast 2010-04-06 9:20 ` [PATCH 1/2] diff: add --word-diff option that generalizes --color-words Thomas Rast 1 sibling, 1 reply; 30+ messages in thread From: Miles Bader @ 2010-04-05 18:53 UTC (permalink / raw) To: Junio C Hamano Cc: Thomas Rast, git, Johannes Schindelin, Eelis van der Weegen, Paul Mackerras On Tue, Apr 6, 2010 at 3:46 AM, Junio C Hamano <gitster@pobox.com> wrote: > I also think --color --word-diff=plain could show "{+new+}" in green and > "[-old-]" in red and that may make things more consistent. I agree. For some reason, even though I can see the red and green well enough, I find the current --color-words output hard to parse. The use of _only_ color to distinguish old/new somehow doesn't jive with my brain... I find _highlighting_ using color very useful, but I think using syntax and color together is better than using just color. -Miles -- Do not taunt Happy Fun Ball. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 0/2] gitk --color-words 2010-04-05 18:53 ` Miles Bader @ 2010-04-12 13:07 ` Thomas Rast 2010-04-12 13:07 ` [PATCH v3 1/3] diff: add --word-diff option that generalizes --color-words Thomas Rast ` (2 more replies) 0 siblings, 3 replies; 30+ messages in thread From: Thomas Rast @ 2010-04-12 13:07 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader, Jens Lehmann During development an unrelated gitk diff parsing bug hit me, which is the new 2/3. I think it should go in maint if the patch turns out to be correct, but I'd like to have an ACK from Jens first. Junio C Hamano wrote: > I also think --color --word-diff=plain could show "{+new+}" in green and > "[-old-]" in red and that may make things more consistent. So here's a patch that does that. I may be overdoing the "generic diff style" code a bit, but it makes for slightly nicer code. I similarly extended the support in gitk to have two different modes. My use of the dropdown is again sheer voodoo and works merely by accident... I also taught gitk to not show the option and not parse the command-line options unless the git version is at least v1.7.2, which I expect will be the version that has the underlying diff support. BTW: Miles Bader wrote: > For some reason, even though I can see the red and green well enough, > I find the current --color-words output hard to parse. > The use of _only_ color to distinguish old/new somehow doesn't jive > with my brain... I set my diff colors to bold red/blue which makes the changed words very visible even in light conditions where green is very hard to tell from black (sitting outside in the sun or so). > I find _highlighting_ using color very useful, but I think using > syntax and color together is better than using just color. Well, I for one find the extra markup *very* confusing because I need to mentally untangle it from the actual content... Thomas Rast (3): diff: add --word-diff option that generalizes --color-words gitk: do not parse " >" context as submodule change gitk: add the equivalent of diff --color-words ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 1/3] diff: add --word-diff option that generalizes --color-words 2010-04-12 13:07 ` [PATCH v3 0/2] gitk --color-words Thomas Rast @ 2010-04-12 13:07 ` Thomas Rast 2010-04-12 16:31 ` Junio C Hamano 2010-04-12 13:07 ` [PATCH v3 2/3] gitk: do not parse " >" context as submodule change Thomas Rast 2010-04-12 13:07 ` [PATCH v3 3/3] gitk: add the equivalent of diff --color-words Thomas Rast 2 siblings, 1 reply; 30+ messages in thread From: Thomas Rast @ 2010-04-12 13:07 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader, Jens Lehmann This teaches the --color-words engine a more general interface that supports two new modes: * --word-diff=plain, inspired by the 'wdiff' utility (most similar to 'wdiff -n <old> <new>'): uses delimiters [-removed-] and {+added+} * --word-diff=porcelain, which generates an ad-hoc machine readable format: - each diff unit is prefixed by [-+ ] and terminated by newline as in unified diff - newlines in the input are output as a line consisting only of a tilde '~' Both of these formats still support color if it is enabled, using it to highlight the differences. --color-words becomes a synonym for --word-diff=color, which is the color-only format. Also adds some compatibility/convenience options. Thanks to Junio C Hamano and Miles Bader for good ideas. Signed-off-by: Thomas Rast <trast@student.ethz.ch> --- Documentation/diff-options.txt | 40 +++++++++- Documentation/gitattributes.txt | 2 +- color.c | 28 ------- color.h | 1 - diff.c | 154 ++++++++++++++++++++++++++++++++++----- diff.h | 10 ++- t/t4034-diff-words.sh | 122 +++++++++++++++++++++++++++++++ 7 files changed, 303 insertions(+), 54 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 60e922e..a616ca5 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -126,11 +126,39 @@ any of those replacements occurred. gives the default to color output. Same as `--color=never`. ---color-words[=<regex>]:: - Show colored word diff, i.e., color words which have changed. - By default, words are separated by whitespace. +--word-diff[=<mode>]:: + Show a word diff, using the <mode> to delimit changed words. + By default, words are delimited by whitespace; see + `--word-diff-regex` below. The <mode> defaults to 'plain', and + must be one of: ++ +-- +color:: + Highlight changed words using only colors. Implies `--color`. +plain:: + Show words as `[-removed-]` and `{+added+}`. Makes no + attempts to escape the delimiters if they appear in the input, + so the output may be ambiguous. +porcelain:: + Use a special line-based format intended for script + consumption. Added/removed/unchanged runs are printed in the + usual unified diff format, starting with a `+`/`-`/` ` + character at the beginning of the line and extending to the + end of the line. Newlines in the input are represented by a + tilde `~` on a line of its own. +none:: + Disable word diff again. +-- ++ +Note that despite the name of the first mode, color is used to +highlight the changed parts in all modes if enabled. + +--word-diff-regex=<regex>:: + Use <regex> to decide what a word is, instead of considering + runs of non-whitespace to be a word. Also implies + `--word-diff` unless it was already enabled. + -When a <regex> is specified, every non-overlapping match of the +Every non-overlapping match of the <regex> is considered a word. Anything between these matches is considered whitespace and ignored(!) for the purposes of finding differences. You may want to append `|[^[:space:]]` to your regular @@ -142,6 +170,10 @@ The regex can also be set via a diff driver or configuration option, see linkgit:gitattributes[1] or linkgit:git-config[1]. Giving it explicitly overrides any diff driver or configuration setting. Diff drivers override configuration settings. + +--color-words[=<regex>]:: + Equivalent to `--word-diff=color` plus (if a regex was + specified) `--word-diff-regex=<regex>`. endif::git-format-patch[] --no-renames:: diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index a8500d1..0523a57 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -360,7 +360,7 @@ patterns are available: Customizing word diff ^^^^^^^^^^^^^^^^^^^^^ -You can customize the rules that `git diff --color-words` uses to +You can customize the rules that `git diff --word-diff` uses to split words in a line, by specifying an appropriate regular expression in the "diff.*.wordRegex" configuration variable. For example, in TeX a backslash followed by a sequence of letters forms a command, but diff --git a/color.c b/color.c index bcf4e2c..1b00554 100644 --- a/color.c +++ b/color.c @@ -211,31 +211,3 @@ int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...) va_end(args); return r; } - -/* - * This function splits the buffer by newlines and colors the lines individually. - * - * Returns 0 on success. - */ -int color_fwrite_lines(FILE *fp, const char *color, - size_t count, const char *buf) -{ - if (!*color) - return fwrite(buf, count, 1, fp) != 1; - while (count) { - char *p = memchr(buf, '\n', count); - if (p != buf && (fputs(color, fp) < 0 || - fwrite(buf, p ? p - buf : count, 1, fp) != 1 || - fputs(GIT_COLOR_RESET, fp) < 0)) - return -1; - if (!p) - return 0; - if (fputc('\n', fp) < 0) - return -1; - count -= p + 1 - buf; - buf = p + 1; - } - return 0; -} - - diff --git a/color.h b/color.h index 5c264b0..03ca064 100644 --- a/color.h +++ b/color.h @@ -61,6 +61,5 @@ int color_fprintf(FILE *fp, const char *color, const char *fmt, ...); __attribute__((format (printf, 3, 4))) int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...); -int color_fwrite_lines(FILE *fp, const char *color, size_t count, const char *buf); #endif /* COLOR_H */ diff --git a/diff.c b/diff.c index 7effdac..ca054d4 100644 --- a/diff.c +++ b/diff.c @@ -560,16 +560,83 @@ static void diff_words_append(char *line, unsigned long len, buffer->text.ptr[buffer->text.size] = '\0'; } +struct diff_words_style_elem +{ + const char *prefix; + const char *suffix; + const char *color; /* NULL; filled in by the setup code if + * color is enabled */ +}; + +struct diff_words_style +{ + enum diff_words_type type; + struct diff_words_style_elem new, old, ctx; + const char *newline; +}; + +struct diff_words_style diff_words_styles[] = { + { DIFF_WORDS_PORCELAIN, + {"+", "\n", NULL}, + {"-", "\n", NULL}, + {" ", "\n", NULL}, + "~\n" + }, + { DIFF_WORDS_PLAIN, + {"{+", "+}", NULL}, + {"[-", "-]", NULL}, + {"", "", NULL}, + "\n" + }, + { DIFF_WORDS_COLOR, + {"", "", NULL}, + {"", "", NULL}, + {"", "", NULL}, + "\n" + } +}; + struct diff_words_data { struct diff_words_buffer minus, plus; const char *current_plus; FILE *file; regex_t *word_regex; + enum diff_words_type type; + struct diff_words_style *style; }; +static int fn_out_diff_words_write_helper(FILE *fp, + struct diff_words_style_elem st_el, + const char *newline, + size_t count, const char *buf) +{ + while (count) { + char *p = memchr(buf, '\n', count); + if (p != buf) { + if (st_el.color && fputs(st_el.color, fp) < 0) + return -1; + if (fputs(st_el.prefix, fp) < 0 || + fwrite(buf, p ? p - buf : count, 1, fp) != 1 || + fputs(st_el.suffix, fp) < 0) + return -1; + if (st_el.color && strlen(st_el.color) + && fputs(GIT_COLOR_RESET, fp) < 0) + return -1; + } + if (!p) + return 0; + if (fputs(newline, fp) < 0) + return -1; + count -= p + 1 - buf; + buf = p + 1; + } + return 0; +} + static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len) { struct diff_words_data *diff_words = priv; + struct diff_words_style *style = diff_words->style; int minus_first, minus_len, plus_first, plus_len; const char *minus_begin, *minus_end, *plus_begin, *plus_end; @@ -593,16 +660,17 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len) plus_begin = plus_end = diff_words->plus.orig[plus_first].end; if (diff_words->current_plus != plus_begin) - fwrite(diff_words->current_plus, - plus_begin - diff_words->current_plus, 1, - diff_words->file); + fn_out_diff_words_write_helper(diff_words->file, + style->ctx, style->newline, + plus_begin - diff_words->current_plus, + diff_words->current_plus); if (minus_begin != minus_end) - color_fwrite_lines(diff_words->file, - diff_get_color(1, DIFF_FILE_OLD), + fn_out_diff_words_write_helper(diff_words->file, + style->old, style->newline, minus_end - minus_begin, minus_begin); if (plus_begin != plus_end) - color_fwrite_lines(diff_words->file, - diff_get_color(1, DIFF_FILE_NEW), + fn_out_diff_words_write_helper(diff_words->file, + style->new, style->newline, plus_end - plus_begin, plus_begin); diff_words->current_plus = plus_end; @@ -685,11 +753,12 @@ static void diff_words_show(struct diff_words_data *diff_words) xdemitconf_t xecfg; xdemitcb_t ecb; mmfile_t minus, plus; + struct diff_words_style *style = diff_words->style; /* special case: only removal */ if (!diff_words->plus.text.size) { - color_fwrite_lines(diff_words->file, - diff_get_color(1, DIFF_FILE_OLD), + fn_out_diff_words_write_helper(diff_words->file, + style->old, style->newline, diff_words->minus.text.size, diff_words->minus.text.ptr); diff_words->minus.text.size = 0; return; @@ -710,10 +779,10 @@ static void diff_words_show(struct diff_words_data *diff_words) free(plus.ptr); if (diff_words->current_plus != diff_words->plus.text.ptr + diff_words->plus.text.size) - fwrite(diff_words->current_plus, + fn_out_diff_words_write_helper(diff_words->file, + style->ctx, style->newline, diff_words->plus.text.ptr + diff_words->plus.text.size - - diff_words->current_plus, 1, - diff_words->file); + - diff_words->current_plus, diff_words->current_plus); diff_words->minus.text.size = diff_words->plus.text.size = 0; } @@ -825,6 +894,9 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) if (len < 1) { emit_line(ecbdata->file, reset, reset, line, len); + if (ecbdata->diff_words + && ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) + fputs("~\n", ecbdata->file); return; } @@ -839,9 +911,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) return; } diff_words_flush(ecbdata); - line++; - len--; - emit_line(ecbdata->file, plain, reset, line, len); + if (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) { + emit_line(ecbdata->file, plain, reset, line, len); + fputs("~\n", ecbdata->file); + } else { + /* don't print the prefix character */ + emit_line(ecbdata->file, plain, reset, line+1, len-1); + } return; } @@ -1739,10 +1815,13 @@ 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 (DIFF_OPT_TST(o, COLOR_DIFF_WORDS)) { + if (o->word_diff) { + int i; + ecbdata.diff_words = xcalloc(1, sizeof(struct diff_words_data)); ecbdata.diff_words->file = o->file; + ecbdata.diff_words->type = o->word_diff; if (!o->word_regex) o->word_regex = userdiff_word_regex(one); if (!o->word_regex) @@ -1758,10 +1837,23 @@ static void builtin_diff(const char *name_a, 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 (DIFF_OPT_TST(o, COLOR_DIFF)) { + 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); + } } xdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata, &xpp, &xecfg, &ecb); - if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS)) + if (o->word_diff) free_diff_words_data(&ecbdata); if (textconv_one) free(mf1.ptr); @@ -2830,13 +2922,37 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) DIFF_OPT_CLR(options, COLOR_DIFF); else if (!strcmp(arg, "--color-words")) { DIFF_OPT_SET(options, COLOR_DIFF); - DIFF_OPT_SET(options, COLOR_DIFF_WORDS); + options->word_diff = DIFF_WORDS_COLOR; } else if (!prefixcmp(arg, "--color-words=")) { DIFF_OPT_SET(options, COLOR_DIFF); - DIFF_OPT_SET(options, COLOR_DIFF_WORDS); + options->word_diff = DIFF_WORDS_COLOR; options->word_regex = arg + 14; } + else if (!strcmp(arg, "--word-diff")) { + if (options->word_diff == DIFF_WORDS_NONE) + options->word_diff = DIFF_WORDS_PLAIN; + } + else if (!prefixcmp(arg, "--word-diff=")) { + const char *type = arg + 12; + if (!strcmp(type, "plain")) + options->word_diff = DIFF_WORDS_PLAIN; + else if (!strcmp(type, "color")) { + DIFF_OPT_SET(options, COLOR_DIFF); + options->word_diff = DIFF_WORDS_COLOR; + } + else if (!strcmp(type, "porcelain")) + options->word_diff = DIFF_WORDS_PORCELAIN; + else if (!strcmp(type, "none")) + options->word_diff = DIFF_WORDS_NONE; + else + die("bad --word-diff argument: %s", type); + } + else if (!prefixcmp(arg, "--word-diff-regex=")) { + if (options->word_diff == DIFF_WORDS_NONE) + options->word_diff = DIFF_WORDS_PLAIN; + options->word_regex = arg + 18; + } else if (!strcmp(arg, "--exit-code")) DIFF_OPT_SET(options, EXIT_WITH_STATUS); else if (!strcmp(arg, "--quiet")) diff --git a/diff.h b/diff.h index 6a71013..9ace08c 100644 --- a/diff.h +++ b/diff.h @@ -54,7 +54,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q, #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6) #define DIFF_OPT_FOLLOW_RENAMES (1 << 7) #define DIFF_OPT_COLOR_DIFF (1 << 8) -#define DIFF_OPT_COLOR_DIFF_WORDS (1 << 9) +/* (1 << 9) unused */ #define DIFF_OPT_HAS_CHANGES (1 << 10) #define DIFF_OPT_QUICK (1 << 11) #define DIFF_OPT_NO_INDEX (1 << 12) @@ -79,6 +79,13 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q, #define DIFF_XDL_SET(opts, flag) ((opts)->xdl_opts |= XDF_##flag) #define DIFF_XDL_CLR(opts, flag) ((opts)->xdl_opts &= ~XDF_##flag) +enum diff_words_type { + DIFF_WORDS_NONE = 0, + DIFF_WORDS_PORCELAIN, + DIFF_WORDS_PLAIN, + DIFF_WORDS_COLOR +}; + struct diff_options { const char *filter; const char *orderfile; @@ -108,6 +115,7 @@ struct diff_options { int stat_width; int stat_name_width; const char *word_regex; + enum diff_words_type word_diff; /* this is set by diffcore for DIFF_FORMAT_PATCH */ int found_changes; diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh index 2e2e103..6f7548c 100755 --- a/t/t4034-diff-words.sh +++ b/t/t4034-diff-words.sh @@ -55,6 +55,93 @@ test_expect_success 'word diff with runs of whitespace' ' ' +test_expect_success '--word-diff=color' ' + + word_diff --word-diff=color + +' + +test_expect_success '--color --word-diff=color' ' + + word_diff --color --word-diff=color + +' + +sed 's/#.*$//' > 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] +~ + # significant space +~ + a = b + c +~ +~ ++aa = a +~ +~ ++aeff = aeff * ( aaa ) +~ +EOF + +test_expect_success '--word-diff=porcelain' ' + + word_diff --word-diff=porcelain + +' + +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=plain' ' + + word_diff --word-diff=plain + +' + +test_expect_success '--word-diff=plain --no-color' ' + + word_diff --word-diff=plain --no-color + +' + +cat > expect <<EOF +<WHITE>diff --git a/pre b/post<RESET> +<WHITE>index 330b04f..5ed8eff 100644<RESET> +<WHITE>--- a/pre<RESET> +<WHITE>+++ b/post<RESET> +<CYAN>@@ -1,3 +1,7 @@<RESET> +<RED>[-h(4)-]<RESET><GREEN>{+h(4),hh[44]+}<RESET> + +a = b + c<RESET> + +<GREEN>{+aa = a+}<RESET> + +<GREEN>{+aeff = aeff * ( aaa )+}<RESET> +EOF + +test_expect_success '--word-diff=plain --color' ' + + word_diff --word-diff=plain --color + +' + cat > expect <<\EOF <WHITE>diff --git a/pre b/post<RESET> <WHITE>index 330b04f..5ed8eff 100644<RESET> @@ -143,6 +230,25 @@ test_expect_success 'command-line overrides config' ' word_diff --color-words="[a-z]+" ' +cat > expect <<\EOF +<WHITE>diff --git a/pre b/post<RESET> +<WHITE>index 330b04f..5ed8eff 100644<RESET> +<WHITE>--- a/pre<RESET> +<WHITE>+++ b/post<RESET> +<CYAN>@@ -1,3 +1,7 @@<RESET> +h(4),<GREEN>{+hh+}<RESET>[44] + +a = b + c<RESET> + +<GREEN>{+aa = a+}<RESET> + +<GREEN>{+aeff = aeff * ( aaa+}<RESET> ) +EOF + +test_expect_success 'command-line overrides config: --word-diff-regex' ' + word_diff --color --word-diff-regex="[a-z]+" +' + cp expect.non-whitespace-is-word expect test_expect_success '.gitattributes override config' ' @@ -209,4 +315,20 @@ test_expect_success 'test when words are only removed at the end' ' ' +cat > expect <<\EOF +diff --git a/pre b/post +index 289cb9d..2d06f37 100644 +--- a/pre ++++ b/post +@@ -1 +1 @@ +-(: ++( +EOF + +test_expect_success '--word-diff=none' ' + + word_diff --word-diff=plain --word-diff=none + +' + test_done -- 1.7.1.rc0.262.gff1a3 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/3] diff: add --word-diff option that generalizes --color-words 2010-04-12 13:07 ` [PATCH v3 1/3] diff: add --word-diff option that generalizes --color-words Thomas Rast @ 2010-04-12 16:31 ` Junio C Hamano 2010-04-13 9:27 ` Thomas Rast 2010-04-14 15:59 ` [PATCH v4 0/3] git-diff --word-diff/gitk --color-words Thomas Rast 0 siblings, 2 replies; 30+ messages in thread From: Junio C Hamano @ 2010-04-12 16:31 UTC (permalink / raw) To: Thomas Rast Cc: git, Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader, Jens Lehmann Thomas Rast <trast@student.ethz.ch> writes: > diff --git a/diff.c b/diff.c > index 7effdac..ca054d4 100644 > --- a/diff.c > +++ b/diff.c > @@ -560,16 +560,83 @@ static void diff_words_append(char *line, unsigned long len, > buffer->text.ptr[buffer->text.size] = '\0'; > } > > +struct diff_words_style_elem > +{ > + const char *prefix; > + const char *suffix; > + const char *color; /* NULL; filled in by the setup code if > + * color is enabled */ > +}; The reader makes a mental note that this is a 12- to 24-byte structure... > +struct diff_words_style > +{ > + enum diff_words_type type; > + struct diff_words_style_elem new, old, ctx; > + const char *newline; > +}; > + > +struct diff_words_style diff_words_styles[] = { > + { DIFF_WORDS_PORCELAIN, > + {"+", "\n", NULL}, > + {"-", "\n", NULL}, > + {" ", "\n", NULL}, > + "~\n" > + }, > + { DIFF_WORDS_PLAIN, > + {"{+", "+}", NULL}, > + {"[-", "-]", NULL}, > + {"", "", NULL}, > + "\n" > + }, > + { DIFF_WORDS_COLOR, > + {"", "", NULL}, > + {"", "", NULL}, > + {"", "", NULL}, > + "\n" > + } > +}; Beautiful. The style might be a bit iffy. Shouldn't an opening "{", unless closed on the same line with a matching "}", stand on its own line? > +static int fn_out_diff_words_write_helper(FILE *fp, > + struct diff_words_style_elem st_el, Do you need to pass this by value? > + const char *newline, > + size_t count, const char *buf) > +{ > + while (count) { > + char *p = memchr(buf, '\n', count); > + if (p != buf) { > + if (st_el.color && fputs(st_el.color, fp) < 0) > + return -1; > + if (fputs(st_el.prefix, fp) < 0 || > + fwrite(buf, p ? p - buf : count, 1, fp) != 1 || > + fputs(st_el.suffix, fp) < 0) > + return -1; > + if (st_el.color && strlen(st_el.color) I would avoid strlen() only for boolean result here---the compilers may not be as clever as you are. if (st_elp->color && *st_elp->color ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/3] diff: add --word-diff option that generalizes --color-words 2010-04-12 16:31 ` Junio C Hamano @ 2010-04-13 9:27 ` Thomas Rast 2010-04-14 15:59 ` [PATCH v4 0/3] git-diff --word-diff/gitk --color-words Thomas Rast 1 sibling, 0 replies; 30+ messages in thread From: Thomas Rast @ 2010-04-13 9:27 UTC (permalink / raw) To: Junio C Hamano Cc: git, Johannes Schindelin, Eelis van der Weegen, Paul Mackerras, Miles Bader, Jens Lehmann Junio C Hamano wrote: > Thomas Rast <trast@student.ethz.ch> writes: [...] > > + { DIFF_WORDS_COLOR, > > + {"", "", NULL}, > > + {"", "", NULL}, > > + {"", "", NULL}, > > + "\n" > > + } > > +}; > > Beautiful. > > The style might be a bit iffy. Shouldn't an opening "{", unless closed on > the same line with a matching "}", stand on its own line? Perhaps. OTOH I just noticed I could also drop the NULL (and let the implicit 0-padding take over) which makes every style fit on a single line. That should make it a bit easier to read. > > +static int fn_out_diff_words_write_helper(FILE *fp, > > + struct diff_words_style_elem st_el, > > Do you need to pass this by value? No, thanks for catching this. -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 0/3] git-diff --word-diff/gitk --color-words 2010-04-12 16:31 ` Junio C Hamano 2010-04-13 9:27 ` Thomas Rast @ 2010-04-14 15:59 ` Thomas Rast 2010-04-14 15:59 ` [PATCH v4 1/3] diff: add --word-diff option that generalizes --color-words Thomas Rast ` (2 more replies) 1 sibling, 3 replies; 30+ messages in thread From: Thomas Rast @ 2010-04-14 15:59 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader, Jens Lehmann This merely addresses Junio's comments on 1/3 of the last round, since I haven't heard anything about the gitk patches. Thomas Rast (3): diff: add --word-diff option that generalizes --color-words gitk: do not parse " >" context as submodule change gitk: add the equivalent of diff --color-words ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/3] diff: add --word-diff option that generalizes --color-words 2010-04-14 15:59 ` [PATCH v4 0/3] git-diff --word-diff/gitk --color-words Thomas Rast @ 2010-04-14 15:59 ` Thomas Rast 2010-04-14 21:23 ` Junio C Hamano 2010-04-14 15:59 ` [PATCH v4 2/3] gitk: do not parse " >" context as submodule change Thomas Rast 2010-04-14 15:59 ` [PATCH v4 3/3] gitk: add the equivalent of diff --color-words Thomas Rast 2 siblings, 1 reply; 30+ messages in thread From: Thomas Rast @ 2010-04-14 15:59 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader, Jens Lehmann This teaches the --color-words engine a more general interface that supports two new modes: * --word-diff=plain, inspired by the 'wdiff' utility (most similar to 'wdiff -n <old> <new>'): uses delimiters [-removed-] and {+added+} * --word-diff=porcelain, which generates an ad-hoc machine readable format: - each diff unit is prefixed by [-+ ] and terminated by newline as in unified diff - newlines in the input are output as a line consisting only of a tilde '~' Both of these formats still support color if it is enabled, using it to highlight the differences. --color-words becomes a synonym for --word-diff=color, which is the color-only format. Also adds some compatibility/convenience options. Thanks to Junio C Hamano and Miles Bader for good ideas. Signed-off-by: Thomas Rast <trast@student.ethz.ch> --- Documentation/diff-options.txt | 40 ++++++++++- Documentation/gitattributes.txt | 2 +- color.c | 28 -------- color.h | 1 - diff.c | 139 +++++++++++++++++++++++++++++++++----- diff.h | 10 +++- t/t4034-diff-words.sh | 122 ++++++++++++++++++++++++++++++++++ 7 files changed, 288 insertions(+), 54 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 60e922e..a616ca5 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -126,11 +126,39 @@ any of those replacements occurred. gives the default to color output. Same as `--color=never`. ---color-words[=<regex>]:: - Show colored word diff, i.e., color words which have changed. - By default, words are separated by whitespace. +--word-diff[=<mode>]:: + Show a word diff, using the <mode> to delimit changed words. + By default, words are delimited by whitespace; see + `--word-diff-regex` below. The <mode> defaults to 'plain', and + must be one of: ++ +-- +color:: + Highlight changed words using only colors. Implies `--color`. +plain:: + Show words as `[-removed-]` and `{+added+}`. Makes no + attempts to escape the delimiters if they appear in the input, + so the output may be ambiguous. +porcelain:: + Use a special line-based format intended for script + consumption. Added/removed/unchanged runs are printed in the + usual unified diff format, starting with a `+`/`-`/` ` + character at the beginning of the line and extending to the + end of the line. Newlines in the input are represented by a + tilde `~` on a line of its own. +none:: + Disable word diff again. +-- ++ +Note that despite the name of the first mode, color is used to +highlight the changed parts in all modes if enabled. + +--word-diff-regex=<regex>:: + Use <regex> to decide what a word is, instead of considering + runs of non-whitespace to be a word. Also implies + `--word-diff` unless it was already enabled. + -When a <regex> is specified, every non-overlapping match of the +Every non-overlapping match of the <regex> is considered a word. Anything between these matches is considered whitespace and ignored(!) for the purposes of finding differences. You may want to append `|[^[:space:]]` to your regular @@ -142,6 +170,10 @@ The regex can also be set via a diff driver or configuration option, see linkgit:gitattributes[1] or linkgit:git-config[1]. Giving it explicitly overrides any diff driver or configuration setting. Diff drivers override configuration settings. + +--color-words[=<regex>]:: + Equivalent to `--word-diff=color` plus (if a regex was + specified) `--word-diff-regex=<regex>`. endif::git-format-patch[] --no-renames:: diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index a8500d1..0523a57 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -360,7 +360,7 @@ patterns are available: Customizing word diff ^^^^^^^^^^^^^^^^^^^^^ -You can customize the rules that `git diff --color-words` uses to +You can customize the rules that `git diff --word-diff` uses to split words in a line, by specifying an appropriate regular expression in the "diff.*.wordRegex" configuration variable. For example, in TeX a backslash followed by a sequence of letters forms a command, but diff --git a/color.c b/color.c index bcf4e2c..1b00554 100644 --- a/color.c +++ b/color.c @@ -211,31 +211,3 @@ int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...) va_end(args); return r; } - -/* - * This function splits the buffer by newlines and colors the lines individually. - * - * Returns 0 on success. - */ -int color_fwrite_lines(FILE *fp, const char *color, - size_t count, const char *buf) -{ - if (!*color) - return fwrite(buf, count, 1, fp) != 1; - while (count) { - char *p = memchr(buf, '\n', count); - if (p != buf && (fputs(color, fp) < 0 || - fwrite(buf, p ? p - buf : count, 1, fp) != 1 || - fputs(GIT_COLOR_RESET, fp) < 0)) - return -1; - if (!p) - return 0; - if (fputc('\n', fp) < 0) - return -1; - count -= p + 1 - buf; - buf = p + 1; - } - return 0; -} - - diff --git a/color.h b/color.h index 5c264b0..03ca064 100644 --- a/color.h +++ b/color.h @@ -61,6 +61,5 @@ int color_fprintf(FILE *fp, const char *color, const char *fmt, ...); __attribute__((format (printf, 3, 4))) int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...); -int color_fwrite_lines(FILE *fp, const char *color, size_t count, const char *buf); #endif /* COLOR_H */ diff --git a/diff.c b/diff.c index 7effdac..f587b83 100644 --- a/diff.c +++ b/diff.c @@ -560,16 +560,68 @@ static void diff_words_append(char *line, unsigned long len, buffer->text.ptr[buffer->text.size] = '\0'; } +struct diff_words_style_elem +{ + const char *prefix; + const char *suffix; + const char *color; /* NULL; filled in by the setup code if + * color is enabled */ +}; + +struct diff_words_style +{ + enum diff_words_type type; + struct diff_words_style_elem new, old, ctx; + const char *newline; +}; + +struct diff_words_style diff_words_styles[] = { + { DIFF_WORDS_PORCELAIN, {"+", "\n"}, {"-", "\n"}, {" ", "\n"}, "~\n" }, + { DIFF_WORDS_PLAIN, {"{+", "+}"}, {"[-", "-]"}, {"", ""}, "\n" }, + { DIFF_WORDS_COLOR, {"", ""}, {"", ""}, {"", ""}, "\n" } +}; + struct diff_words_data { struct diff_words_buffer minus, plus; const char *current_plus; FILE *file; regex_t *word_regex; + enum diff_words_type type; + struct diff_words_style *style; }; +static int fn_out_diff_words_write_helper(FILE *fp, + struct diff_words_style_elem *st_el, + const char *newline, + size_t count, const char *buf) +{ + while (count) { + char *p = memchr(buf, '\n', count); + if (p != buf) { + if (st_el->color && fputs(st_el->color, fp) < 0) + return -1; + if (fputs(st_el->prefix, fp) < 0 || + fwrite(buf, p ? p - buf : count, 1, fp) != 1 || + fputs(st_el->suffix, fp) < 0) + return -1; + if (st_el->color && *st_el->color + && fputs(GIT_COLOR_RESET, fp) < 0) + return -1; + } + if (!p) + return 0; + if (fputs(newline, fp) < 0) + return -1; + count -= p + 1 - buf; + buf = p + 1; + } + return 0; +} + static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len) { struct diff_words_data *diff_words = priv; + struct diff_words_style *style = diff_words->style; int minus_first, minus_len, plus_first, plus_len; const char *minus_begin, *minus_end, *plus_begin, *plus_end; @@ -593,16 +645,17 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len) plus_begin = plus_end = diff_words->plus.orig[plus_first].end; if (diff_words->current_plus != plus_begin) - fwrite(diff_words->current_plus, - plus_begin - diff_words->current_plus, 1, - diff_words->file); + fn_out_diff_words_write_helper(diff_words->file, + &style->ctx, style->newline, + plus_begin - diff_words->current_plus, + diff_words->current_plus); if (minus_begin != minus_end) - color_fwrite_lines(diff_words->file, - diff_get_color(1, DIFF_FILE_OLD), + fn_out_diff_words_write_helper(diff_words->file, + &style->old, style->newline, minus_end - minus_begin, minus_begin); if (plus_begin != plus_end) - color_fwrite_lines(diff_words->file, - diff_get_color(1, DIFF_FILE_NEW), + fn_out_diff_words_write_helper(diff_words->file, + &style->new, style->newline, plus_end - plus_begin, plus_begin); diff_words->current_plus = plus_end; @@ -685,11 +738,12 @@ static void diff_words_show(struct diff_words_data *diff_words) xdemitconf_t xecfg; xdemitcb_t ecb; mmfile_t minus, plus; + struct diff_words_style *style = diff_words->style; /* special case: only removal */ if (!diff_words->plus.text.size) { - color_fwrite_lines(diff_words->file, - diff_get_color(1, DIFF_FILE_OLD), + fn_out_diff_words_write_helper(diff_words->file, + &style->old, style->newline, diff_words->minus.text.size, diff_words->minus.text.ptr); diff_words->minus.text.size = 0; return; @@ -710,10 +764,10 @@ static void diff_words_show(struct diff_words_data *diff_words) free(plus.ptr); if (diff_words->current_plus != diff_words->plus.text.ptr + diff_words->plus.text.size) - fwrite(diff_words->current_plus, + fn_out_diff_words_write_helper(diff_words->file, + &style->ctx, style->newline, diff_words->plus.text.ptr + diff_words->plus.text.size - - diff_words->current_plus, 1, - diff_words->file); + - diff_words->current_plus, diff_words->current_plus); diff_words->minus.text.size = diff_words->plus.text.size = 0; } @@ -825,6 +879,9 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) if (len < 1) { emit_line(ecbdata->file, reset, reset, line, len); + if (ecbdata->diff_words + && ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) + fputs("~\n", ecbdata->file); return; } @@ -839,9 +896,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) return; } diff_words_flush(ecbdata); - line++; - len--; - emit_line(ecbdata->file, plain, reset, line, len); + if (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) { + emit_line(ecbdata->file, plain, reset, line, len); + fputs("~\n", ecbdata->file); + } else { + /* don't print the prefix character */ + emit_line(ecbdata->file, plain, reset, line+1, len-1); + } return; } @@ -1739,10 +1800,13 @@ 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 (DIFF_OPT_TST(o, COLOR_DIFF_WORDS)) { + if (o->word_diff) { + int i; + ecbdata.diff_words = xcalloc(1, sizeof(struct diff_words_data)); ecbdata.diff_words->file = o->file; + ecbdata.diff_words->type = o->word_diff; if (!o->word_regex) o->word_regex = userdiff_word_regex(one); if (!o->word_regex) @@ -1758,10 +1822,23 @@ static void builtin_diff(const char *name_a, 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 (DIFF_OPT_TST(o, COLOR_DIFF)) { + 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); + } } xdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata, &xpp, &xecfg, &ecb); - if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS)) + if (o->word_diff) free_diff_words_data(&ecbdata); if (textconv_one) free(mf1.ptr); @@ -2830,13 +2907,37 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) DIFF_OPT_CLR(options, COLOR_DIFF); else if (!strcmp(arg, "--color-words")) { DIFF_OPT_SET(options, COLOR_DIFF); - DIFF_OPT_SET(options, COLOR_DIFF_WORDS); + options->word_diff = DIFF_WORDS_COLOR; } else if (!prefixcmp(arg, "--color-words=")) { DIFF_OPT_SET(options, COLOR_DIFF); - DIFF_OPT_SET(options, COLOR_DIFF_WORDS); + options->word_diff = DIFF_WORDS_COLOR; options->word_regex = arg + 14; } + else if (!strcmp(arg, "--word-diff")) { + if (options->word_diff == DIFF_WORDS_NONE) + options->word_diff = DIFF_WORDS_PLAIN; + } + else if (!prefixcmp(arg, "--word-diff=")) { + const char *type = arg + 12; + if (!strcmp(type, "plain")) + options->word_diff = DIFF_WORDS_PLAIN; + else if (!strcmp(type, "color")) { + DIFF_OPT_SET(options, COLOR_DIFF); + options->word_diff = DIFF_WORDS_COLOR; + } + else if (!strcmp(type, "porcelain")) + options->word_diff = DIFF_WORDS_PORCELAIN; + else if (!strcmp(type, "none")) + options->word_diff = DIFF_WORDS_NONE; + else + die("bad --word-diff argument: %s", type); + } + else if (!prefixcmp(arg, "--word-diff-regex=")) { + if (options->word_diff == DIFF_WORDS_NONE) + options->word_diff = DIFF_WORDS_PLAIN; + options->word_regex = arg + 18; + } else if (!strcmp(arg, "--exit-code")) DIFF_OPT_SET(options, EXIT_WITH_STATUS); else if (!strcmp(arg, "--quiet")) diff --git a/diff.h b/diff.h index 6a71013..9ace08c 100644 --- a/diff.h +++ b/diff.h @@ -54,7 +54,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q, #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6) #define DIFF_OPT_FOLLOW_RENAMES (1 << 7) #define DIFF_OPT_COLOR_DIFF (1 << 8) -#define DIFF_OPT_COLOR_DIFF_WORDS (1 << 9) +/* (1 << 9) unused */ #define DIFF_OPT_HAS_CHANGES (1 << 10) #define DIFF_OPT_QUICK (1 << 11) #define DIFF_OPT_NO_INDEX (1 << 12) @@ -79,6 +79,13 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q, #define DIFF_XDL_SET(opts, flag) ((opts)->xdl_opts |= XDF_##flag) #define DIFF_XDL_CLR(opts, flag) ((opts)->xdl_opts &= ~XDF_##flag) +enum diff_words_type { + DIFF_WORDS_NONE = 0, + DIFF_WORDS_PORCELAIN, + DIFF_WORDS_PLAIN, + DIFF_WORDS_COLOR +}; + struct diff_options { const char *filter; const char *orderfile; @@ -108,6 +115,7 @@ struct diff_options { int stat_width; int stat_name_width; const char *word_regex; + enum diff_words_type word_diff; /* this is set by diffcore for DIFF_FORMAT_PATCH */ int found_changes; diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh index 2e2e103..6f7548c 100755 --- a/t/t4034-diff-words.sh +++ b/t/t4034-diff-words.sh @@ -55,6 +55,93 @@ test_expect_success 'word diff with runs of whitespace' ' ' +test_expect_success '--word-diff=color' ' + + word_diff --word-diff=color + +' + +test_expect_success '--color --word-diff=color' ' + + word_diff --color --word-diff=color + +' + +sed 's/#.*$//' > 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] +~ + # significant space +~ + a = b + c +~ +~ ++aa = a +~ +~ ++aeff = aeff * ( aaa ) +~ +EOF + +test_expect_success '--word-diff=porcelain' ' + + word_diff --word-diff=porcelain + +' + +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=plain' ' + + word_diff --word-diff=plain + +' + +test_expect_success '--word-diff=plain --no-color' ' + + word_diff --word-diff=plain --no-color + +' + +cat > expect <<EOF +<WHITE>diff --git a/pre b/post<RESET> +<WHITE>index 330b04f..5ed8eff 100644<RESET> +<WHITE>--- a/pre<RESET> +<WHITE>+++ b/post<RESET> +<CYAN>@@ -1,3 +1,7 @@<RESET> +<RED>[-h(4)-]<RESET><GREEN>{+h(4),hh[44]+}<RESET> + +a = b + c<RESET> + +<GREEN>{+aa = a+}<RESET> + +<GREEN>{+aeff = aeff * ( aaa )+}<RESET> +EOF + +test_expect_success '--word-diff=plain --color' ' + + word_diff --word-diff=plain --color + +' + cat > expect <<\EOF <WHITE>diff --git a/pre b/post<RESET> <WHITE>index 330b04f..5ed8eff 100644<RESET> @@ -143,6 +230,25 @@ test_expect_success 'command-line overrides config' ' word_diff --color-words="[a-z]+" ' +cat > expect <<\EOF +<WHITE>diff --git a/pre b/post<RESET> +<WHITE>index 330b04f..5ed8eff 100644<RESET> +<WHITE>--- a/pre<RESET> +<WHITE>+++ b/post<RESET> +<CYAN>@@ -1,3 +1,7 @@<RESET> +h(4),<GREEN>{+hh+}<RESET>[44] + +a = b + c<RESET> + +<GREEN>{+aa = a+}<RESET> + +<GREEN>{+aeff = aeff * ( aaa+}<RESET> ) +EOF + +test_expect_success 'command-line overrides config: --word-diff-regex' ' + word_diff --color --word-diff-regex="[a-z]+" +' + cp expect.non-whitespace-is-word expect test_expect_success '.gitattributes override config' ' @@ -209,4 +315,20 @@ test_expect_success 'test when words are only removed at the end' ' ' +cat > expect <<\EOF +diff --git a/pre b/post +index 289cb9d..2d06f37 100644 +--- a/pre ++++ b/post +@@ -1 +1 @@ +-(: ++( +EOF + +test_expect_success '--word-diff=none' ' + + word_diff --word-diff=plain --word-diff=none + +' + test_done -- 1.7.1.rc1.260.g41ab89 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/3] diff: add --word-diff option that generalizes --color-words 2010-04-14 15:59 ` [PATCH v4 1/3] diff: add --word-diff option that generalizes --color-words Thomas Rast @ 2010-04-14 21:23 ` Junio C Hamano 0 siblings, 0 replies; 30+ messages in thread From: Junio C Hamano @ 2010-04-14 21:23 UTC (permalink / raw) To: Thomas Rast Cc: git, Johannes Schindelin, Eelis van der Weegen, Paul Mackerras, Miles Bader, Jens Lehmann Looks good; I'll queue this one and hopefully Paul can give feedback to the gitk patches. Thanks. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 2/3] gitk: do not parse " >" context as submodule change 2010-04-14 15:59 ` [PATCH v4 0/3] git-diff --word-diff/gitk --color-words Thomas Rast 2010-04-14 15:59 ` [PATCH v4 1/3] diff: add --word-diff option that generalizes --color-words Thomas Rast @ 2010-04-14 15:59 ` Thomas Rast 2010-04-15 19:57 ` Jens Lehmann 2010-04-17 6:33 ` Paul Mackerras 2010-04-14 15:59 ` [PATCH v4 3/3] gitk: add the equivalent of diff --color-words Thomas Rast 2 siblings, 2 replies; 30+ messages in thread From: Thomas Rast @ 2010-04-14 15:59 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader, Jens Lehmann Since 5c838d2 (gitk: Use the --submodule option for displaying diffs when available, 2009-10-28) gitk erroneously matches " >" and " <" at the beginning of a line in the submodule code even if we're in the diff text section and the lines should be treated as context. Fix by (ab)using the $diffinhdr variable also in the 'Submodule...' case, and move the " >"/" <" specific code inside the $diffinhdr test. The existing code will set $diffinhdr to 0 when it hits a "+++", so that it is always 0 when we can hit a context line. --- gitk | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-) diff --git a/gitk b/gitk index 1f36a3e..7b8799e 100755 --- a/gitk +++ b/gitk @@ -7688,12 +7688,8 @@ proc getblobdiffline {bdf ids} { lappend ctext_file_lines $fname makediffhdr $fname $ids $ctext insert end "\n$line\n" filesep - } elseif {![string compare -length 3 " >" $line]} { - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" dresult - } elseif {![string compare -length 3 " <" $line]} { - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" d0 + # pretend we're in a file header to correctly parse " [><]" + set diffinhdr 1 } elseif {$diffinhdr} { if {![string compare -length 12 "rename from " $line]} { set fname [string range $line [expr 6 + [string first " from " $line] ] end] @@ -7712,6 +7708,14 @@ proc getblobdiffline {bdf ids} { set fname [lindex $fname 0] } makediffhdr $fname $ids + } elseif {![string compare -length 3 " >" $line]} { + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" dresult + continue + } elseif {![string compare -length 3 " <" $line]} { + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" d0 + continue } elseif {[string compare -length 3 $line "---"] == 0} { # do nothing continue -- 1.7.1.rc1.260.g41ab89 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4 2/3] gitk: do not parse " >" context as submodule change 2010-04-14 15:59 ` [PATCH v4 2/3] gitk: do not parse " >" context as submodule change Thomas Rast @ 2010-04-15 19:57 ` Jens Lehmann 2010-04-17 6:33 ` Paul Mackerras 1 sibling, 0 replies; 30+ messages in thread From: Jens Lehmann @ 2010-04-15 19:57 UTC (permalink / raw) To: Thomas Rast Cc: git, Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader Am 14.04.2010 17:59, schrieb Thomas Rast: > Since 5c838d2 (gitk: Use the --submodule option for displaying diffs > when available, 2009-10-28) gitk erroneously matches " >" and " <" > at the beginning of a line in the submodule code even if we're in the > diff text section and the lines should be treated as context. > > Fix by (ab)using the $diffinhdr variable also in the 'Submodule...' > case, and move the " >"/" <" specific code inside the $diffinhdr > test. The existing code will set $diffinhdr to 0 when it hits a > "+++", so that it is always 0 when we can hit a context line. Thanks for fixing this issue I accidentally introduced! In that said patch I unfortunately also managed to screw up the submodule name detection when it was not followed just by commits (but e.g. by "contains untracked content"). I did already send a patch to address this issue, but here is a rebased version on top of your patch series just in case: --------------------8<-------------------- From: Jens Lehmann <Jens.Lehmann@web.de> Date: Thu, 15 Apr 2010 21:53:12 +0200 Subject: [PATCH] Teach gitk to display dirty submodules correctly Since 1.7.1 "git diff --submodule" prints out extra lines when the submodule contains untracked or modified files. Show all those lines for one submodule under the same header. Also e.g. for newly added or removed submodules the submodule name contained trailing garbage because the extraction of the name was not done right. Now it scans either for the first hex number followed by a ".." or the string "contains ". Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> --- gitk-git/gitk | 27 +++++++++++++++++++-------- 1 files changed, 19 insertions(+), 8 deletions(-) diff --git a/gitk-git/gitk b/gitk-git/gitk index f2a1eb7..93d25ec 100644 --- a/gitk-git/gitk +++ b/gitk-git/gitk @@ -7530,7 +7530,7 @@ proc getblobdiffs {ids} { global worddiff global limitdiffs vfilelimit curview global diffencoding targetline diffnparents - global git_version + global git_version currdiffsubmod set textconv {} if {[package vcompare $git_version "1.6.1"] >= 0} { @@ -7560,6 +7560,7 @@ proc getblobdiffs {ids} { set diffencoding [get_path_encoding {}] fconfigure $bdf -blocking 0 -encoding binary -eofchar {} set blobdifffd($ids) $bdf + set currdiffsubmod "" filerun $bdf [list getblobdiffline $bdf $diffids] } @@ -7631,7 +7632,7 @@ proc getblobdiffline {bdf ids} { global ctext_file_names ctext_file_lines global diffinhdr treediffs mergemax diffnparents global diffencoding jump_to_here targetline diffline - global worddiff + global worddiff currdiffsubmod set nr 0 $ctext conf -state normal @@ -7712,15 +7713,24 @@ proc getblobdiffline {bdf ids} { } elseif {![string compare -length 10 "Submodule " $line]} { # start of a new submodule - if {[string compare [$ctext get "end - 4c" end] "\n \n\n"]} { + if {[regexp -indices "\[0-9a-f\]+\\.\\." $line nameend]} { + set fname [string range $line 10 [expr [lindex $nameend 0] - 2]] + } else { + set fname [string range $line 10 [expr [string first "contains " $line] - 2]] + } + if {$currdiffsubmod != $fname} { $ctext insert end "\n"; # Add newline after commit message } set curdiffstart [$ctext index "end - 1c"] lappend ctext_file_names "" - set fname [string range $line 10 [expr [string last " " $line] - 1]] - lappend ctext_file_lines $fname - makediffhdr $fname $ids - $ctext insert end "\n$line\n" filesep + if {$currdiffsubmod != $fname} { + lappend ctext_file_lines $fname + makediffhdr $fname $ids + set currdiffsubmod $fname + $ctext insert end "\n$line\n" filesep + } else { + $ctext insert end "$line\n" filesep + } # pretend we're in a file header to correctly parse " [><]" set diffinhdr 1 } elseif {$diffinhdr} { @@ -8588,7 +8598,7 @@ proc do_cmp_commits {a b} { } proc diffcommits {a b} { - global diffcontext diffids blobdifffd diffinhdr + global diffcontext diffids blobdifffd diffinhdr currdiffsubmod set tmpdir [gitknewtmpdir] set fna [file join $tmpdir "commit-[string range $a 0 7]"] @@ -8609,6 +8619,7 @@ proc diffcommits {a b} { set diffids [list commits $a $b] set blobdifffd($diffids) $fd set diffinhdr 0 + set currdiffsubmod "" filerun $fd [list getblobdiffline $fd $diffids] } -- 1.7.1.rc1.252.gff9f ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4 2/3] gitk: do not parse " >" context as submodule change 2010-04-14 15:59 ` [PATCH v4 2/3] gitk: do not parse " >" context as submodule change Thomas Rast 2010-04-15 19:57 ` Jens Lehmann @ 2010-04-17 6:33 ` Paul Mackerras 2010-04-17 12:20 ` Thomas Rast 1 sibling, 1 reply; 30+ messages in thread From: Paul Mackerras @ 2010-04-17 6:33 UTC (permalink / raw) To: Thomas Rast Cc: git, Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Miles Bader, Jens Lehmann On Wed, Apr 14, 2010 at 05:59:07PM +0200, Thomas Rast wrote: > Since 5c838d2 (gitk: Use the --submodule option for displaying diffs > when available, 2009-10-28) gitk erroneously matches " >" and " <" > at the beginning of a line in the submodule code even if we're in the > diff text section and the lines should be treated as context. > > Fix by (ab)using the $diffinhdr variable also in the 'Submodule...' > case, and move the " >"/" <" specific code inside the $diffinhdr > test. The existing code will set $diffinhdr to 0 when it hits a > "+++", so that it is always 0 when we can hit a context line. Looks good, but there's no Signed-off-by? Paul. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 2/3] gitk: do not parse " >" context as submodule change 2010-04-17 6:33 ` Paul Mackerras @ 2010-04-17 12:20 ` Thomas Rast 2010-04-19 1:08 ` Paul Mackerras 0 siblings, 1 reply; 30+ messages in thread From: Thomas Rast @ 2010-04-17 12:20 UTC (permalink / raw) To: Paul Mackerras Cc: git, Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Miles Bader, Jens Lehmann Paul Mackerras wrote: > On Wed, Apr 14, 2010 at 05:59:07PM +0200, Thomas Rast wrote: > > > Since 5c838d2 (gitk: Use the --submodule option for displaying diffs > > when available, 2009-10-28) gitk erroneously matches " >" and " <" > > at the beginning of a line in the submodule code even if we're in the > > diff text section and the lines should be treated as context. > > > > Fix by (ab)using the $diffinhdr variable also in the 'Submodule...' > > case, and move the " >"/" <" specific code inside the $diffinhdr > > test. The existing code will set $diffinhdr to 0 when it hits a > > "+++", so that it is always 0 when we can hit a context line. > > Looks good, but there's no Signed-off-by? Whoops, sorry: Signed-off-by: Thomas Rast <trast@student.ethz.ch> -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 2/3] gitk: do not parse " >" context as submodule change 2010-04-17 12:20 ` Thomas Rast @ 2010-04-19 1:08 ` Paul Mackerras 2010-04-19 16:27 ` [PATCH v4' 1/2] " Thomas Rast 0 siblings, 1 reply; 30+ messages in thread From: Paul Mackerras @ 2010-04-19 1:08 UTC (permalink / raw) To: Thomas Rast Cc: git, Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Miles Bader, Jens Lehmann On Sat, Apr 17, 2010 at 02:20:55PM +0200, Thomas Rast wrote: > Whoops, sorry: > > Signed-off-by: Thomas Rast <trast@student.ethz.ch> Thanks, but now that I have applied Jens Lehmann's patch that also touches this area, your patch doesn't apply. Could you rebase it and send it again? Thanks, Paul. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4' 1/2] gitk: do not parse " >" context as submodule change 2010-04-19 1:08 ` Paul Mackerras @ 2010-04-19 16:27 ` Thomas Rast 2010-04-19 16:27 ` [PATCH v4' 2/2] gitk: add the equivalent of diff --color-words Thomas Rast 2010-04-19 17:22 ` [PATCH v4' 1/2] gitk: do not parse " >" context as submodule change Jens Lehmann 0 siblings, 2 replies; 30+ messages in thread From: Thomas Rast @ 2010-04-19 16:27 UTC (permalink / raw) To: Paul Mackerras; +Cc: git Since 5c838d2 (gitk: Use the --submodule option for displaying diffs when available, 2009-10-28) gitk erroneously matches " >" and " <" at the beginning of a line in the submodule code even if we're in the diff text section and the lines should be treated as context. Fix by (ab)using the $diffinhdr variable also in the 'Submodule...' case, and move the " >"/" <" specific code inside the $diffinhdr test. The existing code will set $diffinhdr to 0 when it hits a "+++", so that it is always 0 when we can hit a context line. Signed-off-by: Thomas Rast <trast@student.ethz.ch> --- > Thanks, but now that I have applied Jens Lehmann's patch that also > touches this area, your patch doesn't apply. Could you rebase it and > send it again? Sure. gitk | 18 ++++++++++-------- 1 files changed, 10 insertions(+), 8 deletions(-) diff --git a/gitk b/gitk index 1b0e09a..6513ef8 100755 --- a/gitk +++ b/gitk @@ -7706,14 +7706,8 @@ proc getblobdiffline {bdf ids} { } else { $ctext insert end "$line\n" filesep } - } elseif {![string compare -length 3 " >" $line]} { - set $currdiffsubmod "" - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" dresult - } elseif {![string compare -length 3 " <" $line]} { - set $currdiffsubmod "" - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" d0 + # pretend we're in a file header to correctly parse " [><]" + set diffinhdr 1 } elseif {$diffinhdr} { if {![string compare -length 12 "rename from " $line]} { set fname [string range $line [expr 6 + [string first " from " $line] ] end] @@ -7732,6 +7726,14 @@ proc getblobdiffline {bdf ids} { set fname [lindex $fname 0] } makediffhdr $fname $ids + } elseif {![string compare -length 3 " >" $line]} { + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" dresult + continue + } elseif {![string compare -length 3 " <" $line]} { + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" d0 + continue } elseif {[string compare -length 3 $line "---"] == 0} { # do nothing continue -- 1.7.1.rc1.284.g4b2e3 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4' 2/2] gitk: add the equivalent of diff --color-words 2010-04-19 16:27 ` [PATCH v4' 1/2] " Thomas Rast @ 2010-04-19 16:27 ` Thomas Rast 2010-04-19 17:22 ` [PATCH v4' 1/2] gitk: do not parse " >" context as submodule change Jens Lehmann 1 sibling, 0 replies; 30+ messages in thread From: Thomas Rast @ 2010-04-19 16:27 UTC (permalink / raw) To: Paul Mackerras; +Cc: git Use the newly added 'diff --word-diff=porcelain' to teach gitk a color-words mode, with two different modes analogous to the --word-diff=plain and --word-diff=color settings. These are selected by a dropdown box. As an extra twist, automatically enable this word-diff support when the user mentions a word-diff related option on the command line. These options were previously ignored because they would break diff parsing. Both of these features are only enabled if we have a version of git that supports --word-diff=porcelain, tentatively set to 1.7.2. Signed-off-by: Thomas Rast <trast@student.ethz.ch> --- gitk | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 61 insertions(+), 3 deletions(-) diff --git a/gitk b/gitk index 6513ef8..46d1b0e 100755 --- a/gitk +++ b/gitk @@ -131,6 +131,7 @@ proc unmerged_files {files} { proc parseviewargs {n arglist} { global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env + global worddiff git_version set vdatemode($n) 0 set vmergeonly($n) 0 @@ -168,7 +169,7 @@ proc parseviewargs {n arglist} { lappend diffargs $arg } "--raw" - "--patch-with-raw" - "--patch-with-stat" - - "--name-only" - "--name-status" - "--color" - "--color-words" - + "--name-only" - "--name-status" - "--color" - "--log-size" - "--pretty=*" - "--decorate" - "--abbrev-commit" - "--cc" - "-z" - "--header" - "--parents" - "--boundary" - "--no-color" - "-g" - "--walk-reflogs" - "--no-walk" - @@ -177,6 +178,18 @@ proc parseviewargs {n arglist} { # These cause our parsing of git log's output to fail, or else # they're options we want to set ourselves, so ignore them. } + "--color-words*" - "--word-diff=color" { + # These trigger a word diff in the console interface, + # so help the user by enabling our own support + if {[package vcompare $git_version "1.7.2"] >= 0} { + set worddiff [mc "Color words"] + } + } + "--word-diff*" { + if {[package vcompare $git_version "1.7.2"] >= 0} { + set worddiff [mc "Markup words"] + } + } "--stat=*" - "--numstat" - "--shortstat" - "--summary" - "--check" - "--exit-code" - "--quiet" - "--topo-order" - "--full-history" - "--dense" - "--sparse" - @@ -1970,6 +1983,8 @@ proc makewindow {} { global fprogitem fprogcoord lastprogupdate progupdatepending global rprogitem rprogcoord rownumsel numcommits global have_tk85 use_ttk NS + global git_version + global worddiff # The "mc" arguments here are purely so that xgettext # sees the following string as needing to be translated @@ -2243,6 +2258,15 @@ proc makewindow {} { ${NS}::checkbutton .bleft.mid.ignspace -text [mc "Ignore space change"] \ -command changeignorespace -variable ignorespace pack .bleft.mid.ignspace -side left -padx 5 + + set worddiff [mc "Line diff"] + if {[package vcompare $git_version "1.7.2"] >= 0} { + makedroplist .bleft.mid.worddiff worddiff [mc "Line diff"] \ + [mc "Markup words"] [mc "Color words"] + trace add variable worddiff write changeworddiff + pack .bleft.mid.worddiff -side left -padx 5 + } + set ctext .bleft.bottom.ctext text $ctext -background $bgcolor -foreground $fgcolor \ -state disabled -font textfont \ @@ -7502,11 +7526,16 @@ proc changeignorespace {} { reselectline } +proc changeworddiff {name ix op} { + reselectline +} + proc getblobdiffs {ids} { global blobdifffd diffids env global diffinhdr treediffs global diffcontext global ignorespace + global worddiff global limitdiffs vfilelimit curview global diffencoding targetline diffnparents global git_version currdiffsubmod @@ -7523,6 +7552,9 @@ proc getblobdiffs {ids} { if {$ignorespace} { append cmd " -w" } + if {$worddiff ne [mc "Line diff"]} { + append cmd " --word-diff=porcelain" + } if {$limitdiffs && $vfilelimit($curview) ne {}} { set cmd [concat $cmd -- $vfilelimit($curview)] } @@ -7608,6 +7640,7 @@ proc getblobdiffline {bdf ids} { global ctext_file_names ctext_file_lines global diffinhdr treediffs mergemax diffnparents global diffencoding jump_to_here targetline diffline currdiffsubmod + global worddiff set nr 0 $ctext conf -state normal @@ -7749,15 +7782,28 @@ proc getblobdiffline {bdf ids} { # parse the prefix - one ' ', '-' or '+' for each parent set prefix [string range $line 0 [expr {$diffnparents - 1}]] set tag [expr {$diffnparents > 1? "m": "d"}] + set dowords [expr {$worddiff ne [mc "Line diff"] && $diffnparents == 1}] + set words_pre_markup "" + set words_post_markup "" if {[string trim $prefix " -+"] eq {}} { # prefix only has " ", "-" and "+" in it: normal diff line set num [string first "-" $prefix] + if {$dowords} { + set line [string range $line 1 end] + } if {$num >= 0} { # removed line, first parent with line is $num if {$num >= $mergemax} { set num "max" } - $ctext insert end "$line\n" $tag$num + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "\[-$line-\]" $tag$num + } else { + $ctext insert end "$line" $tag$num + } + if {!$dowords} { + $ctext insert end "\n" $tag$num + } } else { set tags {} if {[string first "+" $prefix] >= 0} { @@ -7772,6 +7818,8 @@ proc getblobdiffline {bdf ids} { lappend tags m$num } } + set words_pre_markup "{+" + set words_post_markup "+}" } if {$targetline ne {}} { if {$diffline == $targetline} { @@ -7781,8 +7829,17 @@ proc getblobdiffline {bdf ids} { incr diffline } } - $ctext insert end "$line\n" $tags + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "$words_pre_markup$line$words_post_markup" $tags + } else { + $ctext insert end "$line" $tags + } + if {!$dowords} { + $ctext insert end "\n" $tags + } } + } elseif {$dowords && $prefix eq "~"} { + $ctext insert end "\n" {} } else { # "\ No newline at end of file", # or something else we don't recognize @@ -11393,6 +11450,7 @@ if {[tk windowingsystem] eq "win32"} { set diffcolors {red "#00a000" blue} set diffcontext 3 set ignorespace 0 +set worddiff "" set markbgcolor "#e0e0ff" set circlecolors {white blue gray blue blue} -- 1.7.1.rc1.284.g4b2e3 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4' 1/2] gitk: do not parse " >" context as submodule change 2010-04-19 16:27 ` [PATCH v4' 1/2] " Thomas Rast 2010-04-19 16:27 ` [PATCH v4' 2/2] gitk: add the equivalent of diff --color-words Thomas Rast @ 2010-04-19 17:22 ` Jens Lehmann 2010-04-20 17:05 ` Jens Lehmann 1 sibling, 1 reply; 30+ messages in thread From: Jens Lehmann @ 2010-04-19 17:22 UTC (permalink / raw) To: Thomas Rast; +Cc: Paul Mackerras, git Am 19.04.2010 18:27, schrieb Thomas Rast: >> Thanks, but now that I have applied Jens Lehmann's patch that also >> touches this area, your patch doesn't apply. Could you rebase it and >> send it again? > > Sure. There might be a problem with this rebase. Unfortunately I am very short on time, so I can't test this today. I think setting the $currdiffsubmod variable to the empty string has to show up in the two sections formatting the lines with " >" & " <". > gitk | 18 ++++++++++-------- > 1 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/gitk b/gitk > index 1b0e09a..6513ef8 100755 > --- a/gitk > +++ b/gitk > @@ -7706,14 +7706,8 @@ proc getblobdiffline {bdf ids} { > } else { > $ctext insert end "$line\n" filesep > } > - } elseif {![string compare -length 3 " >" $line]} { > - set $currdiffsubmod "" > - set line [encoding convertfrom $diffencoding $line] > - $ctext insert end "$line\n" dresult > - } elseif {![string compare -length 3 " <" $line]} { > - set $currdiffsubmod "" > - set line [encoding convertfrom $diffencoding $line] > - $ctext insert end "$line\n" d0 > + # pretend we're in a file header to correctly parse " [><]" > + set diffinhdr 1 > } elseif {$diffinhdr} { > if {![string compare -length 12 "rename from " $line]} { > set fname [string range $line [expr 6 + [string first " from " $line] ] end] > @@ -7732,6 +7726,14 @@ proc getblobdiffline {bdf ids} { > set fname [lindex $fname 0] > } > makediffhdr $fname $ids > + } elseif {![string compare -length 3 " >" $line]} { I suspect we need a 'set $currdiffsubmod ""' here > + set line [encoding convertfrom $diffencoding $line] > + $ctext insert end "$line\n" dresult > + continue > + } elseif {![string compare -length 3 " <" $line]} { and here. > + set line [encoding convertfrom $diffencoding $line] > + $ctext insert end "$line\n" d0 > + continue > } elseif {[string compare -length 3 $line "---"] == 0} { > # do nothing > continue If you can wait until tomorrow I can check that. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4' 1/2] gitk: do not parse " >" context as submodule change 2010-04-19 17:22 ` [PATCH v4' 1/2] gitk: do not parse " >" context as submodule change Jens Lehmann @ 2010-04-20 17:05 ` Jens Lehmann 0 siblings, 0 replies; 30+ messages in thread From: Jens Lehmann @ 2010-04-20 17:05 UTC (permalink / raw) To: Jens Lehmann; +Cc: Thomas Rast, Paul Mackerras, git Am 19.04.2010 19:22, schrieb Jens Lehmann: > Am 19.04.2010 18:27, schrieb Thomas Rast: >>> Thanks, but now that I have applied Jens Lehmann's patch that also >>> touches this area, your patch doesn't apply. Could you rebase it and >>> send it again? >> >> Sure. > > There might be a problem with this rebase. Unfortunately I am very > short on time, so I can't test this today. I successfully tested this patch, this was a false alarm from my side. Sorry for the noise. Tested-by: Jens.Lehmann@web.de ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 3/3] gitk: add the equivalent of diff --color-words 2010-04-14 15:59 ` [PATCH v4 0/3] git-diff --word-diff/gitk --color-words Thomas Rast 2010-04-14 15:59 ` [PATCH v4 1/3] diff: add --word-diff option that generalizes --color-words Thomas Rast 2010-04-14 15:59 ` [PATCH v4 2/3] gitk: do not parse " >" context as submodule change Thomas Rast @ 2010-04-14 15:59 ` Thomas Rast 2010-04-17 6:35 ` Paul Mackerras 2 siblings, 1 reply; 30+ messages in thread From: Thomas Rast @ 2010-04-14 15:59 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader, Jens Lehmann Use the newly added 'diff --word-diff=porcelain' to teach gitk a color-words mode, with two different modes analogous to the --word-diff=plain and --word-diff=color settings. These are selected by a dropdown box. As an extra twist, automatically enable this word-diff support when the user mentions a word-diff related option on the command line. These options were previously ignored because they would break diff parsing. Both of these features are only enabled if we have a version of git that supports --word-diff=porcelain, tentatively set to 1.7.2. Signed-off-by: Thomas Rast <trast@student.ethz.ch> --- gitk | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 61 insertions(+), 3 deletions(-) diff --git a/gitk b/gitk index 7b8799e..f2a1eb7 100755 --- a/gitk +++ b/gitk @@ -131,6 +131,7 @@ proc unmerged_files {files} { proc parseviewargs {n arglist} { global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env + global worddiff git_version set vdatemode($n) 0 set vmergeonly($n) 0 @@ -168,7 +169,7 @@ proc parseviewargs {n arglist} { lappend diffargs $arg } "--raw" - "--patch-with-raw" - "--patch-with-stat" - - "--name-only" - "--name-status" - "--color" - "--color-words" - + "--name-only" - "--name-status" - "--color" - "--log-size" - "--pretty=*" - "--decorate" - "--abbrev-commit" - "--cc" - "-z" - "--header" - "--parents" - "--boundary" - "--no-color" - "-g" - "--walk-reflogs" - "--no-walk" - @@ -177,6 +178,18 @@ proc parseviewargs {n arglist} { # These cause our parsing of git log's output to fail, or else # they're options we want to set ourselves, so ignore them. } + "--color-words*" - "--word-diff=color" { + # These trigger a word diff in the console interface, + # so help the user by enabling our own support + if {[package vcompare $git_version "1.7.2"] >= 0} { + set worddiff [mc "Color words"] + } + } + "--word-diff*" { + if {[package vcompare $git_version "1.7.2"] >= 0} { + set worddiff [mc "Markup words"] + } + } "--stat=*" - "--numstat" - "--shortstat" - "--summary" - "--check" - "--exit-code" - "--quiet" - "--topo-order" - "--full-history" - "--dense" - "--sparse" - @@ -1967,6 +1980,8 @@ proc makewindow {} { global fprogitem fprogcoord lastprogupdate progupdatepending global rprogitem rprogcoord rownumsel numcommits global have_tk85 use_ttk NS + global git_version + global worddiff # The "mc" arguments here are purely so that xgettext # sees the following string as needing to be translated @@ -2240,6 +2255,15 @@ proc makewindow {} { ${NS}::checkbutton .bleft.mid.ignspace -text [mc "Ignore space change"] \ -command changeignorespace -variable ignorespace pack .bleft.mid.ignspace -side left -padx 5 + + set worddiff [mc "Line diff"] + if {[package vcompare $git_version "1.7.2"] >= 0} { + makedroplist .bleft.mid.worddiff worddiff [mc "Line diff"] \ + [mc "Markup words"] [mc "Color words"] + trace add variable worddiff write changeworddiff + pack .bleft.mid.worddiff -side left -padx 5 + } + set ctext .bleft.bottom.ctext text $ctext -background $bgcolor -foreground $fgcolor \ -state disabled -font textfont \ @@ -7494,11 +7518,16 @@ proc changeignorespace {} { reselectline } +proc changeworddiff {name ix op} { + reselectline +} + proc getblobdiffs {ids} { global blobdifffd diffids env global diffinhdr treediffs global diffcontext global ignorespace + global worddiff global limitdiffs vfilelimit curview global diffencoding targetline diffnparents global git_version @@ -7515,6 +7544,9 @@ proc getblobdiffs {ids} { if {$ignorespace} { append cmd " -w" } + if {$worddiff ne [mc "Line diff"]} { + append cmd " --word-diff=porcelain" + } if {$limitdiffs && $vfilelimit($curview) ne {}} { set cmd [concat $cmd -- $vfilelimit($curview)] } @@ -7599,6 +7631,7 @@ proc getblobdiffline {bdf ids} { global ctext_file_names ctext_file_lines global diffinhdr treediffs mergemax diffnparents global diffencoding jump_to_here targetline diffline + global worddiff set nr 0 $ctext conf -state normal @@ -7731,15 +7764,28 @@ proc getblobdiffline {bdf ids} { # parse the prefix - one ' ', '-' or '+' for each parent set prefix [string range $line 0 [expr {$diffnparents - 1}]] set tag [expr {$diffnparents > 1? "m": "d"}] + set dowords [expr {$worddiff ne [mc "Line diff"] && $diffnparents == 1}] + set words_pre_markup "" + set words_post_markup "" if {[string trim $prefix " -+"] eq {}} { # prefix only has " ", "-" and "+" in it: normal diff line set num [string first "-" $prefix] + if {$dowords} { + set line [string range $line 1 end] + } if {$num >= 0} { # removed line, first parent with line is $num if {$num >= $mergemax} { set num "max" } - $ctext insert end "$line\n" $tag$num + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "\[-$line-\]" $tag$num + } else { + $ctext insert end "$line" $tag$num + } + if {!$dowords} { + $ctext insert end "\n" $tag$num + } } else { set tags {} if {[string first "+" $prefix] >= 0} { @@ -7754,6 +7800,8 @@ proc getblobdiffline {bdf ids} { lappend tags m$num } } + set words_pre_markup "{+" + set words_post_markup "+}" } if {$targetline ne {}} { if {$diffline == $targetline} { @@ -7763,8 +7811,17 @@ proc getblobdiffline {bdf ids} { incr diffline } } - $ctext insert end "$line\n" $tags + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "$words_pre_markup$line$words_post_markup" $tags + } else { + $ctext insert end "$line" $tags + } + if {!$dowords} { + $ctext insert end "\n" $tags + } } + } elseif {$dowords && $prefix eq "~"} { + $ctext insert end "\n" {} } else { # "\ No newline at end of file", # or something else we don't recognize @@ -11383,6 +11440,7 @@ if {[tk windowingsystem] eq "win32"} { set diffcolors {red "#00a000" blue} set diffcontext 3 set ignorespace 0 +set worddiff "" set markbgcolor "#e0e0ff" set circlecolors {white blue gray blue blue} -- 1.7.1.rc1.260.g41ab89 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4 3/3] gitk: add the equivalent of diff --color-words 2010-04-14 15:59 ` [PATCH v4 3/3] gitk: add the equivalent of diff --color-words Thomas Rast @ 2010-04-17 6:35 ` Paul Mackerras 2010-04-17 6:55 ` Junio C Hamano 0 siblings, 1 reply; 30+ messages in thread From: Paul Mackerras @ 2010-04-17 6:35 UTC (permalink / raw) To: Thomas Rast Cc: git, Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Miles Bader, Jens Lehmann On Wed, Apr 14, 2010 at 05:59:08PM +0200, Thomas Rast wrote: > Use the newly added 'diff --word-diff=porcelain' to teach gitk a > color-words mode, with two different modes analogous to the > --word-diff=plain and --word-diff=color settings. These are selected > by a dropdown box. > > As an extra twist, automatically enable this word-diff support when > the user mentions a word-diff related option on the command line. > These options were previously ignored because they would break diff > parsing. > > Both of these features are only enabled if we have a version of git > that supports --word-diff=porcelain, tentatively set to 1.7.2. > > Signed-off-by: Thomas Rast <trast@student.ethz.ch> Looks fine. The only nit I can see is that a "--word-diffoobar" option would get treated as "--word-diff=plain" rather than giving an error or being passed as-is to git log. When does this need to go in, i.e. when is the git patch likely to go in? Paul. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 3/3] gitk: add the equivalent of diff --color-words 2010-04-17 6:35 ` Paul Mackerras @ 2010-04-17 6:55 ` Junio C Hamano 0 siblings, 0 replies; 30+ messages in thread From: Junio C Hamano @ 2010-04-17 6:55 UTC (permalink / raw) To: Paul Mackerras Cc: Thomas Rast, git, Johannes Schindelin, Eelis van der Weegen, Miles Bader, Jens Lehmann Paul Mackerras <paulus@samba.org> writes: > When does this need to go in, i.e. when is the git patch likely to go > in? I am expecting that this will be in soon after the current 1.7.1 cycle, sometime around early-to-mid May timeframe. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 2/3] gitk: do not parse " >" context as submodule change 2010-04-12 13:07 ` [PATCH v3 0/2] gitk --color-words Thomas Rast 2010-04-12 13:07 ` [PATCH v3 1/3] diff: add --word-diff option that generalizes --color-words Thomas Rast @ 2010-04-12 13:07 ` Thomas Rast 2010-04-12 13:25 ` [PATCH v3.1] " Thomas Rast 2010-04-12 13:07 ` [PATCH v3 3/3] gitk: add the equivalent of diff --color-words Thomas Rast 2 siblings, 1 reply; 30+ messages in thread From: Thomas Rast @ 2010-04-12 13:07 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader, Jens Lehmann Since 5c838d2 (gitk: Use the --submodule option for displaying diffs when available, 2009-10-28) gitk erroneously matches " >" and " <" at the beginning of a line in the submodule code even if we're in the diff text section and the lines should be treated as context. Fix by (ab)using the $diffinhdr variable also in the 'Submodule...' case, and move the " >"/" <" specific code inside the $diffinhdr test. The existing code will set $diffinhdr to 0 when it hits a "+++", so that it is always 0 when we can hit a context line. --- gitk | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-) diff --git a/gitk b/gitk index 1f36a3e..e9e284e 100755 --- a/gitk +++ b/gitk @@ -7688,12 +7688,8 @@ proc getblobdiffline {bdf ids} { lappend ctext_file_lines $fname makediffhdr $fname $ids $ctext insert end "\n$line\n" filesep - } elseif {![string compare -length 3 " >" $line]} { - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" dresult - } elseif {![string compare -length 3 " <" $line]} { - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" d0 + # pretend we're in a file header to correctly parse " [><]" + set diffinhdr 1 } elseif {$diffinhdr} { if {![string compare -length 12 "rename from " $line]} { set fname [string range $line [expr 6 + [string first " from " $line] ] end] @@ -7712,6 +7708,12 @@ proc getblobdiffline {bdf ids} { set fname [lindex $fname 0] } makediffhdr $fname $ids + } elseif {![string compare -length 3 " >" $line]} { + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" dresult + } elseif {![string compare -length 3 " <" $line]} { + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" d0 } elseif {[string compare -length 3 $line "---"] == 0} { # do nothing continue -- 1.7.1.rc0.262.gff1a3 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3.1] gitk: do not parse " >" context as submodule change 2010-04-12 13:07 ` [PATCH v3 2/3] gitk: do not parse " >" context as submodule change Thomas Rast @ 2010-04-12 13:25 ` Thomas Rast 0 siblings, 0 replies; 30+ messages in thread From: Thomas Rast @ 2010-04-12 13:25 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader, Jens Lehmann Since 5c838d2 (gitk: Use the --submodule option for displaying diffs when available, 2009-10-28) gitk erroneously matches " >" and " <" at the beginning of a line in the submodule code even if we're in the diff text section and the lines should be treated as context. Fix by (ab)using the $diffinhdr variable also in the 'Submodule...' case, and move the " >"/" <" specific code inside the $diffinhdr test. The existing code will set $diffinhdr to 0 when it hits a "+++", so that it is always 0 when we can hit a context line. --- I just noticed that I forgot to add the 'continue' that prevents the final 'insert' in the $diffinhdr block from kicking in, which resulted in doubling of the lines. This one has them. Sorry for the noise. gitk | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-) diff --git a/gitk b/gitk index 1f36a3e..7b8799e 100755 --- a/gitk +++ b/gitk @@ -7688,12 +7688,8 @@ proc getblobdiffline {bdf ids} { lappend ctext_file_lines $fname makediffhdr $fname $ids $ctext insert end "\n$line\n" filesep - } elseif {![string compare -length 3 " >" $line]} { - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" dresult - } elseif {![string compare -length 3 " <" $line]} { - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" d0 + # pretend we're in a file header to correctly parse " [><]" + set diffinhdr 1 } elseif {$diffinhdr} { if {![string compare -length 12 "rename from " $line]} { set fname [string range $line [expr 6 + [string first " from " $line] ] end] @@ -7712,6 +7708,14 @@ proc getblobdiffline {bdf ids} { set fname [lindex $fname 0] } makediffhdr $fname $ids + } elseif {![string compare -length 3 " >" $line]} { + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" dresult + continue + } elseif {![string compare -length 3 " <" $line]} { + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" d0 + continue } elseif {[string compare -length 3 $line "---"] == 0} { # do nothing continue -- 1.7.1.rc0.260.g2b919 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 3/3] gitk: add the equivalent of diff --color-words 2010-04-12 13:07 ` [PATCH v3 0/2] gitk --color-words Thomas Rast 2010-04-12 13:07 ` [PATCH v3 1/3] diff: add --word-diff option that generalizes --color-words Thomas Rast 2010-04-12 13:07 ` [PATCH v3 2/3] gitk: do not parse " >" context as submodule change Thomas Rast @ 2010-04-12 13:07 ` Thomas Rast 2 siblings, 0 replies; 30+ messages in thread From: Thomas Rast @ 2010-04-12 13:07 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader, Jens Lehmann Use the newly added 'diff --word-diff=porcelain' to teach gitk a color-words mode, with two different modes analogous to the --word-diff=plain and --word-diff=color settings. These are selected by a dropdown box. As an extra twist, automatically enable this word-diff support when the user mentions a word-diff related option on the command line. These options were previously ignored because they would break diff parsing. Both of these features are only enabled if we have a version of git that supports --word-diff=porcelain, tentatively set to 1.7.2. Signed-off-by: Thomas Rast <trast@student.ethz.ch> --- gitk | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 61 insertions(+), 3 deletions(-) diff --git a/gitk b/gitk index e9e284e..466c328 100755 --- a/gitk +++ b/gitk @@ -131,6 +131,7 @@ proc unmerged_files {files} { proc parseviewargs {n arglist} { global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env + global worddiff git_version set vdatemode($n) 0 set vmergeonly($n) 0 @@ -168,7 +169,7 @@ proc parseviewargs {n arglist} { lappend diffargs $arg } "--raw" - "--patch-with-raw" - "--patch-with-stat" - - "--name-only" - "--name-status" - "--color" - "--color-words" - + "--name-only" - "--name-status" - "--color" - "--log-size" - "--pretty=*" - "--decorate" - "--abbrev-commit" - "--cc" - "-z" - "--header" - "--parents" - "--boundary" - "--no-color" - "-g" - "--walk-reflogs" - "--no-walk" - @@ -177,6 +178,18 @@ proc parseviewargs {n arglist} { # These cause our parsing of git log's output to fail, or else # they're options we want to set ourselves, so ignore them. } + "--color-words*" - "--word-diff=color" { + # These trigger a word diff in the console interface, + # so help the user by enabling our own support + if {[package vcompare $git_version "1.7.2"] >= 0} { + set worddiff [mc "Color words"] + } + } + "--word-diff*" { + if {[package vcompare $git_version "1.7.2"] >= 0} { + set worddiff [mc "Markup words"] + } + } "--stat=*" - "--numstat" - "--shortstat" - "--summary" - "--check" - "--exit-code" - "--quiet" - "--topo-order" - "--full-history" - "--dense" - "--sparse" - @@ -1967,6 +1980,8 @@ proc makewindow {} { global fprogitem fprogcoord lastprogupdate progupdatepending global rprogitem rprogcoord rownumsel numcommits global have_tk85 use_ttk NS + global git_version + global worddiff # The "mc" arguments here are purely so that xgettext # sees the following string as needing to be translated @@ -2240,6 +2255,15 @@ proc makewindow {} { ${NS}::checkbutton .bleft.mid.ignspace -text [mc "Ignore space change"] \ -command changeignorespace -variable ignorespace pack .bleft.mid.ignspace -side left -padx 5 + + set worddiff [mc "Line diff"] + if {[package vcompare $git_version "1.7.2"] >= 0} { + makedroplist .bleft.mid.worddiff worddiff [mc "Line diff"] \ + [mc "Markup words"] [mc "Color words"] + trace add variable worddiff write changeworddiff + pack .bleft.mid.worddiff -side left -padx 5 + } + set ctext .bleft.bottom.ctext text $ctext -background $bgcolor -foreground $fgcolor \ -state disabled -font textfont \ @@ -7494,11 +7518,16 @@ proc changeignorespace {} { reselectline } +proc changeworddiff {name ix op} { + reselectline +} + proc getblobdiffs {ids} { global blobdifffd diffids env global diffinhdr treediffs global diffcontext global ignorespace + global worddiff global limitdiffs vfilelimit curview global diffencoding targetline diffnparents global git_version @@ -7515,6 +7544,9 @@ proc getblobdiffs {ids} { if {$ignorespace} { append cmd " -w" } + if {$worddiff ne [mc "Line diff"]} { + append cmd " --word-diff=porcelain" + } if {$limitdiffs && $vfilelimit($curview) ne {}} { set cmd [concat $cmd -- $vfilelimit($curview)] } @@ -7599,6 +7631,7 @@ proc getblobdiffline {bdf ids} { global ctext_file_names ctext_file_lines global diffinhdr treediffs mergemax diffnparents global diffencoding jump_to_here targetline diffline + global worddiff set nr 0 $ctext conf -state normal @@ -7729,15 +7762,28 @@ proc getblobdiffline {bdf ids} { # parse the prefix - one ' ', '-' or '+' for each parent set prefix [string range $line 0 [expr {$diffnparents - 1}]] set tag [expr {$diffnparents > 1? "m": "d"}] + set dowords [expr {$worddiff ne [mc "Line diff"] && $diffnparents == 1}] + set words_pre_markup "" + set words_post_markup "" if {[string trim $prefix " -+"] eq {}} { # prefix only has " ", "-" and "+" in it: normal diff line set num [string first "-" $prefix] + if {$dowords} { + set line [string range $line 1 end] + } if {$num >= 0} { # removed line, first parent with line is $num if {$num >= $mergemax} { set num "max" } - $ctext insert end "$line\n" $tag$num + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "\[-$line-\]" $tag$num + } else { + $ctext insert end "$line" $tag$num + } + if {!$dowords} { + $ctext insert end "\n" $tag$num + } } else { set tags {} if {[string first "+" $prefix] >= 0} { @@ -7752,6 +7798,8 @@ proc getblobdiffline {bdf ids} { lappend tags m$num } } + set words_pre_markup "{+" + set words_post_markup "+}" } if {$targetline ne {}} { if {$diffline == $targetline} { @@ -7761,8 +7809,17 @@ proc getblobdiffline {bdf ids} { incr diffline } } - $ctext insert end "$line\n" $tags + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "$words_pre_markup$line$words_post_markup" $tags + } else { + $ctext insert end "$line" $tags + } + if {!$dowords} { + $ctext insert end "\n" $tags + } } + } elseif {$dowords && $prefix eq "~"} { + $ctext insert end "\n" {} } else { # "\ No newline at end of file", # or something else we don't recognize @@ -11381,6 +11438,7 @@ if {[tk windowingsystem] eq "win32"} { set diffcolors {red "#00a000" blue} set diffcontext 3 set ignorespace 0 +set worddiff "" set markbgcolor "#e0e0ff" set circlecolors {white blue gray blue blue} -- 1.7.1.rc0.262.gff1a3 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] diff: add --word-diff option that generalizes --color-words 2010-04-05 18:46 ` Junio C Hamano 2010-04-05 18:53 ` Miles Bader @ 2010-04-06 9:20 ` Thomas Rast 1 sibling, 0 replies; 30+ messages in thread From: Thomas Rast @ 2010-04-06 9:20 UTC (permalink / raw) To: Junio C Hamano Cc: git, Johannes Schindelin, Eelis van der Weegen, Paul Mackerras, Miles Bader Junio C Hamano wrote: > > There is one difference between other uses of colors and color-words, but > I can imagine that ordinary people may not have even realized nor thought > about it. > > To people who are somewhat but not completely color-challenged (like > myself), it still helps to paint hunk headers in a color that is different > from the body text to make the boundary of each hunk more visible. Even > without the perception of exact color/hue, the contrast alone helps in > that case. I see now. My apologies for not thinking of that case! I'll make a new patch. -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2/2] gitk: add the equivalent of diff --color-words 2010-04-04 13:46 ` [PATCH 0/2] gitk --color-words Thomas Rast 2010-04-04 13:46 ` [PATCH 1/2] diff: add --word-diff option that generalizes --color-words Thomas Rast @ 2010-04-04 13:46 ` Thomas Rast 1 sibling, 0 replies; 30+ messages in thread From: Thomas Rast @ 2010-04-04 13:46 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Eelis van der Weegen, Junio C Hamano, Paul Mackerras, Miles Bader Use the newly added 'diff --word-diff=porcelain' to teach gitk a color-words mode, triggered by a checkbox. As an extra twist, automatically enable this word-diff support when the user mentions a word-diff related option on the command line. These options were previously ignored because they would break diff parsing. Signed-off-by: Thomas Rast <trast@student.ethz.ch> --- gitk | 37 ++++++++++++++++++++++++++++++++++--- 1 files changed, 34 insertions(+), 3 deletions(-) diff --git a/gitk b/gitk index 1f36a3e..05ed053 100755 --- a/gitk +++ b/gitk @@ -131,6 +131,7 @@ proc unmerged_files {files} { proc parseviewargs {n arglist} { global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env + global worddiff set vdatemode($n) 0 set vmergeonly($n) 0 @@ -168,7 +169,7 @@ proc parseviewargs {n arglist} { lappend diffargs $arg } "--raw" - "--patch-with-raw" - "--patch-with-stat" - - "--name-only" - "--name-status" - "--color" - "--color-words" - + "--name-only" - "--name-status" - "--color" - "--log-size" - "--pretty=*" - "--decorate" - "--abbrev-commit" - "--cc" - "-z" - "--header" - "--parents" - "--boundary" - "--no-color" - "-g" - "--walk-reflogs" - "--no-walk" - @@ -177,6 +178,11 @@ proc parseviewargs {n arglist} { # These cause our parsing of git log's output to fail, or else # they're options we want to set ourselves, so ignore them. } + "--color-words*" - "--word-diff*" { + # These trigger a word diff in the console interface, + # so help the user by enabling our own support + set worddiff 1 + } "--stat=*" - "--numstat" - "--shortstat" - "--summary" - "--check" - "--exit-code" - "--quiet" - "--topo-order" - "--full-history" - "--dense" - "--sparse" - @@ -2240,6 +2246,9 @@ proc makewindow {} { ${NS}::checkbutton .bleft.mid.ignspace -text [mc "Ignore space change"] \ -command changeignorespace -variable ignorespace pack .bleft.mid.ignspace -side left -padx 5 + ${NS}::checkbutton .bleft.mid.worddiff -text [mc "Word diff"] \ + -command changeworddiff -variable worddiff + pack .bleft.mid.worddiff -side left -padx 5 set ctext .bleft.bottom.ctext text $ctext -background $bgcolor -foreground $fgcolor \ -state disabled -font textfont \ @@ -7494,11 +7503,16 @@ proc changeignorespace {} { reselectline } +proc changeworddiff {} { + reselectline +} + proc getblobdiffs {ids} { global blobdifffd diffids env global diffinhdr treediffs global diffcontext global ignorespace + global worddiff global limitdiffs vfilelimit curview global diffencoding targetline diffnparents global git_version @@ -7515,6 +7529,9 @@ proc getblobdiffs {ids} { if {$ignorespace} { append cmd " -w" } + if {$worddiff} { + append cmd " --word-diff=porcelain" + } if {$limitdiffs && $vfilelimit($curview) ne {}} { set cmd [concat $cmd -- $vfilelimit($curview)] } @@ -7599,6 +7616,7 @@ proc getblobdiffline {bdf ids} { global ctext_file_names ctext_file_lines global diffinhdr treediffs mergemax diffnparents global diffencoding jump_to_here targetline diffline + global worddiff set nr 0 $ctext conf -state normal @@ -7727,15 +7745,22 @@ proc getblobdiffline {bdf ids} { # parse the prefix - one ' ', '-' or '+' for each parent set prefix [string range $line 0 [expr {$diffnparents - 1}]] set tag [expr {$diffnparents > 1? "m": "d"}] + set dowords [expr {$worddiff && $diffnparents == 1}] if {[string trim $prefix " -+"] eq {}} { # prefix only has " ", "-" and "+" in it: normal diff line set num [string first "-" $prefix] + if {$dowords} { + set line [string range $line 1 end] + } if {$num >= 0} { # removed line, first parent with line is $num if {$num >= $mergemax} { set num "max" } - $ctext insert end "$line\n" $tag$num + $ctext insert end "$line" $tag$num + if {!$dowords} { + $ctext insert end "\n" $tag$num + } } else { set tags {} if {[string first "+" $prefix] >= 0} { @@ -7759,8 +7784,13 @@ proc getblobdiffline {bdf ids} { incr diffline } } - $ctext insert end "$line\n" $tags + $ctext insert end "$line" $tags + if {!$dowords} { + $ctext insert end "\n" $tags + } } + } elseif {$dowords && $prefix eq "~"} { + $ctext insert end "\n" {} } else { # "\ No newline at end of file", # or something else we don't recognize @@ -11379,6 +11409,7 @@ if {[tk windowingsystem] eq "win32"} { set diffcolors {red "#00a000" blue} set diffcontext 3 set ignorespace 0 +set worddiff 0 set markbgcolor "#e0e0ff" set circlecolors {white blue gray blue blue} -- 1.7.0.3.521.ga1e9e ^ permalink raw reply related [flat|nested] 30+ messages in thread
end of thread, other threads:[~2010-04-20 17:05 UTC | newest] Thread overview: 30+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <cover.1269996525.git.trast@student.ethz> 2010-04-04 13:46 ` [PATCH 0/2] gitk --color-words Thomas Rast 2010-04-04 13:46 ` [PATCH 1/2] diff: add --word-diff option that generalizes --color-words Thomas Rast 2010-04-05 2:06 ` Junio C Hamano 2010-04-05 10:20 ` Thomas Rast 2010-04-05 18:46 ` Junio C Hamano 2010-04-05 18:53 ` Miles Bader 2010-04-12 13:07 ` [PATCH v3 0/2] gitk --color-words Thomas Rast 2010-04-12 13:07 ` [PATCH v3 1/3] diff: add --word-diff option that generalizes --color-words Thomas Rast 2010-04-12 16:31 ` Junio C Hamano 2010-04-13 9:27 ` Thomas Rast 2010-04-14 15:59 ` [PATCH v4 0/3] git-diff --word-diff/gitk --color-words Thomas Rast 2010-04-14 15:59 ` [PATCH v4 1/3] diff: add --word-diff option that generalizes --color-words Thomas Rast 2010-04-14 21:23 ` Junio C Hamano 2010-04-14 15:59 ` [PATCH v4 2/3] gitk: do not parse " >" context as submodule change Thomas Rast 2010-04-15 19:57 ` Jens Lehmann 2010-04-17 6:33 ` Paul Mackerras 2010-04-17 12:20 ` Thomas Rast 2010-04-19 1:08 ` Paul Mackerras 2010-04-19 16:27 ` [PATCH v4' 1/2] " Thomas Rast 2010-04-19 16:27 ` [PATCH v4' 2/2] gitk: add the equivalent of diff --color-words Thomas Rast 2010-04-19 17:22 ` [PATCH v4' 1/2] gitk: do not parse " >" context as submodule change Jens Lehmann 2010-04-20 17:05 ` Jens Lehmann 2010-04-14 15:59 ` [PATCH v4 3/3] gitk: add the equivalent of diff --color-words Thomas Rast 2010-04-17 6:35 ` Paul Mackerras 2010-04-17 6:55 ` Junio C Hamano 2010-04-12 13:07 ` [PATCH v3 2/3] gitk: do not parse " >" context as submodule change Thomas Rast 2010-04-12 13:25 ` [PATCH v3.1] " Thomas Rast 2010-04-12 13:07 ` [PATCH v3 3/3] gitk: add the equivalent of diff --color-words Thomas Rast 2010-04-06 9:20 ` [PATCH 1/2] diff: add --word-diff option that generalizes --color-words Thomas Rast 2010-04-04 13:46 ` [PATCH 2/2] gitk: add the equivalent of diff --color-words Thomas Rast
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).