Subject: Re: gitweb pathinfo improvements

From: Jakub Narebski <[email-masked]>
Date: Sat, 6 Sep 2008 14:47:39 +0200

On Sat, 6 Sep 2008, Giuseppe Bilotta wrote:
> On Sat, Sep 6, 2008, Jakub Narebski <[email-masked]> 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?

Yes, if you have new enough git.

First, to have numbered patches (just in case mail threading screws up,
for example if emails come out of order, or if you don't use chain
reply to), you can use -n / --numbered option, or format.numbered=auto
(I think that now auto numbering is the default).

To generate template for cover letter you can use new --cover-letter
option. I don't think that there is configuration option for that.

Additionally when sending series of patches I usually output patches to 
separate directory (mdir.1, mdir.2,...) using -o <directory> option,
but that is just my workflow.

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

I think it would be better to have it correct the first time;
especially if you are resending series anyway.

> > By the way, this is perhs 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.

I think that it would be good idea to first refactor generation of 
log-like views in git (log, shortlog, history, search, feed) to 
consolidate them somehow, so revision limiting would work on _all_ 
log-like views.

This is in my TODO list, but I didn't even began implementing it.

-- 
Jakub Narebski
Poland