Re: [PATCH] gitweb: readme fixed regarding per user project root repository

[Sylvain Rabot <sylvain@abstraction.fr>]

[-- Attachment #1: Type: text/plain, Size: 4354 bytes --]

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@abstraction.fr> 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@abstraction.fr>
> 
> > 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@abstraction.fr>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]