git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michał Kiedrowicz" <michal.kiedrowicz@gmail.com>
To: git@vger.kernel.org
Cc: "Michał Kiedrowicz" <michal.kiedrowicz@gmail.com>
Subject: [PATCH 8/8] gitweb: Highlight combined diffs
Date: Fri, 10 Feb 2012 10:18:14 +0100	[thread overview]
Message-ID: <1328865494-24415-9-git-send-email-michal.kiedrowicz@gmail.com> (raw)
In-Reply-To: <1328865494-24415-1-git-send-email-michal.kiedrowicz@gmail.com>

The highlightning of combined diffs is currently disabled.  This is
because output from a combined diff is much harder to highlight because
it's not obvious which removed and added lines should be compared.

Moreover, code that compares added and removed lines doesn't care about
combined diffs. It only skips first +/- character, treating second +/-
as a line content.

Let's start with a simple case: only highlight changes that come from
one parent, i.e. when every removed line has a corresponding added line
for the same parent.  This way the highlightning cannot get wrong. For
example, following diffs would be highlighted:

	- removed line for first parent
	+ added line for first parent
	  context line
	 -removed line for second parent
	 +added line for second parent

or

	- removed line for first parent
	 -removed line for second parent
	+ added line for first parent
	 +added line for second parent

but following output will not:

	- removed line for first parent
	 -removed line for second parent
	 +added line for second parent
	++added line for both parents

