On Wed, 31 May 2017 17:24:29 -0700 Stefan Beller <sbel...@google.com> wrote:
> When a patch consists mostly of moving blocks of code around, it can > be quite tedious to ensure that the blocks are moved verbatim, and not > undesirably modified in the move. To that end, color blocks that are > moved within the same patch differently. For example (OM, del, add, > and NM are different colors): [snip] Junio asks "are we happy with these changes" [1] and my answer is, in general, yes - this seems like a very useful feature to have, and I'm OK with the current design. I do feel a bit of unease at how the emitted strings are collected without many guarantees as to their contents (e.g. whether they are full lines or even whether they originate from the text of a file), but this is already true for the existing code. The potential danger is that we are now relying more on the format of these strings, but we don't plan to do anything other than to color them, so this seems fine. I would also prefer if there was only one coloring method, to ease testing, but I can tolerate the current multiplicity of options. I think there was some discussion at trying to avoid indicating that small blocks (consisting of few non-whitespace characters) are moved, but I think that that can be added later. [1] https://public-inbox.org/git/cagz79ky2z-fjyxczbzheu1hchlkkkdjecdmwsp-hkn0tjub...@mail.gmail.com/ > --- a/Documentation/diff-options.txt > +++ b/Documentation/diff-options.txt > @@ -231,6 +231,38 @@ ifdef::git-diff[] > endif::git-diff[] > It is the same as `--color=never`. > > +--color-moved[=<mode>]:: > + Moved lines of code are colored differently. > +ifdef::git-diff[] > + It can be changed by the `diff.colorMoved` configuration setting. > +endif::git-diff[] > + The <mode> defaults to 'no' if the option is not given > + and to 'adjacentbounds' if the option with no mode is given. > + The mode must be one of: > ++ > +-- > +no:: > + Moved lines are not highlighted. > +nobounds:: > + Any line that is added in one location and was removed > + in another location will be colored with 'color.diff.newmoved'. > + Similarly 'color.diff.oldmoved' will be used for removed lines Probably best to consistently capitalize newMoved and oldMoved. > +static unsigned get_line_hash(struct diff_line *line, unsigned ignore_ws) > +{ > + static struct strbuf sb = STRBUF_INIT; > + > + if (ignore_ws) { > + strbuf_reset(&sb); > + get_ws_cleaned_string(line, &sb); > + return memhash(sb.buf, sb.len); > + } else { > + return memhash(line->line, line->len); > + } > +} Can be written without the "else". > +test_expect_success 'move detection does not mess up colored words' ' Probably name this test "--color-moved has no effect when used with --word-diff".
