threads / patch / 10555

patch, 2 partsgitweb: Always set 'from_file' and 'to_file' in parse_difftree_raw_line

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

## tl;dr

5 messages between Nov 1, 2007 and Nov 1, 2007. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Jakub Narebski· Nov 1, 2007, 11:38 UTC · lore

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

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

Jakub Narebski· Nov 1, 2007, 11:38 UTC · re: Jakub Narebski · lore

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(-)

Show changes to gitweb/gitweb.perl +10 −7

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

Junio C Hamano· Nov 1, 2007, 22:47 UTC · re: Jakub Narebski · lore

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

Jakub Narebski <jnareb@gmail.com> writes:

Show 9 quoted lines

> 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?

Jakub Narebski· Nov 1, 2007, 23:46 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:

Show 13 quoted lines

> 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

Jakub Narebski· Nov 1, 2007, 11:38 UTC · re: Jakub Narebski · lore

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

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(-)

Show changes to gitweb/gitweb.perl +3 −2

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