public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Daniel Hahler <git@thequod•de>
Cc: git@vger•kernel.org, "René Scharfe" <l.s.r@web•de>,
	"Jeff King" <peff@peff•net>
Subject: Re: [PATCH] xdiff: trim common tail with -U0 after diff
Date: Fri, 23 Jun 2017 12:13:55 -0700	[thread overview]
Message-ID: <xmqqinjmecuk.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170623103612.4694-1-git@thequod.de> (Daniel Hahler's message of "Fri, 23 Jun 2017 12:36:12 +0200")

Daniel Hahler <git@thequod•de> 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,

next prev parent reply	other threads:[~2017-06-23 19:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-23 10:36 [PATCH] xdiff: trim common tail with -U0 after diff Daniel Hahler
2017-06-23 19:13 ` Junio C Hamano [this message]
2017-06-23 19:37   ` Stefan Beller
2017-06-23 19:44     ` Junio C Hamano
2017-06-23 20:39 ` René Scharfe
2017-06-23 22:57   ` Daniel Hahler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqinjmecuk.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@thequod$(echo .)de \
    --cc=git@vger$(echo .)kernel.org \
    --cc=l.s.r@web$(echo .)de \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox