Stefan Beller <[email protected]> writes:

>  static void emit_add_line(const char *reset,
>                         struct emit_callback *ecbdata,
>                         const char *line, int len)
>  {
> -     emit_line_checked(reset, ecbdata, line, len,
> -                       DIFF_FILE_NEW, WSEH_NEW, '+');
> +     unsigned flags = WSEH_NEW | ecbdata->ws_rule;
> +     if (new_blank_line_at_eof(ecbdata, line, len))
> +             flags |= DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF;
> +
> +     emit_diff_symbol(ecbdata->opt, DIFF_SYMBOL_PLUS, line, len, flags);
>  }

This is a bit unsatisfactory that the original code didn't even have
to make a call to new_blank_line_at_eof() at all when we know we are
not checking for / coloring whitespace errors, but the function is
called unconditionally in the new code.

> diff --git a/diff.h b/diff.h
> index 5be1ee77a7..8483ca0991 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -148,9 +148,9 @@ struct diff_options {
>       int abbrev;
>       int ita_invisible_in_index;
>  /* white-space error highlighting */
> -#define WSEH_NEW 1
> -#define WSEH_CONTEXT 2
> -#define WSEH_OLD 4
> +#define WSEH_NEW (1<<12)
> +#define WSEH_CONTEXT (1<<13)
> +#define WSEH_OLD (1<<14)

Hopefully this is a safe conversion, as everything should come from
parse_ws_error_highlight() that uses these constants.

But this is brittle; there must be some hint to future readers of
this code why these bits begin at 12th (it is linked to 07777 we saw
earlier); otherwise when they change something that needs to widen
that 07777, they will forget to shift these up, which would screw
up everything that is carefully laid out here.


Reply via email to