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

From: Junio C Hamano <gitster@pobox•com>
To: David Kastrup <dak@gnu•org>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] Require 0 context lines in git-blame algorithm
Date: Fri, 27 May 2016 13:59:03 -0700	[thread overview]
Message-ID: <xmqqtwhjp694.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1464358592-5409-1-git-send-email-dak@gnu.org> (David Kastrup's message of "Fri, 27 May 2016 16:16:32 +0200")

David Kastrup <dak@gnu•org> writes:

> Previously, the core part of git blame -M required 1 context line.
> There is no rationale to be found in the code (one guess would be that
> the old blame algorithm was unable to deal with immediately adjacent
> regions), and it causes artifacts like discussed in the thread
> <URL:http://thread.gmane.org/gmane.comp.version-control.git/255289/>

The only thing that remotely hints why we thought a non-zero context
was a good idea was this:

http://thread.gmane.org/gmane.comp.version-control.git/28336/focus=28580

in which I said:

 | we may need to use a handful surrounding context lines for
 | better identification of copy source by the "ciff" algorithm but
 | that is a minor implementation detail.

But I do not think the amount of context affects the quality of the
match.  So it could be that it was completely a misguided attempt
since the very beginning, cee7f245 (git-pickaxe: blame rewritten.,
2006-10-19), which allowed the caller to specify context when
calling compare_buffer(), the function that corresponds to
diff_hunks() in today's code.

> ---
>  builtin/blame.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

I totally forgot about the discussion around $gmane/255289; thanks
for bringing this back again.

As usual, we'd need your sign-off to use this patch.

Thanks.


> diff --git a/builtin/blame.c b/builtin/blame.c
> index 21f42b0..a3f6874 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -134,7 +134,7 @@ struct progress_info {
>  	int blamed_lines;
>  };
>  
> -static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,
> +static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b,
>  		      xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)
>  {
>  	xpparam_t xpp = {0};
> @@ -142,7 +142,6 @@ static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,
>  	xdemitcb_t ecb = {NULL};
>  
>  	xpp.flags = xdl_opts;
> -	xecfg.ctxlen = ctxlen;
>  	xecfg.hunk_func = hunk_func;
>  	ecb.priv = cb_data;
>  	return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);
> @@ -980,7 +979,7 @@ static void pass_blame_to_parent(struct scoreboard *sb,
>  	fill_origin_blob(&sb->revs->diffopt, target, &file_o);
>  	num_get_patch++;
>  
> -	if (diff_hunks(&file_p, &file_o, 0, blame_chunk_cb, &d))
> +	if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d))
>  		die("unable to generate diff (%s -> %s)",
>  		    oid_to_hex(&parent->commit->object.oid),
>  		    oid_to_hex(&target->commit->object.oid));
> @@ -1129,7 +1128,7 @@ static void find_copy_in_blob(struct scoreboard *sb,
>  	 * file_p partially may match that image.
>  	 */
>  	memset(split, 0, sizeof(struct blame_entry [3]));
> -	if (diff_hunks(file_p, &file_o, 1, handle_split_cb, &d))
> +	if (diff_hunks(file_p, &file_o, handle_split_cb, &d))
>  		die("unable to generate diff (%s)",
>  		    oid_to_hex(&parent->commit->object.oid));
>  	/* remainder, if any, all match the preimage */

     prev parent reply	other threads:[~2016-05-27 20:59 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-27 14:16 [PATCH] Require 0 context lines in git-blame algorithm David Kastrup
2016-05-27 20:59 ` Junio C Hamano [this message]

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=xmqqtwhjp694.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=dak@gnu$(echo .)org \
    --cc=git@vger$(echo .)kernel.org \
    /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