* gitweb pathinfo improvements @ 2008-09-03 9:57 Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: action in path with use_pathinfo Giuseppe Bilotta ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Giuseppe Bilotta @ 2008-09-03 9:57 UTC (permalink / raw) To: git; +Cc: Jakub Narebski, Petr Baudis, Lea Wiemann The following patchset improves on gitweb's support for PATH_INFO by supporting paths in the form project/action/[parent..]hash, both in generating them and in accepting them. The old path info style project/hash is still supported as long as it doesn't conflict with the new style For those that prefer git trees to patch bombs, my git tree is available for gitweb browsing at http://git.oblomov.eu/git and for git cloning at git://git.oblomov.eu/git/git The changes are very local to the PATH_INFO parsing and creation, so I hope they don't conflict with Lea's cache work. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] gitweb: action in path with use_pathinfo 2008-09-03 9:57 gitweb pathinfo improvements Giuseppe Bilotta @ 2008-09-03 9:57 ` Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: use_pathinfo filenames start with / Giuseppe Bilotta 2008-09-03 10:02 ` gitweb pathinfo improvements Giuseppe Bilotta 2008-09-06 11:22 ` Jakub Narebski 2 siblings, 1 reply; 11+ messages in thread From: Giuseppe Bilotta @ 2008-09-03 9:57 UTC (permalink / raw) To: git; +Cc: Jakub Narebski, Petr Baudis, Lea Wiemann, Giuseppe Bilotta When using path info, reduce the number of CGI parameters by embedding the action in the path. The typicial cgiweb path is thus $project/$action/$hash_base:$file_name or $project/$action/$hash This is mostly backwards compatible with the old-style gitweb paths, except when $project/$branch was used to access a branch whose name matches a gitweb action. Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com> --- gitweb/gitweb.perl | 109 ++++++++++++++++++++++++++++++++++------------------ 1 files changed, 72 insertions(+), 37 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 29e2156..496dce4 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -488,6 +488,37 @@ if (defined $searchtext) { $search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext; } +# dispatch +my %actions = ( + "blame" => \&git_blame, + "blobdiff" => \&git_blobdiff, + "blobdiff_plain" => \&git_blobdiff_plain, + "blob" => \&git_blob, + "blob_plain" => \&git_blob_plain, + "commitdiff" => \&git_commitdiff, + "commitdiff_plain" => \&git_commitdiff_plain, + "commit" => \&git_commit, + "forks" => \&git_forks, + "heads" => \&git_heads, + "history" => \&git_history, + "log" => \&git_log, + "rss" => \&git_rss, + "atom" => \&git_atom, + "search" => \&git_search, + "search_help" => \&git_search_help, + "shortlog" => \&git_shortlog, + "summary" => \&git_summary, + "tag" => \&git_tag, + "tags" => \&git_tags, + "tree" => \&git_tree, + "snapshot" => \&git_snapshot, + "object" => \&git_object, + # those below don't need $project + "opml" => \&git_opml, + "project_list" => \&git_project_list, + "project_index" => \&git_project_index, +); + # now read PATH_INFO and use it as alternative to parameters sub evaluate_path_info { return if defined $project; @@ -512,6 +543,16 @@ sub evaluate_path_info { # do not change any parameters if an action is given using the query string return if $action; $path_info =~ s,^\Q$project\E/*,,; + + # next comes the action + $action = $path_info; + $action =~ s,/.*$,,; + if (exists $actions{$action}) { + $path_info =~ s,^\Q$action\E/*,,; + } else { + $action = undef; + } + my ($refname, $pathname) = split(/:/, $path_info, 2); if (defined $pathname) { # we got "project.git/branch:filename" or "project.git/branch:dir/" @@ -525,10 +566,12 @@ sub evaluate_path_info { } $hash_base ||= validate_refname($refname); $file_name ||= validate_pathname($pathname); + $hash ||= git_get_hash_by_path($hash_base, $file_name); } elsif (defined $refname) { # we got "project.git/branch" - $action ||= "shortlog"; - $hash ||= validate_refname($refname); + $action ||= "shortlog"; + $hash ||= validate_refname($refname); + $hash_base ||= validate_refname($refname); } } evaluate_path_info(); @@ -537,37 +580,6 @@ evaluate_path_info(); our $git_dir; $git_dir = "$projectroot/$project" if $project; -# dispatch -my %actions = ( - "blame" => \&git_blame, - "blobdiff" => \&git_blobdiff, - "blobdiff_plain" => \&git_blobdiff_plain, - "blob" => \&git_blob, - "blob_plain" => \&git_blob_plain, - "commitdiff" => \&git_commitdiff, - "commitdiff_plain" => \&git_commitdiff_plain, - "commit" => \&git_commit, - "forks" => \&git_forks, - "heads" => \&git_heads, - "history" => \&git_history, - "log" => \&git_log, - "rss" => \&git_rss, - "atom" => \&git_atom, - "search" => \&git_search, - "search_help" => \&git_search_help, - "shortlog" => \&git_shortlog, - "summary" => \&git_summary, - "tag" => \&git_tag, - "tags" => \&git_tags, - "tree" => \&git_tree, - "snapshot" => \&git_snapshot, - "object" => \&git_object, - # those below don't need $project - "opml" => \&git_opml, - "project_list" => \&git_project_list, - "project_index" => \&git_project_index, -); - if (!defined $action) { if (defined $hash) { $action = git_get_type($hash); @@ -624,8 +636,13 @@ sub href (%) { if ($params{-replay}) { while (my ($name, $symbol) = each %mapping) { if (!exists $params{$name}) { - # to allow for multivalued params we use arrayref form - $params{$name} = [ $cgi->param($symbol) ]; + if ($cgi->param($symbol)) { + # to allow for multivalued params we use arrayref form + $params{$name} = [ $cgi->param($symbol) ]; + } else { + no strict 'refs'; + $params{$name} = $$name if $$name; + } } } } @@ -636,10 +653,28 @@ sub href (%) { $href .= "/".esc_url($params{'project'}) if defined $params{'project'}; delete $params{'project'}; - # Summary just uses the project path URL - if (defined $params{'action'} && $params{'action'} eq 'summary') { + # Summary just uses the project path URL, any other action come next + # in the URL + if (defined $params{'action'}) { + $href .= "/".esc_url($params{'action'}) unless $params{'action'} eq 'summary'; delete $params{'action'}; } + + # next, we put either hash_base:file_name or hash + if (defined $params{'hash_base'}) { + $href .= "/".esc_url($params{'hash_base'}); + if (defined $params{'file_name'}) { + $href .= ":".esc_url($params{'file_name'}); + delete $params{'hash'} if $params{'hash'} eq git_get_hash_by_path($params{'hash_base'},$params{'file_name'}); + delete $params{'file_name'}; + } else { + delete $params{'hash'} if $params{'hash'} eq $params{'hash_base'}; + } + delete $params{'hash_base'}; + } elsif (defined $params{'hash'}) { + $href .= "/".esc_url($params{'hash'}); + delete $params{'hash'}; + } } # now encode the parameters explicitly -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] gitweb: use_pathinfo filenames start with / 2008-09-03 9:57 ` [PATCH] gitweb: action in path with use_pathinfo Giuseppe Bilotta @ 2008-09-03 9:57 ` Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: parse parent..current syntax from pathinfo Giuseppe Bilotta 0 siblings, 1 reply; 11+ messages in thread From: Giuseppe Bilotta @ 2008-09-03 9:57 UTC (permalink / raw) To: git; +Cc: Jakub Narebski, Petr Baudis, Lea Wiemann, Giuseppe Bilotta When using path info, make filenames start with a / (right after the : that separates them from the hash base). This minimal change allows relative navigation to work properly when viewing HTML files. Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com> --- gitweb/gitweb.perl | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 496dce4..9d4952f 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -664,7 +664,7 @@ sub href (%) { if (defined $params{'hash_base'}) { $href .= "/".esc_url($params{'hash_base'}); if (defined $params{'file_name'}) { - $href .= ":".esc_url($params{'file_name'}); + $href .= ":/".esc_url($params{'file_name'}); delete $params{'hash'} if $params{'hash'} eq git_get_hash_by_path($params{'hash_base'},$params{'file_name'}); delete $params{'file_name'}; } else { -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] gitweb: parse parent..current syntax from pathinfo 2008-09-03 9:57 ` [PATCH] gitweb: use_pathinfo filenames start with / Giuseppe Bilotta @ 2008-09-03 9:57 ` Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: use_pathinfo creates parent..current paths Giuseppe Bilotta 0 siblings, 1 reply; 11+ messages in thread From: Giuseppe Bilotta @ 2008-09-03 9:57 UTC (permalink / raw) To: git; +Cc: Jakub Narebski, Petr Baudis, Lea Wiemann, Giuseppe Bilotta This makes it possible to use an URL such as $project/somebranch..otherbranch:/filename to get a diff between different version of a file. Paths like $project/$action/somebranch:/somefile..otherbranch:/otherfile are parsed as well. Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com> --- gitweb/gitweb.perl | 26 ++++++++++++++++++++++++-- 1 files changed, 24 insertions(+), 2 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 9d4952f..3d21624 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -553,7 +553,9 @@ sub evaluate_path_info { $action = undef; } - my ($refname, $pathname) = split(/:/, $path_info, 2); + $path_info =~ /^((.+?)(:(.+))?\.\.)?(.+?)(:(.+))?$/; + my ($parentrefname, $parentpathname, $refname, $pathname) = ( + $2, $4, $5, $7); if (defined $pathname) { # we got "project.git/branch:filename" or "project.git/branch:dir/" # we could use git_get_type(branch:pathname), but it needs $git_dir @@ -562,7 +564,11 @@ sub evaluate_path_info { $action ||= "tree"; $pathname =~ s,/$,,; } else { - $action ||= "blob_plain"; + if ($parentrefname) { + $action ||= "blobdiff_plain"; + } else { + $action ||= "blob_plain"; + } } $hash_base ||= validate_refname($refname); $file_name ||= validate_pathname($pathname); @@ -573,6 +579,22 @@ sub evaluate_path_info { $hash ||= validate_refname($refname); $hash_base ||= validate_refname($refname); } + # the parent part might be missing the pathname, in which case we use the $file_name, if present + if (defined $parentrefname) { + $hash_parent_base ||= validate_refname($parentrefname); + if ($parentpathname) { + $parentpathname =~ s,^/+,,; + $parentpathname =~ s,/$,,; + $file_parent ||= validate_pathname($parentpathname); + } else { + $file_parent ||= $file_name + } + if (defined $file_parent) { + $hash_parent ||= git_get_hash_by_path($hash_parent_base, $file_parent); + } else { + $hash_parent ||= validate_refname($parentrefname); + } + } } evaluate_path_info(); -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] gitweb: use_pathinfo creates parent..current paths 2008-09-03 9:57 ` [PATCH] gitweb: parse parent..current syntax from pathinfo Giuseppe Bilotta @ 2008-09-03 9:57 ` Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: remove PATH_INFO from $my_url and $my_uri Giuseppe Bilotta 0 siblings, 1 reply; 11+ messages in thread From: Giuseppe Bilotta @ 2008-09-03 9:57 UTC (permalink / raw) To: git; +Cc: Jakub Narebski, Petr Baudis, Lea Wiemann, Giuseppe Bilotta If use_pathinfo is enabled, href now creates links that contain paths in the form $project/$action/oldhash:/oldname..newhash:/newname for actions that use hash_parent etc. Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com> --- gitweb/gitweb.perl | 26 +++++++++++++++++++++++--- 1 files changed, 23 insertions(+), 3 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 3d21624..573b416 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -682,9 +682,29 @@ sub href (%) { delete $params{'action'}; } - # next, we put either hash_base:file_name or hash + # next, we put hash_parent_base:/file_parent..hash_base:/file_name, stripping nonexistent or useless pieces + $href .= "/" if ($params{'hash_base'} || $params{'hash_parent_base'} || $params{'hash_parent'} || $params{'hash'}); if (defined $params{'hash_base'}) { - $href .= "/".esc_url($params{'hash_base'}); + if (defined $params{'hash_parent_base'}) { + $href .= esc_url($params{'hash_parent_base'}); + if (defined $params{'file_parent'}) { + $href .= ":/".esc_url($params{'file_parent'}) unless $params{'file_parent'} eq $params{'file_name'}; + delete $params{'hash_parent'} if $params{'hash_parent'} eq git_get_hash_by_path($params{'hash_parent_base'},$params{'file_parent'}); + delete $params{'file_parent'}; + } else { + delete $params{'hash_parent'} if $params{'hash_parent'} eq $params{'hash_parent_base'}; + if ($params{'file_name'}) { + delete $params{'hash_parent'} if $params{'hash_parent'} eq git_get_hash_by_path($params{'hash_parent_base'},$params{'file_name'}); + } + } + $href .= ".."; + delete $params{'hash_parent_base'}; + } elsif (defined $params{'hash_parent'}) { + $href .= esc_url($params{'hash_parent'}). ".."; + delete $params{'hash_parent'}; + } + + $href .= esc_url($params{'hash_base'}); if (defined $params{'file_name'}) { $href .= ":/".esc_url($params{'file_name'}); delete $params{'hash'} if $params{'hash'} eq git_get_hash_by_path($params{'hash_base'},$params{'file_name'}); @@ -694,7 +714,7 @@ sub href (%) { } delete $params{'hash_base'}; } elsif (defined $params{'hash'}) { - $href .= "/".esc_url($params{'hash'}); + $href .= esc_url($params{'hash'}); delete $params{'hash'}; } } -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] gitweb: remove PATH_INFO from $my_url and $my_uri 2008-09-03 9:57 ` [PATCH] gitweb: use_pathinfo creates parent..current paths Giuseppe Bilotta @ 2008-09-03 9:57 ` Giuseppe Bilotta 0 siblings, 0 replies; 11+ messages in thread From: Giuseppe Bilotta @ 2008-09-03 9:57 UTC (permalink / raw) To: git; +Cc: Jakub Narebski, Petr Baudis, Lea Wiemann, Giuseppe Bilotta This patch (combined with appropriate server configuration) allows usage of the gitweb CGI script as DirectoryIndex for the server root even when the pathinfo feature is enabled. Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com> --- gitweb/gitweb.perl | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 573b416..99c891e 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -26,6 +26,10 @@ our $cgi = new CGI; our $version = "++GIT_VERSION++"; our $my_url = $cgi->url(); our $my_uri = $cgi->url(-absolute => 1); +if (my $path_info = $ENV{"PATH_INFO"}) { + $my_url =~ s,$path_info$,,; + $my_uri =~ s,$path_info$,,; +} # core git executable to use # this can just be "git" if your webserver has a sensible PATH -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: gitweb pathinfo improvements 2008-09-03 9:57 gitweb pathinfo improvements Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: action in path with use_pathinfo Giuseppe Bilotta @ 2008-09-03 10:02 ` Giuseppe Bilotta 2008-09-04 19:02 ` Junio C Hamano 2008-09-06 11:22 ` Jakub Narebski 2 siblings, 1 reply; 11+ messages in thread From: Giuseppe Bilotta @ 2008-09-03 10:02 UTC (permalink / raw) To: git Damn, I forgot the patch sequence numbers. On Wednesday 03 September 2008 11:57, Giuseppe Bilotta wrote: > The following patchset improves on gitweb's support for PATH_INFO > by supporting paths in the form project/action/[parent..]hash, > both in generating them and in accepting them. The old path info > style project/hash is still supported as long as it doesn't > conflict with the new style > > For those that prefer git trees to patch bombs, my git tree is > available for gitweb browsing at http://git.oblomov.eu/git and for > git cloning at git://git.oblomov.eu/git/git > > The changes are very local to the PATH_INFO parsing and creation, > so I hope they don't conflict with Lea's cache work. -- Giuseppe "Oblomov" Bilotta ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: gitweb pathinfo improvements 2008-09-03 10:02 ` gitweb pathinfo improvements Giuseppe Bilotta @ 2008-09-04 19:02 ` Junio C Hamano 0 siblings, 0 replies; 11+ messages in thread From: Junio C Hamano @ 2008-09-04 19:02 UTC (permalink / raw) To: Giuseppe Bilotta; +Cc: git Please don't top post ;-). Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes: > Damn, I forgot the patch sequence numbers. And please don't send a message without useful contents. Instead of cursing alone, after that sentence you could have said: Each of them is a reply to the previous one in the sequence so if your MUA threads correctly that is the order of the patches in the sequence, but in case yours doesn't, they should apply in this order: [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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: gitweb pathinfo improvements 2008-09-03 9:57 gitweb pathinfo improvements Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: action in path with use_pathinfo Giuseppe Bilotta 2008-09-03 10:02 ` gitweb pathinfo improvements Giuseppe Bilotta @ 2008-09-06 11:22 ` Jakub Narebski 2008-09-06 11:55 ` Giuseppe Bilotta 2 siblings, 1 reply; 11+ messages in thread From: Jakub Narebski @ 2008-09-06 11:22 UTC (permalink / raw) To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Lea Wiemann Hi, Giuseppe! I'm sorry for the delay in reviewing this series On Wed, 3 September 2008, Giuseppe Bilotta wrote: > > The following patchset improves on gitweb's support for PATH_INFO > by supporting paths in the form project/action/[parent..]hash, > both in generating them and in accepting them. The old path info > style project/hash is still supported as long as it doesn't > conflict with the new style > > For those that prefer git trees to patch bombs, my git tree is > available for gitweb browsing at http://git.oblomov.eu/git and for > git cloning at git://git.oblomov.eu/git/git > > The changes are very local to the PATH_INFO parsing and creation, > so I hope they don't conflict with Lea's cache work. Let me comment first on _intent_ of the patches, and on series as a whole, before examining individual patches in detail. 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 This series of patches deal with moving more parameters from CGI query to pathinfo (beside 2nd and 5th, which are about relative navigation in HTML files ('blob_plain' view) and ability to act as DirectoryIndex, which means that they are about using gitweb as web server). There are two sides of those pathinfo improvement: first is to be able to get more parameters from pathinfo, second is to use pathinfo-URLs in gitweb links if requested/configured. 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. 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. 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. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: gitweb pathinfo improvements 2008-09-06 11:22 ` Jakub Narebski @ 2008-09-06 11:55 ` Giuseppe Bilotta 2008-09-06 12:47 ` Jakub Narebski 0 siblings, 1 reply; 11+ messages in thread From: Giuseppe Bilotta @ 2008-09-06 11:55 UTC (permalink / raw) To: Jakub Narebski; +Cc: git, Petr Baudis, Lea Wiemann, Junio C Hamano 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: gitweb pathinfo improvements 2008-09-06 11:55 ` Giuseppe Bilotta @ 2008-09-06 12:47 ` Jakub Narebski 0 siblings, 0 replies; 11+ messages in thread From: Jakub Narebski @ 2008-09-06 12:47 UTC (permalink / raw) To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Lea Wiemann, Junio C Hamano On Sat, 6 Sep 2008, Giuseppe Bilotta wrote: > On Sat, Sep 6, 2008, 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? 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2008-09-06 12:48 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-09-03 9:57 gitweb pathinfo improvements Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: action in path with use_pathinfo Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: use_pathinfo filenames start with / Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: parse parent..current syntax from pathinfo Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: use_pathinfo creates parent..current paths Giuseppe Bilotta 2008-09-03 9:57 ` [PATCH] gitweb: remove PATH_INFO from $my_url and $my_uri Giuseppe Bilotta 2008-09-03 10:02 ` gitweb pathinfo improvements Giuseppe Bilotta 2008-09-04 19:02 ` Junio C Hamano 2008-09-06 11:22 ` Jakub Narebski 2008-09-06 11:55 ` Giuseppe Bilotta 2008-09-06 12:47 ` Jakub Narebski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).