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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 26, 2010, 18:52 UTC
Message-ID: <7v39zmnceq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1269296130-2237-2-git-send-email-sylvain@abstraction.fr>

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.

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?

> + 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.

> + some typos fixed
>
> Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>

Show 12 quoted lines

> 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).

Show 9 quoted lines

>  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.

Thanks.