On Sat, Sep 6, 2008 at 1:22 PM, Jakub Narebski <jnareb@xxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