Further changes may introduce more intelligent approach that better
handles combined diffs.

Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
---
 gitweb/gitweb.perl |   40 +++++++++++++++++++++++++++++++++++++---
 1 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1a5b454..2b6cb9e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4944,13 +4944,16 @@ sub esc_html_mark_range {
 
 # Format removed and added line, mark changed part and HTML-format them.
 sub format_rem_add_line {
-	my ($rem, $add) = @_;
+	my ($rem, $add, $is_combined) = @_;
 	my @r = split(//, $rem);
 	my @a = split(//, $add);
 	my ($esc_rem, $esc_add);
 	my ($prefix, $suffix_rem, $suffix_add) = (1, $#r, $#a);
 	my ($prefix_is_space, $suffix_is_space) = (1, 1);
 
+	# In combined diff we must ignore two +/- characters.
+	$prefix = 2 if ($is_combined);
+
 	while ($prefix < @r && $prefix < @a) {
 		last if ($r[$prefix] ne $a[$prefix]);
 
@@ -4988,11 +4991,42 @@ sub format_ctx_rem_add_lines {
 	my ($ctx, $rem, $add, $is_combined) = @_;
 	my (@new_ctx, @new_rem, @new_add);
 	my $num_add_lines = @$add;
+	my $can_highlight;
+
+	# Highlight if every removed line has a corresponding added line.
+	if ($num_add_lines > 0 && $num_add_lines == @$rem) {
+		$can_highlight = 1;
+
+		# Highlight lines in combined diff only if the chunk contains
+		# diff between the same version, e.g.
+		#
+		#    - a
+		#   -  b
+		#    + c
+		#   +  d
+		#
+		# Otherwise the highlightling would be confusing.
+		if ($is_combined) {
+			for (my $i = 0; $i < $num_add_lines; $i++) {
+				my $prefix_rem = substr($rem->[$i], 0, 2);
+				my $prefix_add = substr($add->[$i], 0, 2);
+
+				$prefix_rem =~ s/-/+/g;
+
+				if ($prefix_rem ne $prefix_add) {
+					$can_highlight = 0;
+					last;
+				}
+			}
+		}
+	} else {
+		$can_highlight = 0;
+	}
 
-	if (!$is_combined && $num_add_lines > 0 && $num_add_lines == @$rem) {
+	if ($can_highlight) {
 		for (my $i = 0; $i < $num_add_lines; $i++) {
 			my ($line_rem, $line_add) = format_rem_add_line(
-				$rem->[$i], $add->[$i]);
+				$rem->[$i], $add->[$i], $is_combined);
 			push @new_rem, $line_rem;
 			push @new_add, $line_add;
 		}
-- 
1.7.3.4

  parent reply	other threads:[~2012-02-10  9:19 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-10  9:18 [PATCH 0/8] gitweb: Highlight interesting parts of diff Michał Kiedrowicz
2012-02-10  9:18 ` [PATCH 1/8] gitweb: Extract print_sidebyside_diff_lines() Michał Kiedrowicz
2012-02-11 15:20   ` Jakub Narebski
2012-02-11 23:03     ` Michał Kiedrowicz
2012-02-10  9:18 ` [PATCH 2/8] gitweb: Use print_diff_chunk() for both side-by-side and inline diffs Michał Kiedrowicz
2012-02-11 15:53   ` Jakub Narebski
2012-02-11 23:16     ` Michał Kiedrowicz
2012-02-25  9:00     ` Michał Kiedrowicz
2012-02-10  9:18 ` [PATCH 3/8] gitweb: Move HTML-formatting diff line back to process_diff_line() Michał Kiedrowicz
2012-02-11 16:02   ` Jakub Narebski
2012-02-10  9:18 ` [PATCH 4/8] gitweb: Push formatting diff lines to print_diff_chunk() Michał Kiedrowicz
2012-02-11 16:29   ` Jakub Narebski
2012-02-11 23:20     ` Michał Kiedrowicz
2012-02-11 23:30       ` Michał Kiedrowicz
2012-02-10  9:18 ` [PATCH 5/8] gitweb: Format diff lines just before printing Michał Kiedrowicz
2012-02-11 17:14   ` Jakub Narebski
2012-02-11 23:38     ` Michał Kiedrowicz
2012-02-10  9:18 ` [PATCH 6/8] gitweb: Highlight interesting parts of diff Michał Kiedrowicz
2012-02-10 13:23   ` Jakub Narebski
2012-02-10 14:15     ` Michał Kiedrowicz
2012-02-10 14:55       ` Jakub Narebski
2012-02-10 17:33         ` Michał Kiedrowicz
2012-02-10 22:52           ` Splitting gitweb (was: Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff) Jakub Narebski
2012-02-10 20:24         ` [PATCH 6/8] gitweb: Highlight interesting parts of diff Jeff King
2012-02-14  6:54     ` Michal Kiedrowicz
2012-02-14  7:14       ` Junio C Hamano
2012-02-14  8:20         ` Jeff King
2012-02-10 20:20   ` Jeff King
2012-02-10 21:29     ` Michał Kiedrowicz
2012-02-10 21:32       ` Jeff King
2012-02-10 21:36         ` Michał Kiedrowicz
2012-02-10 21:47         ` [PATCH] diff-highlight: Work for multiline changes too Michał Kiedrowicz
2012-02-13 22:27           ` Jeff King
2012-02-13 22:28             ` [PATCH 1/5] diff-highlight: make perl strict and warnings fatal Jeff King
2012-02-13 22:32             ` [PATCH 2/5] diff-highlight: don't highlight whole lines Jeff King
2012-02-14  6:35               ` Michal Kiedrowicz
2012-02-13 22:33             ` [PATCH 3/5] diff-highlight: refactor to prepare for multi-line hunks Jeff King
2012-02-13 22:36             ` [PATCH 4/5] diff-highlight: match " Jeff King
2012-02-13 22:37             ` [PATCH 5/5] diff-highlight: document some non-optimal cases Jeff King
2012-02-14  6:48               ` Michal Kiedrowicz
2012-02-14  0:05             ` [PATCH] diff-highlight: Work for multiline changes too Junio C Hamano
2012-02-14  0:22               ` Jeff King
2012-02-14  1:19                 ` Junio C Hamano
2012-02-14  6:04                   ` Jeff King
2012-02-14  6:28             ` Michal Kiedrowicz
2012-02-10 21:56     ` [PATCH 6/8] gitweb: Highlight interesting parts of diff Jakub Narebski
2012-02-11 23:45   ` Jakub Narebski
2012-02-12 10:42     ` Jakub Narebski
2012-02-13  6:54       ` Michal Kiedrowicz
2012-02-13 19:58         ` Jakub Narebski
2012-02-13 21:10           ` Michał Kiedrowicz
2012-02-13  6:41     ` Michal Kiedrowicz
2012-02-13 18:44       ` Jakub Narebski
2012-02-13 21:09         ` Michał Kiedrowicz
2012-02-14 17:31           ` Jakub Narebski
2012-02-14 18:23             ` Michał Kiedrowicz
2012-02-14 18:52               ` Jeff King
2012-02-14 20:04                 ` Michał Kiedrowicz
2012-02-14 20:38                   ` Jeff King
2012-02-10  9:18 ` [PATCH 7/8] gitweb: Use different colors to present marked changes Michał Kiedrowicz
2012-02-12  0:11   ` Jakub Narebski
2012-02-13  6:46     ` Michal Kiedrowicz
2012-02-10  9:18 ` Michał Kiedrowicz [this message]
2012-02-12  0:03   ` [PATCH 8/8] gitweb: Highlight combined diffs Jakub Narebski
2012-02-13  6:48     ` Michal Kiedrowicz
2012-02-11 18:32 ` [PATCH 0/8] gitweb: Highlight interesting parts of diff Jakub Narebski
2012-02-11 22:56   ` Michał Kiedrowicz

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=1328865494-24415-9-git-send-email-michal.kiedrowicz@gmail.com \
    --to=michal.kiedrowicz@gmail.com \
    --cc=git@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).