git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH try 2] gitweb: Add option to put a trailing slash on pathinfo-style project URLs

From
Jakub Narebski <jnareb@gmail.com>
Date
Dec 14, 2008, 23:58 UTC
Message-ID
<200812150058.36038.jnareb@gmail.com>
In-Reply-To
<200812150020.53370.jnareb@gmail.com>
On Mon, 15 Dec 2008, Jakub Narebski wrote:
> On Sun, 14 Dec 2008, Matt McCutchen wrote:
>> On Sat, 2008-12-13 at 13:47 -0800, Jakub Narebski wrote:
Show 43 quoted lines
>>>> @@ -845,7 +850,12 @@ sub href (%) {
>>>>  		$href =~ s,/$,,;
>>>>  
>>>>  		# Then add the project name, if present
>>>> -		$href .= "/".esc_url($params{'project'}) if defined $params{'project'};
>>>> +		my $proj_href = undef;
>>>> +		if (defined $params{'project'}) {
>>>> +			$href .= "/".esc_url($params{'project'});
>>>> +			# Save for trailing-slash check below.
>>>> +			$proj_href = $href;
>>>> +		}
>>>>  		delete $params{'project'};
>>>>  
>>>>  		# since we destructively absorb parameters, we keep this
>>>> @@ -903,6 +913,10 @@ sub href (%) {
>>>>  			$href .= $known_snapshot_formats{$fmt}{'suffix'};
>>>>  			delete $params{'snapshot_format'};
>>>>  		}
>>>> +
>>>> +		# If requested in the configuration, add a trailing slash to a URL that
>>>> +		# has nothing appended after the project path.
>>>> +		$href .= '/' if ($use_pathinfo[1] && defined $proj_href && $href eq $proj_href);
>>>>  	}
>>> 
>>> The check _feels_ inefficient.  I think (but feel free to disagree) that
>>> it would be better to use something like $project_pathinfo, set it
>>> when adding project as pathinfo, and unset if we add anything else as
>>> pathinfo.
>> 
>> I considered doing that, but I decided that not having to litter the
>> preceding code with manipulation of $project_pathinfo outweighed
>> whatever negligible performance difference there might be.
> 
> On the other hand, with having boolean variable named for example
> $trailing_slash or $add_trailing_slash, you can set it to appropriate
> value by default (should project list URL: http://example.com/ have
> trailing slash), and at [almost] each 'delete $params{<param>}' either
> set it to true, or set it to false. This way it would be easy to
> extend to have trailing slash also for example for OPML link
> http://example.com/opml/ or not have it and use http://example.com/opml
> 
> I think it is not only more efficient, but is also more flexible.
> Admittedly it is also more complicated...

On the other hand you don't need such flexibility, so perhaps simpler code (or at least less changes) outweights this issue...

-- 
Jakub Narebski
Poland
Previous: Jakub Narebski
Message 11 of 11 in “gitweb: Add option to put a trailing slash on pathinfo-style project URLs”
  1. gitweb: Add option to put a trailing slash on pathinfo-style project URLsMatt McCutchen, Dec 13, 2008
  2. gitweb: Add option to put a trailing slash on pathinfo-style project URLsMatt McCutchen, Dec 13, 2008
  3. Jakub NarebskiDec 13, 2008
  4. Giuseppe BilottaDec 13, 2008
  5. Matt McCutchenDec 14, 2008
  6. Jakub NarebskiDec 14, 2008
  7. Jakub NarebskiDec 14, 2008
  8. Junio C HamanoDec 13, 2008
  9. Matt McCutchenDec 14, 2008
  10. Jakub NarebskiDec 14, 2008
  11. Jakub NarebskiDec 14, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.