All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Patrick Palka <patrick@parcs.ath.cx>
Subject: Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks
Date: Thu, 18 Jun 2015 08:50:36 -0700	[thread overview]
Message-ID: <xmqqwpz1f22b.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1434388853-23915-1-git-send-email-patrick@parcs.ath.cx> (Patrick Palka's message of "Mon, 15 Jun 2015 13:20:53 -0400")

Patrick Palka <patrick@parcs.ath.cx> writes:

> Currently the diff-highlight script does not try to highlight hunks that
> have different numbers of removed/added lines.  But we can be a little
> smarter than that, without introducing much magic and complexity.
>
> In the case of unevenly-sized hunks, we could still highlight the first
> few (lexicographical) add/remove pairs.  It is not uncommon for hunks to
> have common "prefixes", and in such a case this change is very useful
> for spotting differences.
>
> Signed-off-by: Patrick Palka <patrick@parcs.ath.cx>
> ---

Patrick, "git shortlog --no-merges contrib/diff-highlight/" is your
friend to see who may be able to give you a good feedback.

Jeff, what do you think?

I have this nagging feeling that it is just as likely that two
uneven hunks align at the top as they align at the bottom, so while
this might not hurt it may not be the right approach for a better
solution, in the sense that when somebody really wants to do a
better solution, this change and the original code may need to be
ripped out and redone from scratch.

>  contrib/diff-highlight/diff-highlight | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight
> index ffefc31..0dfbebd 100755
> --- a/contrib/diff-highlight/diff-highlight
> +++ b/contrib/diff-highlight/diff-highlight
> @@ -88,22 +88,30 @@ sub show_hunk {
>  		return;
>  	}
>  
> -	# If we have mismatched numbers of lines on each side, we could try to
> -	# be clever and match up similar lines. But for now we are simple and
> -	# stupid, and only handle multi-line hunks that remove and add the same
> -	# number of lines.
> -	if (@$a != @$b) {
> -		print @$a, @$b;
> -		return;
> -	}
> +	# We match up the first MIN(a, b) lines on each side.
> +	my $c = @$a < @$b ? @$a : @$b;
>  
> +	# Highlight each pair, and print each removed line of that pair.
>  	my @queue;
> -	for (my $i = 0; $i < @$a; $i++) {
> +	for (my $i = 0; $i < $c; $i++) {
>  		my ($rm, $add) = highlight_pair($a->[$i], $b->[$i]);
>  		print $rm;
>  		push @queue, $add;
>  	}
> +
> +	# Print the remaining unmatched removed lines of the hunk.
> +	for (my $i = $c; $i < @$a; $i++) {
> +		print $a->[$i];
> +	}
> +
> +	# Print the added lines of each highlighted pair.
>  	print @queue;
> +
> +	# Print the remaining unmatched added lines of the hunk.
> +	for (my $i = $c; $i < @$b; $i++) {
> +		print $b->[$i];
> +	}
> +
>  }
>  
>  sub highlight_pair {

  reply	other threads:[~2015-06-18 15:50 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-15 17:20 [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks Patrick Palka
2015-06-18 15:50 ` Junio C Hamano [this message]
2015-06-18 16:28   ` Patrick Palka
2015-06-18 18:08     ` Junio C Hamano
2015-06-18 19:04       ` Jeff King
2015-06-18 20:14         ` Patrick Palka
2015-06-18 20:45           ` Jeff King
2015-06-18 21:23             ` Jeff King
2015-06-18 21:39               ` Junio C Hamano
2015-06-18 22:25               ` Patrick Palka
2015-06-19  3:54               ` Jeff King
2015-06-19  4:49                 ` Junio C Hamano
2015-06-19  5:32                   ` Jeff King
2015-06-19  7:34                     ` Jeff King
2015-06-19 11:38                       ` Jeff King
2015-06-19 17:20                     ` Junio C Hamano
2015-06-18 23:06             ` Patrick Palka
2015-06-18 20:23       ` Patrick Palka
2015-06-18 19:08     ` Jeff King
2015-06-18 20:27       ` Patrick Palka

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=xmqqwpz1f22b.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=patrick@parcs.ath.cx \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.