Re: gitweb pathinfo improvements

["Giuseppe Bilotta" <giuseppe.bilotta@gmail.com>]

On Sat, Sep 6, 2008 at 1:22 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> Below there is table of contents / shortlog of this series, which I
> think is a good practice to include in cover letter describing the
> series:
>
> Table of contents:
> ==================
>  * [PATCH 1/5] gitweb: action in path with use_pathinfo
>  * [PATCH 2/5] gitweb: use_pathinfo filenames start with /
>  * [PATCH 3/5] gitweb: parse parent..current syntax from pathinfo
>  * [PATCH 4/5] gitweb: use_pathinfo creates parent..current paths
>  * [PATCH 5/5] gitweb: remove PATH_INFO from $my_url and $my_uri

Yes, I really have to learn proper behaviour when sending a patchest.
I'll make treasure of yours and Junio's hints on the matter 8-)

I'll probably have to resend this patch series anyway, since I've
already found a quirk for which an additional patch is ready, and the
double-dot-filename thing you mention below needs fixing as well.

BTW, is there a way to automate this summary generation when using
format-patch or send-email?

> The problem with fitting more parameters in pathinfo is first backwards
> compatibility (this is not strict requirement, but we would rather not
> make existing bookmarked pathinfo URLs invalid), and second with
> ordering those parameters and detecting when one parameter ends and
> next one begins (which is made more complicated by the fact that some
> parameters, like action or hash/hash_base can be skipped).
>
> This trouble with fitting parameters in pathinfo creates some
> limitations and tradeoffs. For example (optionally!) embedding
> the action in the pathinfo, by putting it after project and before
> hash/hash_base (usually refname) and filename makes old-style
> $project/$branch lead to incorrect view. This also means that we have
> to be careful about creating pathinfo links, either by always putting
> an action, or using full ref name (which I think we do anyway to avoid
> tag/head ambiguities); or doing it only in the case of possible
> conflict i.e. branch named like one of gitweb actions.

Yes, this was something that got me thinking for a while. I've tried
as hard as possible to preserve backwards compatibility, and old-style
paths should still work correctly except for projects whose branched
are named exactly like gitweb actions.

> Using ':' or ':/' to separate branch name (hash or hash_base) from
> filename doesn't lead to problems, as pathinfo is split on _first_
> occurrence of ':', and refnames cannot contain ':'. Using '..' to
> separate $hash_parent_base:$file_parent from $hash_base:$filename
> is IMVHO a very good idea... but when creating pathinfo links we have
> to consider filenames with '..' in them; an example is there in git
> repository: "t/t5515/refs.master_.._.git" file. Then we probably want
> to fallback on CGI query/CGI parameters. NOTE: I have not read the
> patch yet, perhaps it does this.

Actually, this was not a case I had taken into consideration (a
filename with two dots). It should be straightforward to change the
link-creation code to switch to CGI parameters in this case. Should I
change the corresponding patch, or would it be enough to add a patch
to the series clearing this issue?

> By the way, this is perhaps slightly outside the issue of this series,
> but having a..b syntax would tempt to handcraft gitweb URLs for
> equivalents of "git log a..b", "git log a...b" and "git diff a...b",
> neither of which works yet.

I do have a patch to that effect for the shortlog action:
http://git.oblomov.eu/git/commitdiff/refs/heads/gitweb/shortlog and
you can see it in effect on my gitweb with links such as
http://git.oblomov.eu/git/master..gitweb/pathinfo.


-- 
Giuseppe "Oblomov" Bilotta