git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 4/8] gitweb: Extract print_sidebyside_diff_lines()

From
Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
Date
Apr 11, 2012, 21:18 UTC
Message-ID
<1334179124-14258-5-git-send-email-michal.kiedrowicz@gmail.com>
In-Reply-To
<1334179124-14258-1-git-send-email-michal.kiedrowicz@gmail.com>

Currently, print_sidebyside_diff_chunk() does two things: it accumulates diff lines and prints them. Accumulation may be used to perform additional operations on diff lines, so it makes sense to split these two things. Thus, whole code that formats and prints diff lines in the 'side-by-side' manner is moved out of print_sidebyside_diff_chunk() to a separate subroutine and two conditions that control printing diff liens are merged.

Thanks to that, we can easily (in later patches) replace call to that subroutine with a call to more generic print_diff_lines() that will control whether 'inline' or 'side-by-side' diff should be printed.

As a side effect, context lines are printed just before printing added and removed lines, and at the end of chunk (previously, they were printed immediately on the class change). However, this doesn't change gitweb output.

The outcome of this patch is that print_sidebyside_diff_chunk() is now much shorter and easier to read.

While at it, drop the '# assume that it is change' comment. According to Jakub Narębski:

	What I meant here when I was writing it that they are lines that
	changed between two versions, like '!' in original (not unified)
	context format.
	We can omit this comment.
Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   96 +++++++++++++++++++++++++++------------------------
 1 files changed, 51 insertions(+), 45 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index db1f698..8271749 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5002,6 +5002,52 @@ sub git_difftree_body {
 	print "</table>\n";
 }
 
+# Print context lines and then rem/add lines in a side-by-side manner.
+sub print_sidebyside_diff_lines {
+	my ($ctx, $rem, $add) = @_;
+
+	# print context block before add/rem block
+	if (@$ctx) {
+		print join '',
+			'<div class="chunk_block ctx">',
+				'<div class="old">',
+				@$ctx,
+				'</div>',
+				'<div class="new">',
+				@$ctx,
+				'</div>',
+			'</div>';
+	}
+
+	if (!@$add) {
+		# pure removal
+		print join '',
+			'<div class="chunk_block rem">',
+				'<div class="old">',
+				@$rem,
+				'</div>',
+			'</div>';
+	} elsif (!@$rem) {
+		# pure addition
+		print join '',
+			'<div class="chunk_block add">',
+				'<div class="new">',
+				@$add,
+				'</div>',
+			'</div>';
+	} else {
+		print join '',
+			'<div class="chunk_block chg">',
+				'<div class="old">',
+				@$rem,
+				'</div>',
+				'<div class="new">',
+				@$add,
+				'</div>',
+			'</div>';
+	}
+}
+
 sub print_sidebyside_diff_chunk {
 	my @chunk = @_;
 	my (@ctx, @rem, @add);
@@ -5028,51 +5074,11 @@ sub print_sidebyside_diff_chunk {
 			next;
 		}
 
-		## print from accumulator when type of class of lines change
-		# empty contents block on start rem/add block, or end of chunk
-		if (@ctx && (!$class || $class eq 'rem' || $class eq 'add')) {
-			print join '',
-				'<div class="chunk_block ctx">',
-					'<div class="old">',
-					@ctx,
-					'</div>',
-					'<div class="new">',
-					@ctx,
-					'</div>',
-				'</div>';
-			@ctx = ();
-		}
-		# empty add/rem block on start context block, or end of chunk
-		if ((@rem || @add) && (!$class || $class eq 'ctx')) {
-			if (!@add) {
-				# pure removal
-				print join '',
-					'<div class="chunk_block rem">',
-						'<div class="old">',
-						@rem,
-						'</div>',
-					'</div>';
-			} elsif (!@rem) {
-				# pure addition
-				print join '',
-					'<div class="chunk_block add">',
-						'<div class="new">',
-						@add,
-						'</div>',
-					'</div>';
-			} else {
-				# assume that it is change
-				print join '',
-					'<div class="chunk_block chg">',
-						'<div class="old">',
-						@rem,
-						'</div>',
-						'<div class="new">',
-						@add,
-						'</div>',
-					'</div>';
-			}
-			@rem = @add = ();
+		## print from accumulator when have some add/rem lines or end
+		# of chunk (flush context lines)
+		if (!$class || ((@rem || @add) && $class eq 'ctx')) {
+			print_sidebyside_diff_lines(\@ctx, \@rem, \@add);
+			@ctx = @rem = @add = ();
 		}
 
 		## adding lines to accumulator

-- 
1.7.8.4
Previous: Michał KiedrowiczNext: Michał Kiedrowicz
Message 5 of 10 in “Highlight interesting parts of diff”
  1. 0/8 Highlight interesting parts of diffMichał Kiedrowicz, Apr 11, 2012
  2. 1/8 gitweb: Use descriptive names in esc_html_hl_regions()Michał Kiedrowicz, Apr 11, 2012
  3. 2/8 gitweb: esc_html_hl_regions(): Don't create empty <span> elementsMichał Kiedrowicz, Apr 11, 2012
  4. 3/8 gitweb: Pass esc_html_hl_regions() options to esc_html()Michał Kiedrowicz, Apr 11, 2012
  5. 4/8 gitweb: Extract print_sidebyside_diff_lines()Michał Kiedrowicz, Apr 11, 2012
  6. 5/8 gitweb: Use print_diff_chunk() for both side-by-side and inline diffsMichał Kiedrowicz, Apr 11, 2012
  7. 6/8 gitweb: Push formatting diff lines to print_diff_chunk()Michał Kiedrowicz, Apr 11, 2012
  8. 7/8 gitweb: Highlight interesting parts of diffMichał Kiedrowicz, Apr 11, 2012
  9. 8/8 gitweb: Refinement highlightning in combined diffsMichał Kiedrowicz, Apr 11, 2012
  10. Junio C HamanoApr 11, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.