On Sun, Mar 25, 2007, Martin Koegler wrote:
> git_treediff supports comparing subdirectories. As the output of
> git-difftree is missing the path to the compared directories,
> the links in the output would be wrong.
>
> The patch adds two new parameters to add the missing path prefix.
Wouldn't it be better to concatenate the two "path prefix" patches
together? They are about the same thing.
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 4c371b2..4195b1a 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2372,7 +2372,7 @@ sub git_difftree_body {
> }
>
> sub git_patchset_body {
> - my ($fd, $difftree, $hash, $hash_parent) = @_;
> + my ($fd, $difftree, $hash, $hash_parent, $file_name, $file_parent) = @_;
>
> my $patch_idx = 0;
> my $patch_line;
I'd rather use $from_prefix, $to_prefix here, or $basedif_name,
$basedir_parent, or $dir_name, $dir_parent (my preference is to
$from_prefix, $to_prefix variables).
> @@ -2380,6 +2380,9 @@ sub git_patchset_body {
> my $diffinfo;
> my (%from, %to);
>
> + $file_name = (!defined $file_name)?"":($file_name."/");
> + $file_parent = (!defined $file_parent)?"":($file_parent."/");
> +
> print "<div class=\"patchset\">\n";
>
> # skip to first patch
Minor nit: I'd rather write
+ $from_prefix = !defined $from_prefix ? '' : $from_prefix.'/';
+ $to_prefix = !defined $to_prefix ? '' : $to_prefix . '/';
+ $to_prefix ||= $from_prefix; # to allow to pass common prefix once
+
or something like that, or just modify $from{'file'} and $to{'file'}
$from{'file'} = (!defined $from_prefix ? '' : $from_prefix.'/') . $from{'file'};
$to{'file'} = (!defined $to_prefix ? '' : $to_prefix . '/') . $to{'file'};
just after setting $from{'file'} and $to{'file'}, although the second
solution would additionally add prefix to the shown patch body itself.
> @@ -2439,14 +2442,14 @@ sub git_patchset_body {
> if ($diffinfo->{'status'} ne "A") { # not new (added) file
> $from{'href'} = href(action=>"blob", hash_base=>$hash_parent,
> hash=>$diffinfo->{'from_id'},
> - file_name=>$from{'file'});
> + file_name=>$file_parent.$from{'file'});
> } else {
> delete $from{'href'};
> }
> if ($diffinfo->{'status'} ne "D") { # not deleted file
> $to{'href'} = href(action=>"blob", hash_base=>$hash,
> hash=>$diffinfo->{'to_id'},
> - file_name=>$to{'file'});
> + file_name=>$file_name.$to{'file'});
> } else {
> delete $to{'href'};
> }
Another solution would be to not add additional parameters to
git_difftree_body and git_patchset_body subroutines (although it is nice
touch towards completeness), but modify %diffinfo in the caller, but this
would change also patch contents (in from-file / to-file diff header, etc.)
which might not be a good thing.
I'm not sure if we should not add information somewhere that paths are
prefixed/shortened, but this might be left for later patch.
--
Jakub Narebski
Poland
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html