Re: Re* Consequences of CRLF in index?

From: Stefan Beller <sbeller@google.com>
Date: Oct 27, 2017, 17:06 UTC
Message-ID: <CAGZ79kY0eL42fC+QC9ifvHt-Z_PO7FfVYaEZysGWq+LqLcjduA@mail.gmail.com>
In-Reply-To: <xmqqinf1ce3i.fsf_-_@gitster.mtv.corp.google.com>

On Thu, Oct 26, 2017 at 11:13 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> Subject: [PATCH] xdiff: reassign xpparm_t.flags bits
>
> We have packed the bits too tightly in such a way that it is not
> easy to add a new type of whitespace ignoring option, a new type
> of LCS algorithm, or a new type of post-cleanup heuristics.
>
> Reorder bits a bit to give room for these three classes of options
> to grow.
>
> While at it, add a comment in front of the bit definitions to
> clarify in which structure these defined bits may appear.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Reviewed-by: Stefan Beller <sbeller@google.com>

Show 16 quoted lines

> ---
>  xdiff/xdiff.h | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
> index b090ad8eac..457cac32d8 100644
> --- a/xdiff/xdiff.h
> +++ b/xdiff/xdiff.h
> @@ -27,22 +27,24 @@
>  extern "C" {
>  #endif /* #ifdef __cplusplus */
>
> +/* xpparm_t.flags */
> +#define XDF_NEED_MINIMAL (1 << 0)
>
> -#define XDF_NEED_MINIMAL (1 << 1)

The whitespace flags are also stored in xpparm_t, though these flags can also come in from other sources (e.g. we just pass it in manually via the interface).

Show 8 quoted lines

>  #define XDF_IGNORE_WHITESPACE (1 << 2)
>  #define XDF_IGNORE_WHITESPACE_CHANGE (1 << 3)
>  #define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 4)
>  #define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL)
>
> -#define XDF_PATIENCE_DIFF (1 << 5)
> -#define XDF_HISTOGRAM_DIFF (1 << 6)
> +#define XDF_IGNORE_BLANK_LINES (1 << 7)

Is XDF_IGNORE_BLANK_LINES a whitespace option, just on the lines level instead of the inside one line? I think it still makes sense to keep it here, slightly separated from the IGNORE flags.

Show 13 quoted lines

> +#define XDF_PATIENCE_DIFF (1 << 14)
> +#define XDF_HISTOGRAM_DIFF (1 << 15)
>  #define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF)
>  #define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK)
>
> -#define XDF_IGNORE_BLANK_LINES (1 << 7)
> -
> -#define XDF_INDENT_HEURISTIC (1 << 8)
> +#define XDF_INDENT_HEURISTIC (1 << 23)
>
> +/* xdemitconf_t.flags */
>  #define XDL_EMIT_FUNCNAMES (1 << 0)
>  #define XDL_EMIT_FUNCCONTEXT (1 << 2)

This looked like it was carefully crafted to avoid accidental bit overlap with XDF_NEED_MINIMAL; but these are in different flag fields, this should be fine.

Thanks, Stefan