[PATCH 0/2] gitweb: Simplify dealing with raw diff output

[PATCH 0/2] gitweb: Simplify dealing with raw diff output

[Jakub Narebski]

This series of patches simplifies (and improves) gitweb code dealing
with (parsed) raw diff output format.

See individual patches for more detail.


Table of contents:
 [PATCH 1/2] gitweb: Always set 'from_file' and 'to_file'
             in parse_difftree_raw_line 
 [PATCH 2/2] gitweb: Add 'status_str' to parse_difftree_raw_line output

Diffstat:
 gitweb/gitweb.perl |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)

-- 
Jakub Narebski
Poland

[PATCH 1/2] gitweb: Always set 'from_file' and 'to_file' in parse_difftree_raw_line

[Jakub Narebski]

Always set 'from_file' and 'to_file' keys when parsing raw diff output
format line, even if filename didn't change (file was not renamed).
This allows for simpler code (and no problems with file named '0').

Use
  $diffinfo->{'from_file'}
instead of
  $diffinfo->{'from_file'} || $diffinfo->{'file'}
from now on.

While at it, replace (for merge commits)
  $diffinfo->{'from_file'}[$i] || $diffinfo->{'to_file'}
by
  defined $diffinfo->{'from_file'}[$i] ?
          $diffinfo->{'from_file'}[$i] :
          $diffinfo->{'to_file'};
to have no problems woth file named '0'.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2e00756..79ea7ec 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1995,7 +1995,7 @@ sub parse_difftree_raw_line {
 		if ($res{'status'} eq 'R' || $res{'status'} eq 'C') { # renamed or copied
 			($res{'from_file'}, $res{'to_file'}) = map { unquote($_) } split("\t", $7);
 		} else {
-			$res{'file'} = unquote($7);
+			$res{'from_file'} = $res{'to_file'} = $res{'file'} = unquote($7);
 		}
 	}
 	# '::100755 100755 100755 60e79ca1b01bc8b057abe17ddab484699a7f5fdb 94067cc5f73388f33722d52ae02f44692bc07490 94067cc5f73388f33722d52ae02f44692bc07490 MR	git-gui/git-gui.sh'
