Re: [PATCH v4 0/3] "diff --color-moved" with yet another heuristic

From: Stefan Beller <sbeller@google.com>
Date: Aug 16, 2017, 05:55 UTC
Message-ID: <CAGZ79kYscfRQx8x4rK9R-V73aa-QMSZraO_0ZJDvB+aFDWLi6Q@mail.gmail.com>
In-Reply-To: <cover.1502846454.git.jonathantanmy@google.com>

On Tue, Aug 15, 2017 at 6:27 PM, Jonathan Tan <jonathantanmy@google.com> wrote:

Show 9 quoted lines

> These patches are on sb/diff-color-move.
>
> Patches 1 and 2 are unchanged, except for some line wrapping in a test
> in patch 2.
>
> This has been updated to use the same alphanumeric heuristic as blame
> (20 alnum characters). I tried it out and I thought the results were
> reasonable in a patch set that I'm working on (the pack-related function
> refactoring one).

ok, great!

> As for refactoring blame.c and this file, I'm not sure where best to put
> the new function, so I've added a NEEDSWORK for now.

Let's just cook this heuristic a bit and see if we want to refactor them or tweak them differently.

Show 6 quoted lines

> As for detecting block boundaries in adjust_last_block(), I've left it
> as-is for now. I think it's clearer if the parent function provides that
> information, since it already tracks that. In addition, we avoid corner
> cases such as what happens if the block is at the start of the diff
> output (we must ensure that we don't read off the beginning edge, for
> example).

ok.

Thanks! Stefan

Show 15 quoted lines

>
> Jonathan Tan (3):
>   diff: avoid redundantly clearing a flag
>   diff: respect MIN_BLOCK_LENGTH for last block
>   diff: define block by number of alphanumeric chars
>
>  Documentation/diff-options.txt |   8 +-
>  diff.c                         |  47 ++++++--
>  diff.h                         |   2 +-
>  t/t4015-diff-whitespace.sh     | 261 ++++++++++++++++++++++++++++++-----------
>  4 files changed, 236 insertions(+), 82 deletions(-)
>
> --
> 2.14.1.480.gb18f417b89-goog
>