threads / patch / 8764

patchgitweb: make search form generate pathinfo-style URLs

Subject: [PATCH] gitweb: make search form generate pathinfo-style URLs

## tl;dr

5 messages between Jun 28, 2007 and Jul 9, 2007. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Matt McCutchen· Jun 28, 2007, 18:57 UTC · lore

The search form generated traditional-style URLs with a "p=" parameter even when the pathinfo feature was on. This patch makes it generate pathinfo-style URLs when appropriate.

Signed-off-by: Matt McCutchen <hashproduct@gmail.com>
---
 gitweb/gitweb.perl |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
Show changes to gitweb/gitweb.perl +9 −3
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f36428e..7b0e110 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2215,12 +2215,18 @@ EOF
 		} else {
 			$search_hash = "HEAD";
 		}
+		my $action = $my_uri;
+		my ($use_pathinfo) = gitweb_check_feature('pathinfo');
+		if ($use_pathinfo) {
+			$action .= "/$project";
+		} else {
+			$cgi->param("p", $project);
+		}
 		$cgi->param("a", "search");
 		$cgi->param("h", $search_hash);
-		$cgi->param("p", $project);
-		print $cgi->startform(-method => "get", -action => $my_uri) .
+		print $cgi->startform(-method => "get", -action => $action) .
 		      "<div class=\"search\">\n" .
-		      $cgi->hidden(-name => "p") . "\n" .
+		      (!$use_pathinfo && $cgi->hidden(-name => "p") . "\n") .
 		      $cgi->hidden(-name => "a") . "\n" .
 		      $cgi->hidden(-name => "h") . "\n" .
 		      $cgi->popup_menu(-name => 'st', -default => 'commit',

-- 
1.5.2.2.552.gc32f
Matt McCutchen· Jul 2, 2007, 19:03 UTC · re: Matt McCutchen · lore

Re: [PATCH] gitweb: make search form generate pathinfo-style URLs

On Thu, 2007-06-28 at 14:57 -0400, Matt McCutchen wrote:
> The search form generated traditional-style URLs with a "p=" parameter
> even when the pathinfo feature was on.  This patch makes it generate
> pathinfo-style URLs when appropriate.

I sent this patch four days ago but no one has commented or done anything about it. Please clue me in: is this typical, or did I do something wrong or do I need to do something else?

Thanks, Matt

Junio C Hamano· Jul 2, 2007, 23:11 UTC · re: Matt McCutchen · lore

Re: [PATCH] gitweb: make search form generate pathinfo-style URLs

Matt McCutchen <hashproduct@gmail.com> writes:
> I sent this patch four days ago but no one has commented or done
> anything about it.  Please clue me in: is this typical, or did I do
> something wrong or do I need to do something else?

Yes, it sometimes happens to patches to some areas. No, you did not do anything wrong as far as I can see. Yes, sending this kind of reminder out is exactly what is needed.

I have three recent patches from you on gitweb in my patch queue.

  Subject: [PATCH] gitweb: snapshot cleanups & support for offering multiple
          formats
  Date: Thu, 28 Jun 2007 14:02:13 -0400
  Message-ID: <1183053733.6108.0.camel@mattlaptop2>
  Subject: [PATCH] gitweb: make search form generate pathinfo-style URLs
  Date: Thu, 28 Jun 2007 14:57:07 -0400
  Message-ID: <1183057027.6108.4.camel@mattlaptop2>
  Subject: [PATCH] gitweb: make "No commits" in project list gray, not bold
          green
  Date: Thu, 28 Jun 2007 18:15:22 -0400
  Message-ID: <1183068922.6108.8.camel@mattlaptop2>

There are people whom I consider "gitweb folks", who are more familiar with, and more importantly who are more interested in, gitweb than I am. Having good "subsystem people" makes gitweb a lower-priority area to myself, and for these patches it made even so that nobody commented on them.

I should have asked Ack/Nack/Comments from gitweb folks much earlier, but was a bit too loaded with the day job. Sorry about that.

So, gitweb folks, what do you think about these patches?
Junio C Hamano· Jul 7, 2007, 21:01 UTC · re: Matt McCutchen · lore

Re: [PATCH] gitweb: make search form generate pathinfo-style URLs

Matt McCutchen <hashproduct@gmail.com> writes:
Show 32 quoted lines
> The search form generated traditional-style URLs with a "p=" parameter
> even when the pathinfo feature was on.  This patch makes it generate
> pathinfo-style URLs when appropriate.
>
> Signed-off-by: Matt McCutchen <hashproduct@gmail.com>
> ---
>  gitweb/gitweb.perl |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index f36428e..7b0e110 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2215,12 +2215,18 @@ EOF
>  		} else {
>  			$search_hash = "HEAD";
>  		}
> +		my $action = $my_uri;
> +		my ($use_pathinfo) = gitweb_check_feature('pathinfo');
> +		if ($use_pathinfo) {
> +			$action .= "/$project";
> +		} else {
> +			$cgi->param("p", $project);
> +		}
>  		$cgi->param("a", "search");
>  		$cgi->param("h", $search_hash);
> -		$cgi->param("p", $project);
> -		print $cgi->startform(-method => "get", -action => $my_uri) .
> +		print $cgi->startform(-method => "get", -action => $action) .
>  		      "<div class=\"search\">\n" .
> -		      $cgi->hidden(-name => "p") . "\n" .
> +		      (!$use_pathinfo && $cgi->hidden(-name => "p") . "\n") .

Ahhhhh. (!$use_pathinfo && blah) when $use_pathinfo is true evaluates to an empty string, which saves you from protecting yourself from undef. Perhaps clever, but is not terribly nice.

Other than that, the patch looks sane to me.
Luben Tuikov· Jul 9, 2007, 19:03 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: make search form generate pathinfo-style URLs

--- Junio C Hamano <gitster@pobox.com> wrote:
Show 40 quoted lines
> Matt McCutchen <hashproduct@gmail.com> writes:
> 
> > The search form generated traditional-style URLs with a "p=" parameter
> > even when the pathinfo feature was on.  This patch makes it generate
> > pathinfo-style URLs when appropriate.
> >
> > Signed-off-by: Matt McCutchen <hashproduct@gmail.com>
> > ---
> >  gitweb/gitweb.perl |   12 +++++++++---
> >  1 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index f36428e..7b0e110 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -2215,12 +2215,18 @@ EOF
> >  		} else {
> >  			$search_hash = "HEAD";
> >  		}
> > +		my $action = $my_uri;
> > +		my ($use_pathinfo) = gitweb_check_feature('pathinfo');
> > +		if ($use_pathinfo) {
> > +			$action .= "/$project";
> > +		} else {
> > +			$cgi->param("p", $project);
> > +		}
> >  		$cgi->param("a", "search");
> >  		$cgi->param("h", $search_hash);
> > -		$cgi->param("p", $project);
> > -		print $cgi->startform(-method => "get", -action => $my_uri) .
> > +		print $cgi->startform(-method => "get", -action => $action) .
> >  		      "<div class=\"search\">\n" .
> > -		      $cgi->hidden(-name => "p") . "\n" .
> > +		      (!$use_pathinfo && $cgi->hidden(-name => "p") . "\n") .
> 
> Ahhhhh.  (!$use_pathinfo && blah) when $use_pathinfo is true
> evaluates to an empty string, which saves you from protecting
> yourself from undef.  Perhaps clever, but is not terribly nice.
> 
> Other than that, the patch looks sane to me.
Good catch.
    Luben

← back to recent threads