@@ -2062,7 +2062,10 @@ sub parse_from_to_diffinfo {
 		fill_from_file_info($diffinfo, @parents)
 			unless exists $diffinfo->{'from_file'};
 		for (my $i = 0; $i < $diffinfo->{'nparents'}; $i++) {
-			$from->{'file'}[$i] = $diffinfo->{'from_file'}[$i] || $diffinfo->{'to_file'};
+			$from->{'file'}[$i] = 
+				defined $diffinfo->{'from_file'}[$i] ?
+				        $diffinfo->{'from_file'}[$i] :
+				        $diffinfo->{'to_file'};
 			if ($diffinfo->{'status'}[$i] ne "A") { # not new (added) file
 				$from->{'href'}[$i] = href(action=>"blob",
 				                           hash_base=>$parents[$i],
@@ -2074,7 +2077,7 @@ sub parse_from_to_diffinfo {
 		}
 	} else {
 		# ordinary (not combined) diff
-		$from->{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
+		$from->{'file'} = $diffinfo->{'from_file'};
 		if ($diffinfo->{'status'} ne "A") { # not new (added) file
 			$from->{'href'} = href(action=>"blob", hash_base=>$hash_parent,
 			                       hash=>$diffinfo->{'from_id'},
@@ -2084,7 +2087,7 @@ sub parse_from_to_diffinfo {
 		}
 	}
 
-	$to->{'file'} = $diffinfo->{'to_file'} || $diffinfo->{'file'};
+	$to->{'file'} = $diffinfo->{'to_file'};
 	if (!is_deleted($diffinfo)) { # file exists in result
 		$to->{'href'} = href(action=>"blob", hash_base=>$hash,
 		                     hash=>$diffinfo->{'to_id'},
@@ -2829,7 +2832,7 @@ sub is_patch_split {
 	my ($diffinfo, $patchinfo) = @_;
 
 	return defined $diffinfo && defined $patchinfo
-		&& ($diffinfo->{'to_file'} || $diffinfo->{'file'}) eq $patchinfo->{'to_file'};
+		&& $diffinfo->{'to_file'} eq $patchinfo->{'to_file'};
 }
 
 
@@ -4667,8 +4670,8 @@ sub git_blobdiff {
 		}
 
 		%diffinfo = parse_difftree_raw_line($difftree[0]);
-		$file_parent ||= $diffinfo{'from_file'} || $file_name || $diffinfo{'file'};
-		$file_name   ||= $diffinfo{'to_file'}   || $diffinfo{'file'};
+		$file_parent ||= $diffinfo{'from_file'} || $file_name;
+		$file_name   ||= $diffinfo{'to_file'};
 
 		$hash_parent ||= $diffinfo{'from_id'};
 		$hash        ||= $diffinfo{'to_id'};
-- 
1.5.3.4

[PATCH 2/2] gitweb: Add 'status_str' to parse_difftree_raw_line output

[Jakub Narebski]

Add 'status_str' to diffinfo output, which stores status (also for
merge commit) as a string.  This allows for easy checking if there is
given status among all for merge commit, e.g.
  $diffinfo->{'status_str'} =~ /D/;

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 79ea7ec..e36dec1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1990,7 +1990,7 @@ sub parse_difftree_raw_line {
 		$res{'to_mode'} = $2;
 		$res{'from_id'} = $3;
 		$res{'to_id'} = $4;
-		$res{'status'} = $5;
+		$res{'status'} = $res{'status_str'} = $5;
 		$res{'similarity'} = $6;
 		if ($res{'status'} eq 'R' || $res{'status'} eq 'C') { # renamed or copied
 			($res{'from_file'}, $res{'to_file'}) = map { unquote($_) } split("\t", $7);
@@ -2006,6 +2006,7 @@ sub parse_difftree_raw_line {
 		$res{'to_mode'} = pop @{$res{'from_mode'}};
 		$res{'from_id'} = [ split(' ', $3) ];
 		$res{'to_id'} = pop @{$res{'from_id'}};
+		$res{'status_str'} = $4;
 		$res{'status'} = [ split('', $4) ];
 		$res{'to_file'} = unquote($5);
 	}
@@ -2821,7 +2822,7 @@ sub fill_from_file_info {
 sub is_deleted {
 	my $diffinfo = shift;
 
-	return $diffinfo->{'to_id'} eq ('0' x 40);
+	return $diffinfo->{'status_str'} =~ /D/;
 }
 
 # does patch correspond to [previous] difftree raw line
-- 
1.5.3.4

Re: [PATCH 1/2] gitweb: Always set 'from_file' and 'to_file' in parse_difftree_raw_line

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Always set 'from_file' and 'to_file' keys when parsing raw diff output
> format line, even if filename didn't change (file was not renamed).
> This allows for simpler code (and no problems with file named '0').
>
> Use
>   $diffinfo->{'from_file'}
> instead of
>   $diffinfo->{'from_file'} || $diffinfo->{'file'}
> from now on.

Isn't this description the other way around?

Re: [PATCH 1/2] gitweb: Always set 'from_file' and 'to_file' in parse_difftree_raw_line

[Jakub Narebski]

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Always set 'from_file' and 'to_file' keys when parsing raw diff output
>> format line, even if filename didn't change (file was not renamed).
>> This allows for simpler code (and no problems with file named '0').
>>
>> Use
>>   $diffinfo->{'from_file'}
>> instead of
>>   $diffinfo->{'from_file'} || $diffinfo->{'file'}
>> from now on.
> 
> Isn't this description the other way around?

Description is in good direction, although I agree that it is
a bit awkward. Perhaps it should read:

Instead of
  $diffinfo->{'from_file'} || $diffinfo->{'file'}
it is now enough to use
  $diffinfo->{'from_file'}

(because $diffinfo->{'from_file'} is now _always_ set, not only for
renames and copies, when from_name and to_name differ).

-- 
Jakub Narebski
Poland