On Fri, 2010-03-26 at 11:52 -0700, Junio C Hamano wrote: > I was waiting for gitweb people to respond, but nobody seems to be > interested so here is my take on it. > > Sylvain Rabot <sylvain@xxxxxxxxxxxxxx> writes: > > > + the RewriteRule '/+<user>' is not working as the '+' character is > > replaced by a space in urls when you click on links. it is replaced by '/u/<user>' > > I think the _only_ value of having this example, in addition to the next > one that uses "http://host/user/<me>" notation, was to demonstrate that > you do not necessarily have the actual user name and the magic token (be > it "user" or "u") that introduces the per-user hierarchy as separate path > components delimited with a slash. Changing "+<me>" to "u/<me>" removes > that only additional value from this example. > > Anybody moderately intelligent would be able to guess "u/<me>" if she > finds "user/<me>" too long to her taste, so I would suggest updating the > example to allow "http://host/+<user>/" but spell the rewrite rule in such > a way that actually does work. An alternative is to just remove it. > The problem is http://host/+user works but then, when you click on a link you will be redirected to : "http://host/ user?p=git/git.git;a=tree" -------------^ I will try to look into gitweb.perl to see if the url encoding can be updated smoothly without breaking anything to accept the '+' otherwise I think removing this example would be the right decision like you suggested. > By the way, does mod-rewrite configuration allow "~<me>" (home-directory > expansion) when setting the environment? You currently do: > > E=GITWEB_PROJECTROOT:/home/$1/public_git/ > > but if we somehow could write it like > > E=GITWEB_PROJECTROOT:~$1/public_git/ > > it would be more generally useful, no? I looked and I don't think so, ~user/public_git/ is not evaluated by apache. Maybe it possible to evaluate it in the perl side, I will look into it also. > > > + the RewriteRule '/user/<user>' updated to allow > > '/user/<user>', '/user/<user>/' and '/user/<user>/gitweb.cgi' > > Please describe what you added relative to the original, not just what the > final result looks like. "updated to allow A B C" doesn't tell the reader > "it used to redirect only A and C to gitweb request, but B wasn't > rewritten.", which seems to be the case if I am reading your regexp > correctly. Describing why it is better to also rewrite B would be a good > idea, too, if it is not obvious. Will do. > > > + some typos fixed > > > > Signed-off-by: Sylvain Rabot <sylvain@xxxxxxxxxxxxxx> > > > diff --git a/gitweb/README b/gitweb/README > > index ad6a04c..bc90f4d 100644 > > --- a/gitweb/README > > +++ b/gitweb/README > > @@ -347,18 +347,18 @@ something like the following in your gitweb.conf (or gitweb_config.perl) file: > > $home_link = "/"; > > > > > > -Webserver configuration with multiple projects' root > > ----------------------------------------------------- > > +Webserver configuration with multiple project roots > > +--------------------------------------------------- > > Ok. > > > -If you want to use gitweb with several project roots you can edit your apache > > -virtual host and gitweb.conf configuration files like this : > > +If you want to use gitweb with several project roots then you can edit your > > +apache virtual host and gitweb.conf configuration files like this : > > Ok (you might want to remove SP before colon, though). My bad, French habit, but, according to wikipedia it is also English "compliant" (http://en.wikipedia.org/wiki/Colon_%28punctuation% 29#Spacing). As you want. > > > virtual host configuration : > > > > <VirtualHost *:80> > > - ServerName git.example.org > > - DocumentRoot /pub/git > > - SetEnv GITWEB_CONFIG /etc/gitweb.conf > > + ServerName git.example.org > > + DocumentRoot /pub/git > > + SetEnv GITWEB_CONFIG /etc/gitweb.conf > > What is this reindentation for? "Just cosmetic" is an acceptable answer > as long as the change resulted in cosmetic improvement, but it doesn't > seem to be cosmetic improvement, either. That was the case, it looked better in vim. > > Thanks. -- Sylvain Rabot <sylvain@xxxxxxxxxxxxxx>
Attachment:
signature.asc
Description: This is a digitally signed message part