[PATCH] gitweb: action in path with use_pathinfo

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] gitweb: action in path with use_pathinfo

Giuseppe Bilotta-3
When using path info, reduce the number of CGI parameters by embedding
the action in the path. The typicial cgiweb path is thus
$project/$action/$hash_base:$file_name or $project/$action/$hash

This is mostly backwards compatible with the old-style gitweb paths,
except when $project/$branch was used to access a branch whose name
matches a gitweb action.

Signed-off-by: Giuseppe Bilotta <[hidden email]>
---

I've been thinking a long time about the best way to get rid of most
CGI parameters when using path info, and I think that the attached
patch solves the problem pretty well, since it makes most simple
(one-paramater) actions accessible as plain paths.

The only weak point I see in my approach is with -replay=>1. Since in
our case there are no CGI parameters to be replayed, I use the
corresponding variables. An alternative, safer method would probably
be to store the various request parameters in a separate hash,
obtained by merging the CGI data with the one extracted from the
request path. I didn't follow the latter route because the simpler
approach implemented in this patch seems to work without problems.

 gitweb/gitweb.perl |  111 ++++++++++++++++++++++++++++++++++------------------
 1 files changed, 73 insertions(+), 38 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index aa8c27c..d4ed401 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -512,6 +512,37 @@ if (defined $searchtext) {
  $search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
 }
 
