Daniel Hahler <[email protected]> writes:
> When -U0 is used, trim_common_tail should be called after `xdl_diff`, so
> that `--indent-heuristic` (and other diff processing) works as expected.
I thought common-tail trimming was a hack/optimization to avoid
having to pass the whole thing to have xdl_diff() work on it? What
would we be gaining by unconditionally passing the whole thing first
and then still trimming?
> It also removes the check for `!(xecfg->flags & XDL_EMIT_FUNCCONTEXT)`
> added in e0876bca4, which does not appear to be necessary anymore after
> moving the trimming down.
The code was conditionally disabling the hack/optimization; with it
unconditionally disabled, of course it wouldn't be needed, no?
I could understand if the motivation and the proposed change were
"tail-trimming outlived its usefulness, so remove it altogether", or
"trail-trimming negatively affects the output by robbing useful
information that could be used by indent-heuristic, so disable it
when the heuristic is in use".
But neither is what this patch does, so I am sort of at loss.
> --- a/t/t4061-diff-indent.sh
> +++ b/t/t4061-diff-indent.sh
> @@ -116,6 +116,16 @@ test_expect_success 'prepare' '
> 4
> EOF
>
> + cat <<-\EOF >spaces-compacted-U0-expect &&
> + diff --git a/spaces.txt b/spaces.txt
> + --- a/spaces.txt
> + +++ b/spaces.txt
> + @@ -4,0 +5,3 @@ a
> + +b
> + +a
> + +
> + EOF
> +
> tr "_" " " <<-\EOF >functions-expect &&
> diff --git a/functions.c b/functions.c
> --- a/functions.c
> @@ -184,6 +194,11 @@ test_expect_success 'diff: --indent-heuristic with
> --histogram' '
> compare_diff spaces-compacted-expect out-compacted4
> '
>
> +test_expect_success 'diff: --indent-heuristic with -U0' '
> + git diff -U0 --indent-heuristic old new -- spaces.txt >out-compacted5 &&
> + compare_diff spaces-compacted-U0-expect out-compacted5
> +'
> +
> test_expect_success 'diff: ugly functions' '
> git diff --no-indent-heuristic old new -- functions.c >out &&
> compare_diff functions-expect out
> diff --git a/xdiff-interface.c b/xdiff-interface.c
> index d3f78ca2a..a7e0e3583 100644
> --- a/xdiff-interface.c
> +++ b/xdiff-interface.c
> @@ -125,16 +125,17 @@ static void trim_common_tail(mmfile_t *a, mmfile_t *b)
>
> int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
> xdemitconf_t const *xecfg, xdemitcb_t *xecb)
> {
> + int ret;
> mmfile_t a = *mf1;
> mmfile_t b = *mf2;
>
> if (mf1->size > MAX_XDIFF_SIZE || mf2->size > MAX_XDIFF_SIZE)
> return -1;
>
> - if (!xecfg->ctxlen && !(xecfg->flags & XDL_EMIT_FUNCCONTEXT))
> + ret = xdl_diff(&a, &b, xpp, xecfg, xecb);
> + if (ret && !xecfg->ctxlen)
> trim_common_tail(&a, &b);
> -
> - return xdl_diff(&a, &b, xpp, xecfg, xecb);
> + return ret;
> }
>
> int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
