Re: [RFC PATCH 1/3] diff: avoid redundantly clearing a flag

From: Stefan Beller <sbeller@google.com>
Date: Aug 14, 2017, 17:13 UTC
Message-ID: <CAGZ79kZZLOr70Ly=VGbic_5pj_cey6mfxwNd6WCtAAPZa5GTWQ@mail.gmail.com>
In-Reply-To: <ae81b1cf697b2d541eadd21b40ead497a0c32eb9.1502491372.git.jonathantanmy@google.com>

On Fri, Aug 11, 2017 at 3:49 PM, Jonathan Tan <jonathantanmy@google.com> wrote:

> No code in diff.c sets DIFF_SYMBOL_MOVED_LINE except in
> mark_color_as_moved(), so it is redundant to clear it for the current
> line. Therefore, clear it only for previous lines.

Oh that part. I remember debating with myself if I rather want to have the upper bound adjusted by one less (block_length instead of 'block_length + 1'), and then add a constant to 'buf[n - i];'

The patch as implemented is fine, too.

Show 23 quoted lines

> This makes a refactoring in a subsequent patch easier.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  diff.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/diff.c b/diff.c
> index f84346b47..4965ffbc4 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -895,7 +895,7 @@ static void mark_color_as_moved(struct diff_options *o,
>                 if (!match) {
>                         if (block_length < COLOR_MOVED_MIN_BLOCK_LENGTH &&
>                             o->color_moved != COLOR_MOVED_PLAIN) {
> -                               for (i = 0; i < block_length + 1; i++) {
> +                               for (i = 1; i < block_length + 1; i++) {
>                                         l = &o->emitted_symbols->buf[n - i];
>                                         l->flags &= ~DIFF_SYMBOL_MOVED_LINE;
>                                 }
> --
> 2.14.0.434.g98096fd7a8-goog
>