On Thu, 2 Oct 2008, Giuseppe Bilotta wrote:
> This makes it possible to use an URL such as
> $project/somebranch..otherbranch:/filename to get a diff between
> different version of a file. Paths like
> $project/$action/somebranch:/somefile..otherbranch:/otherfile are parsed
> as well.
>
In short, it allows to have link to '*diff' views using path_info URL,
or in general to pass $hash_[parent_]base and $file_parent using
path_info.
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx>
> ---
> gitweb/gitweb.perl | 26 ++++++++++++++++++++++++--
> 1 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 3e5b2b7..89e360f 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -534,7 +534,9 @@ if ($path_info && !defined $action) {
>
> # we can now parse ref and pathnames in PATH_INFO
> if ($path_info) {
> - my ($refname, $pathname) = split(/:/, $path_info, 2);
> + $path_info =~ /^((.+?)(:(.+))?\.\.)?(.+?)(:(.+))?$/;
> + my ($parentrefname, $parentpathname, $refname, $pathname) = (
> + $2, $4, $5, $7);
Style: I would use (but that is perhaps matter of taste)
+ my ($parentrefname, $parentpathname, $refname, $pathname) =
+ ($2, $4, $5, $7);
Also it would be I think simpler to use instead non-catching grouping,
i.e. (?: xxx ) extended pattern (see perlre(1)), and use
($1, $2, $3, $4), or even simpler 'list = (string =~ regexp)' form.
I also think that the situation is more complicated than that, if we
want to be more correct.
The following path_info layouts with '..' make sense:
hpb:fp..hb:f
hpb..hb:f == hpb:f..hb:f
hp..h
And the layout below can be though to make sense, but it is just
plain weird.
hpb:fp..f == hpb:fp..HEAD:f
> if (defined $pathname) {
> # we got "project.git/action/branch:filename" or "project.git/action/branch:dir/"
> # we could use git_get_type(branch:pathname), but it needs $git_dir
> @@ -543,7 +545,11 @@ if ($path_info) {
> $input_params{'action'} ||= "tree";
> $pathname =~ s,/$,,;
> } else {
> - $input_params{'action'} ||= "blob_plain";
> + if ($parentrefname) {
> + $input_params{'action'} ||= "blobdiff_plain";
> + } else {
> + $input_params{'action'} ||= "blob_plain";
> + }
Good catch.
> }
> $input_params{'hash_base'} ||= $refname;
> $input_params{'file_name'} ||= $pathname;
> @@ -553,6 +559,22 @@ if ($path_info) {
> $input_params{'hash'} ||= $refname;
> $input_params{'hash_base'} ||= $refname;
> }
> + # the parent part might be missing the pathname, in which case we use the $file_name, if present
> + if (defined $parentrefname) {
> + $input_params{'hash_parent_base'} ||= $parentrefname;
> + if ($parentpathname) {
> + $parentpathname =~ s,^/+,,;
> + $parentpathname =~ s,/$,,;
> + $input_params{'file_parent'} ||= $parentpathname;
> + } else {
> + $input_params{'file_parent'} ||= $input_params{'file_name'};
> + }
> + if (defined $input_params{'file_parent'}) {
> + $input_params{'hash_parent'} ||= git_get_hash_by_path($input_params{'hash_parent_base'}, $input_params{'file_parent'});
This line is bit long, and I think it should be wrapped..
> + } else {
> + $input_params{'hash_parent'} ||= $parentrefname;
> + }
> + }
> }
--
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