+# dispatch
+my %actions = (
+ "blame" => \&git_blame,
+ "blobdiff" => \&git_blobdiff,
+ "blobdiff_plain" => \&git_blobdiff_plain,
+ "blob" => \&git_blob,
+ "blob_plain" => \&git_blob_plain,
+ "commitdiff" => \&git_commitdiff,
+ "commitdiff_plain" => \&git_commitdiff_plain,
+ "commit" => \&git_commit,
+ "forks" => \&git_forks,
+ "heads" => \&git_heads,
+ "history" => \&git_history,
+ "log" => \&git_log,
+ "rss" => \&git_rss,
+ "atom" => \&git_atom,
+ "search" => \&git_search,
+ "search_help" => \&git_search_help,
+ "shortlog" => \&git_shortlog,
+ "summary" => \&git_summary,
+ "tag" => \&git_tag,
+ "tags" => \&git_tags,
+ "tree" => \&git_tree,
+ "snapshot" => \&git_snapshot,
+ "object" => \&git_object,
+ # those below don't need $project
+ "opml" => \&git_opml,
+ "project_list" => \&git_project_list,
+ "project_index" => \&git_project_index,
+);
+
 # now read PATH_INFO and use it as alternative to parameters
 sub evaluate_path_info {
  return if defined $project;
@@ -536,6 +568,16 @@ sub evaluate_path_info {
  # do not change any parameters if an action is given using the query string
  return if $action;
  $path_info =~ s,^\Q$project\E/*,,;
+
+ # next comes the action
+ $action = $path_info;
+ $action =~ s,/.*$,,;
+ if (exists $actions{$action}) {
+ $path_info =~ s,^\Q$action\E/*,,;
+ } else {
+ $action  = undef;
+ }
+
  my ($refname, $pathname) = split(/:/, $path_info, 2);
  if (defined $pathname) {
  # we got "project.git/branch:filename" or "project.git/branch:dir/"
@@ -549,10 +591,12 @@ sub evaluate_path_info {
  }
  $hash_base ||= validate_refname($refname);
  $file_name ||= validate_pathname($pathname);
+ $hash      ||= git_get_hash_by_path($hash_base, $file_name);
  } elsif (defined $refname) {
  # we got "project.git/branch"
- $action ||= "shortlog";
- $hash   ||= validate_refname($refname);
+ $action    ||= "shortlog";
+ $hash      ||= validate_refname($refname);
+ $hash_base ||= validate_refname($refname);
  }
 }
 evaluate_path_info();
@@ -561,37 +605,6 @@ evaluate_path_info();
 our $git_dir;
 $git_dir = "$projectroot/$project" if $project;
 
-# dispatch
-my %actions = (
- "blame" => \&git_blame,
- "blobdiff" => \&git_blobdiff,
- "blobdiff_plain" => \&git_blobdiff_plain,
- "blob" => \&git_blob,
- "blob_plain" => \&git_blob_plain,
- "commitdiff" => \&git_commitdiff,
- "commitdiff_plain" => \&git_commitdiff_plain,
- "commit" => \&git_commit,
- "forks" => \&git_forks,
- "heads" => \&git_heads,
- "history" => \&git_history,
- "log" => \&git_log,
- "rss" => \&git_rss,
- "atom" => \&git_atom,
- "search" => \&git_search,
- "search_help" => \&git_search_help,
- "shortlog" => \&git_shortlog,
- "summary" => \&git_summary,
- "tag" => \&git_tag,
- "tags" => \&git_tags,
- "tree" => \&git_tree,
- "snapshot" => \&git_snapshot,
- "object" => \&git_object,
- # those below don't need $project
- "opml" => \&git_opml,
- "project_list" => \&git_project_list,
- "project_index" => \&git_project_index,
-);
-
 if (!defined $action) {
  if (defined $hash) {
  $action = git_get_type($hash);
@@ -649,8 +661,13 @@ sub href (%) {
  if ($params{-replay}) {
  while (my ($name, $symbol) = each %mapping) {
  if (!exists $params{$name}) {
- # to allow for multivalued params we use arrayref form
- $params{$name} = [ $cgi->param($symbol) ];
+ if ($cgi->param($symbol)) {
+ # to allow for multivalued params we use arrayref form
+ $params{$name} = [ $cgi->param($symbol) ];
+ } else {
+ no strict 'refs';
+ $params{$name} = $$name if $$name;
+ }
  }
  }
  }
@@ -661,10 +678,28 @@ sub href (%) {
  $href .= "/".esc_url($params{'project'}) if defined $params{'project'};
  delete $params{'project'};
 
- # Summary just uses the project path URL
- if (defined $params{'action'} && $params{'action'} eq 'summary') {
+ # Summary just uses the project path URL, any other action come next
+ # in the URL
+ if (defined $params{'action'}) {
+ $href .= "/".esc_url($params{'action'}) unless $params{'action'} eq 'summary';
  delete $params{'action'};
  }
+
+ # next, we put either hash_base:file_name or hash
+ if (defined $params{'hash_base'}) {
+ $href .= "/".esc_url($params{'hash_base'});
+ if (defined $params{'file_name'}) {
+ $href .= ":".esc_url($params{'file_name'});
+ delete $params{'hash'} if $params{'hash'} eq git_get_hash_by_path($params{'hash_base'},$params{'file_name'});
+ delete $params{'file_name'};
+ } else {
+ delete $params{'hash'} if $params{'hash'} eq $params{'hash_base'};
+ }
+ delete $params{'hash_base'};
+ } elsif (defined $params{'hash'}) {
+ $href .= "/".esc_url($params{'hash'});
+ delete $params{'hash'};
+ }
  }
 
  # now encode the parameters explicitly
--
1.5.6.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH] gitweb: use_pathinfo filenames start with /

Giuseppe Bilotta-3
When using path info, make filenames start with a / (right after the :
that separates them from the hash base). This minimal change allows
relative navigation to work properly when viewing HTML files.
---

This patch is based on top of my previous patch
  gitweb: action in path with use_pathinfo
(which I sent with the wrong CC: lines)

 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 56fbdab..a8c0887 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -664,7 +664,7 @@ sub href (%) {
  if (defined $params{'hash_base'}) {
  $href .= "/".esc_url($params{'hash_base'});
  if (defined $params{'file_name'}) {
- $href .= ":".esc_url($params{'file_name'});
+ $href .= ":/".esc_url($params{'file_name'});
  delete $params{'hash'} if $params{'hash'} eq git_get_hash_by_path($params{'hash_base'},$params{'file_name'});
  delete $params{'file_name'};
  } else {
--
1.5.6.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] gitweb: parse parent..current syntax from pathinfo

Giuseppe Bilotta-3
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.
---

This patch and the next apply on top of my previous two
gitweb pathinfo patches.

I realize that these patches of mine coming through in burst
can get pretty annoying, but I honestly wasn't planning of
making so many changes to the path info feature. So let me
know if it's better to resend the patches in a series.

Hopefully, with this patch and the next the series should be
complete, as most commands will now be able to get all of
their parameters from the path without using CGI parameters.

 gitweb/gitweb.perl |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 9e90017..8364b71 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -578,7 +578,9 @@ sub evaluate_path_info {
  $action  = undef;
  }
 
- my ($refname, $pathname) = split(/:/, $path_info, 2);
+ $path_info =~ /^((.+?)(:(.+))?\.\.)?(.+?)(:(.+))?$/;
+ my ($parentrefname, $parentpathname, $refname, $pathname) = (
+ $2, $4, $5, $7);
  if (defined $pathname) {
  # we got "project.git/branch:filename" or "project.git/branch:dir/"
  # we could use git_get_type(branch:pathname), but it needs $git_dir
@@ -587,11 +589,27 @@ sub evaluate_path_info {
  $action  ||= "tree";
  $pathname =~ s,/$,,;
  } else {
- $action  ||= "blob_plain";
+ if ($parentrefname) {
+ $action ||= "blobdiff_plain";
+ } else {
+ $action  ||= "blob_plain";
+ }
  }
  $hash_base ||= validate_refname($refname);
  $file_name ||= validate_pathname($pathname);
  $hash      ||= git_get_hash_by_path($hash_base, $file_name);
+
+ if ($parentrefname) {
+ $hash_parent_base ||= validate_refname($parentrefname);
+ if ($parentpathname) {
+ $parentpathname =~ s,^/+,,;
+ $parentpathname =~ s,/$,,;
+ $file_parent ||= validate_pathname($parentpathname);
+ } else {
+ $file_parent ||= $file_name
+ }
+ $hash_parent ||= git_get_hash_by_path($hash_parent_base, $file_parent);
+ }
  } elsif (defined $refname) {
  # we got "project.git/branch"
  $action    ||= "shortlog";
--
1.5.6.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] gitweb: use_pathinfo creates parent..current paths

Giuseppe Bilotta-3
If use_pathinfo is enabled, href now creates links that contain paths in
the form $project/$action/oldhash:/oldname..newhash:/newname for actions
that use hash_parent etc.
---
 gitweb/gitweb.perl |   26 +++++++++++++++++++++++---
 1 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8364b71..e89e1a1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -703,9 +703,29 @@ sub href (%) {
  delete $params{'action'};
  }
 
- # next, we put either hash_base:file_name or hash
+ # next, we put hash_parent_base:/file_parent..hash_base:/file_name, stripping nonexistent or useless pieces
+ $href .= "/" if ($params{'hash_base'} || $params{'hash_parent_base'} || $params{'hash_parent'} || $params{'hash'});
  if (defined $params{'hash_base'}) {
- $href .= "/".esc_url($params{'hash_base'});
+ if (defined $params{'hash_parent_base'}) {
+ $href .= esc_url($params{'hash_parent_base'});
+ if (defined $params{'file_parent'}) {
+ $href .= ":/".esc_url($params{'file_parent'}) unless $params{'file_parent'} eq $params{'file_name'};
+ delete $params{'hash_parent'} if $params{'hash_parent'} eq git_get_hash_by_path($params{'hash_parent_base'},$params{'file_parent'});
+ delete $params{'file_parent'};
+ } else {
+ delete $params{'hash_parent'} if $params{'hash_parent'} eq $params{'hash_parent_base'};
+ if ($params{'file_name'}) {
+ delete $params{'hash_parent'} if $params{'hash_parent'} eq git_get_hash_by_path($params{'hash_parent_base'},$params{'file_name'});
+ }
+ }
+ $href .= "..";
+ delete $params{'hash_parent_base'};
+ } elsif (defined $params{'hash_parent'}) {
+ $href .= esc_url($params{'hash_parent'}). "..";
+ delete $params{'hash_parent'};
+ }
+
+ $href .= esc_url($params{'hash_base'});
  if (defined $params{'file_name'}) {
  $href .= ":/".esc_url($params{'file_name'});
  delete $params{'hash'} if $params{'hash'} eq git_get_hash_by_path($params{'hash_base'},$params{'file_name'});
@@ -715,7 +735,7 @@ sub href (%) {
  }
  delete $params{'hash_base'};
  } elsif (defined $params{'hash'}) {
- $href .= "/".esc_url($params{'hash'});
+ $href .= esc_url($params{'hash'});
  delete $params{'hash'};
  }
  }
--
1.5.6.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html