{"thread":{"id":"15353","subject":"[PATCH] gitweb: action in path with use_pathinfo","startedAt":"2008-09-03T09:57:14Z","lastAt":"2008-09-06T12:47:39Z","messageCount":11,"participants":["Giuseppe Bilotta","Junio C Hamano","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"89611","messageId":"1220435839-29360-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"15353","inReplyTo":null,"subject":"gitweb pathinfo improvements","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-09-03T09:57:14Z","receivedAt":"2008-09-03T09:57:14Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"\nThe following patchset improves on gitweb's support for PATH_INFO\nby supporting paths in the form project/action/[parent..]hash,\nboth in generating them and in accepting them. The old path info\nstyle project/hash is still supported as long as it doesn't\nconflict with the new style\n\nFor those that prefer git trees to patch bombs, my git tree is\navailable for gitweb browsing at http://git.oblomov.eu/git and for\ngit cloning at git://git.oblomov.eu/git/git\n\nThe changes are very local to the PATH_INFO parsing and creation,\nso I hope they don't conflict with Lea's cache work.\n"},{"id":"89607","messageId":"1220435839-29360-2-git-send-email-giuseppe.bilotta@gmail.com","threadId":"15353","inReplyTo":"1220435839-29360-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH] gitweb: action in path with use_pathinfo","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-09-03T09:57:15Z","receivedAt":"2008-09-03T09:57:15Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"When using path info, reduce the number of CGI parameters by embedding\nthe action in the path. The typicial cgiweb path is thus\n$project/$action/$hash_base:$file_name or $project/$action/$hash\n\nThis is mostly backwards compatible with the old-style gitweb paths,\nexcept when $project/$branch was used to access a branch whose name\nmatches a gitweb action.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n gitweb/gitweb.perl |  109 ++++++++++++++++++++++++++++++++++------------------\n 1 files changed, 72 insertions(+), 37 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 29e2156..496dce4 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -488,6 +488,37 @@ if (defined $searchtext) {\n \t$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;\n }\n \n+# dispatch\n+my %actions = (\n+\t\"blame\" => \\&git_blame,\n+\t\"blobdiff\" => \\&git_blobdiff,\n+\t\"blobdiff_plain\" => \\&git_blobdiff_plain,\n+\t\"blob\" => \\&git_blob,\n+\t\"blob_plain\" => \\&git_blob_plain,\n+\t\"commitdiff\" => \\&git_commitdiff,\n+\t\"commitdiff_plain\" => \\&git_commitdiff_plain,\n+\t\"commit\" => \\&git_commit,\n+\t\"forks\" => \\&git_forks,\n+\t\"heads\" => \\&git_heads,\n+\t\"history\" => \\&git_history,\n+\t\"log\" => \\&git_log,\n+\t\"rss\" => \\&git_rss,\n+\t\"atom\" => \\&git_atom,\n+\t\"search\" => \\&git_search,\n+\t\"search_help\" => \\&git_search_help,\n+\t\"shortlog\" => \\&git_shortlog,\n+\t\"summary\" => \\&git_summary,\n+\t\"tag\" => \\&git_tag,\n+\t\"tags\" => \\&git_tags,\n+\t\"tree\" => \\&git_tree,\n+\t\"snapshot\" => \\&git_snapshot,\n+\t\"object\" => \\&git_object,\n+\t# those below don't need $project\n+\t\"opml\" => \\&git_opml,\n+\t\"project_list\" => \\&git_project_list,\n+\t\"project_index\" => \\&git_project_index,\n+);\n+\n # now read PATH_INFO and use it as alternative to parameters\n sub evaluate_path_info {\n \treturn if defined $project;\n@@ -512,6 +543,16 @@ sub evaluate_path_info {\n \t# do not change any parameters if an action is given using the query string\n \treturn if $action;\n \t$path_info =~ s,^\\Q$project\\E/*,,;\n+\n+\t# next comes the action\n+\t$action = $path_info;\n+\t$action =~ s,/.*$,,;\n+\tif (exists $actions{$action}) {\n+\t\t$path_info =~ s,^\\Q$action\\E/*,,;\n+\t} else {\n+\t\t$action  = undef;\n+\t}\n+\n \tmy ($refname, $pathname) = split(/:/, $path_info, 2);\n \tif (defined $pathname) {\n \t\t# we got \"project.git/branch:filename\" or \"project.git/branch:dir/\"\n@@ -525,10 +566,12 @@ sub evaluate_path_info {\n \t\t}\n \t\t$hash_base ||= validate_refname($refname);\n \t\t$file_name ||= validate_pathname($pathname);\n+\t\t$hash      ||= git_get_hash_by_path($hash_base, $file_name);\n \t} elsif (defined $refname) {\n \t\t# we got \"project.git/branch\"\n-\t\t$action ||= \"shortlog\";\n-\t\t$hash   ||= validate_refname($refname);\n+\t\t$action    ||= \"shortlog\";\n+\t\t$hash      ||= validate_refname($refname);\n+\t\t$hash_base ||= validate_refname($refname);\n \t}\n }\n evaluate_path_info();\n@@ -537,37 +580,6 @@ evaluate_path_info();\n our $git_dir;\n $git_dir = \"$projectroot/$project\" if $project;\n \n-# dispatch\n-my %actions = (\n-\t\"blame\" => \\&git_blame,\n-\t\"blobdiff\" => \\&git_blobdiff,\n-\t\"blobdiff_plain\" => \\&git_blobdiff_plain,\n-\t\"blob\" => \\&git_blob,\n-\t\"blob_plain\" => \\&git_blob_plain,\n-\t\"commitdiff\" => \\&git_commitdiff,\n-\t\"commitdiff_plain\" => \\&git_commitdiff_plain,\n-\t\"commit\" => \\&git_commit,\n-\t\"forks\" => \\&git_forks,\n-\t\"heads\" => \\&git_heads,\n-\t\"history\" => \\&git_history,\n-\t\"log\" => \\&git_log,\n-\t\"rss\" => \\&git_rss,\n-\t\"atom\" => \\&git_atom,\n-\t\"search\" => \\&git_search,\n-\t\"search_help\" => \\&git_search_help,\n-\t\"shortlog\" => \\&git_shortlog,\n-\t\"summary\" => \\&git_summary,\n-\t\"tag\" => \\&git_tag,\n-\t\"tags\" => \\&git_tags,\n-\t\"tree\" => \\&git_tree,\n-\t\"snapshot\" => \\&git_snapshot,\n-\t\"object\" => \\&git_object,\n-\t# those below don't need $project\n-\t\"opml\" => \\&git_opml,\n-\t\"project_list\" => \\&git_project_list,\n-\t\"project_index\" => \\&git_project_index,\n-);\n-\n if (!defined $action) {\n \tif (defined $hash) {\n \t\t$action = git_get_type($hash);\n@@ -624,8 +636,13 @@ sub href (%) {\n \tif ($params{-replay}) {\n \t\twhile (my ($name, $symbol) = each %mapping) {\n \t\t\tif (!exists $params{$name}) {\n-\t\t\t\t# to allow for multivalued params we use arrayref form\n-\t\t\t\t$params{$name} = [ $cgi->param($symbol) ];\n+\t\t\t\tif ($cgi->param($symbol)) {\n+\t\t\t\t\t# to allow for multivalued params we use arrayref form\n+\t\t\t\t\t$params{$name} = [ $cgi->param($symbol) ];\n+\t\t\t\t} else {\n+\t\t\t\t\tno strict 'refs';\n+\t\t\t\t\t$params{$name} = $$name if $$name;\n+\t\t\t\t}\n \t\t\t}\n \t\t}\n \t}\n@@ -636,10 +653,28 @@ sub href (%) {\n \t\t$href .= \"/\".esc_url($params{'project'}) if defined $params{'project'};\n \t\tdelete $params{'project'};\n \n-\t\t# Summary just uses the project path URL\n-\t\tif (defined $params{'action'} && $params{'action'} eq 'summary') {\n+\t\t# Summary just uses the project path URL, any other action come next\n+\t\t# in the URL\n+\t\tif (defined $params{'action'}) {\n+\t\t\t$href .= \"/\".esc_url($params{'action'}) unless $params{'action'} eq 'summary';\n \t\t\tdelete $params{'action'};\n \t\t}\n+\n+\t\t# next, we put either hash_base:file_name or hash\n+\t\tif (defined $params{'hash_base'}) {\n+\t\t\t$href .= \"/\".esc_url($params{'hash_base'});\n+\t\t\tif (defined $params{'file_name'}) {\n+\t\t\t\t$href .= \":\".esc_url($params{'file_name'});\n+\t\t\t\tdelete $params{'hash'} if $params{'hash'} eq git_get_hash_by_path($params{'hash_base'},$params{'file_name'});\n+\t\t\t\tdelete $params{'file_name'};\n+\t\t\t} else {\n+\t\t\t\tdelete $params{'hash'} if $params{'hash'} eq $params{'hash_base'};\n+\t\t\t}\n+\t\t\tdelete $params{'hash_base'};\n+\t\t} elsif (defined $params{'hash'}) {\n+\t\t\t$href .= \"/\".esc_url($params{'hash'});\n+\t\t\tdelete $params{'hash'};\n+\t\t}\n \t}\n \n \t# now encode the parameters explicitly\n-- \n1.5.6.5\n"},{"id":"89608","messageId":"1220435839-29360-3-git-send-email-giuseppe.bilotta@gmail.com","threadId":"15353","inReplyTo":"1220435839-29360-2-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH] gitweb: use_pathinfo filenames start with /","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-09-03T09:57:16Z","receivedAt":"2008-09-03T09:57:16Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"When using path info, make filenames start with a / (right after the :\nthat separates them from the hash base). This minimal change allows\nrelative navigation to work properly when viewing HTML files.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n gitweb/gitweb.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 496dce4..9d4952f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -664,7 +664,7 @@ sub href (%) {\n \t\tif (defined $params{'hash_base'}) {\n \t\t\t$href .= \"/\".esc_url($params{'hash_base'});\n \t\t\tif (defined $params{'file_name'}) {\n-\t\t\t\t$href .= \":\".esc_url($params{'file_name'});\n+\t\t\t\t$href .= \":/\".esc_url($params{'file_name'});\n \t\t\t\tdelete $params{'hash'} if $params{'hash'} eq git_get_hash_by_path($params{'hash_base'},$params{'file_name'});\n \t\t\t\tdelete $params{'file_name'};\n \t\t\t} else {\n-- \n1.5.6.5\n"},{"id":"89609","messageId":"1220435839-29360-4-git-send-email-giuseppe.bilotta@gmail.com","threadId":"15353","inReplyTo":"1220435839-29360-3-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH] gitweb: parse parent..current syntax from pathinfo","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-09-03T09:57:17Z","receivedAt":"2008-09-03T09:57:17Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"This makes it possible to use an URL such as\n$project/somebranch..otherbranch:/filename to get a diff between\ndifferent version of a file. Paths like\n$project/$action/somebranch:/somefile..otherbranch:/otherfile are parsed\nas well.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n gitweb/gitweb.perl |   26 ++++++++++++++++++++++++--\n 1 files changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 9d4952f..3d21624 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -553,7 +553,9 @@ sub evaluate_path_info {\n \t\t$action  = undef;\n \t}\n \n-\tmy ($refname, $pathname) = split(/:/, $path_info, 2);\n+\t$path_info =~ /^((.+?)(:(.+))?\\.\\.)?(.+?)(:(.+))?$/;\n+\tmy ($parentrefname, $parentpathname, $refname, $pathname) = (\n+\t\t$2, $4, $5, $7);\n \tif (defined $pathname) {\n \t\t# we got \"project.git/branch:filename\" or \"project.git/branch:dir/\"\n \t\t# we could use git_get_type(branch:pathname), but it needs $git_dir\n@@ -562,7 +564,11 @@ sub evaluate_path_info {\n \t\t\t$action  ||= \"tree\";\n \t\t\t$pathname =~ s,/$,,;\n \t\t} else {\n-\t\t\t$action  ||= \"blob_plain\";\n+\t\t\tif ($parentrefname) {\n+\t\t\t\t$action ||= \"blobdiff_plain\";\n+\t\t\t} else {\n+\t\t\t\t$action  ||= \"blob_plain\";\n+\t\t\t}\n \t\t}\n \t\t$hash_base ||= validate_refname($refname);\n \t\t$file_name ||= validate_pathname($pathname);\n@@ -573,6 +579,22 @@ sub evaluate_path_info {\n \t\t$hash      ||= validate_refname($refname);\n \t\t$hash_base ||= validate_refname($refname);\n \t}\n+\t# the parent part might be missing the pathname, in which case we use the $file_name, if present\n+\tif (defined $parentrefname) {\n+\t\t$hash_parent_base ||= validate_refname($parentrefname);\n+\t\tif ($parentpathname) {\n+\t\t\t$parentpathname =~ s,^/+,,;\n+\t\t\t$parentpathname =~ s,/$,,;\n+\t\t\t$file_parent ||= validate_pathname($parentpathname);\n+\t\t} else {\n+\t\t\t$file_parent ||= $file_name\n+\t\t}\n+\t\tif (defined $file_parent) {\n+\t\t\t$hash_parent ||= git_get_hash_by_path($hash_parent_base, $file_parent);\n+\t\t} else {\n+\t\t\t$hash_parent ||= validate_refname($parentrefname);\n+\t\t}\n+\t}\n }\n evaluate_path_info();\n \n-- \n1.5.6.5\n"},{"id":"89610","messageId":"1220435839-29360-5-git-send-email-giuseppe.bilotta@gmail.com","threadId":"15353","inReplyTo":"1220435839-29360-4-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH] gitweb: use_pathinfo creates parent..current paths","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-09-03T09:57:18Z","receivedAt":"2008-09-03T09:57:18Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"If use_pathinfo is enabled, href now creates links that contain paths in\nthe form $project/$action/oldhash:/oldname..newhash:/newname for actions\nthat use hash_parent etc.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n gitweb/gitweb.perl |   26 +++++++++++++++++++++++---\n 1 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3d21624..573b416 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -682,9 +682,29 @@ sub href (%) {\n \t\t\tdelete $params{'action'};\n \t\t}\n \n-\t\t# next, we put either hash_base:file_name or hash\n+\t\t# next, we put hash_parent_base:/file_parent..hash_base:/file_name, stripping nonexistent or useless pieces\n+\t\t$href .= \"/\" if ($params{'hash_base'} || $params{'hash_parent_base'} || $params{'hash_parent'} || $params{'hash'});\n \t\tif (defined $params{'hash_base'}) {\n-\t\t\t$href .= \"/\".esc_url($params{'hash_base'});\n+\t\t\tif (defined $params{'hash_parent_base'}) {\n+\t\t\t\t$href .= esc_url($params{'hash_parent_base'});\n+\t\t\t\tif (defined $params{'file_parent'}) {\n+\t\t\t\t\t$href .= \":/\".esc_url($params{'file_parent'}) unless $params{'file_parent'} eq $params{'file_name'};\n+\t\t\t\t\tdelete $params{'hash_parent'} if $params{'hash_parent'} eq git_get_hash_by_path($params{'hash_parent_base'},$params{'file_parent'});\n+\t\t\t\t\tdelete $params{'file_parent'};\n+\t\t\t\t} else {\n+\t\t\t\t\tdelete $params{'hash_parent'} if $params{'hash_parent'} eq $params{'hash_parent_base'};\n+\t\t\t\t\tif ($params{'file_name'}) {\n+\t\t\t\t\t\tdelete $params{'hash_parent'} if $params{'hash_parent'} eq git_get_hash_by_path($params{'hash_parent_base'},$params{'file_name'});\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\t$href .= \"..\";\n+\t\t\t\tdelete $params{'hash_parent_base'};\n+\t\t\t} elsif (defined $params{'hash_parent'}) {\n+\t\t\t\t$href .= esc_url($params{'hash_parent'}). \"..\";\n+\t\t\t\tdelete $params{'hash_parent'};\n+\t\t\t}\n+\n+\t\t\t$href .= esc_url($params{'hash_base'});\n \t\t\tif (defined $params{'file_name'}) {\n \t\t\t\t$href .= \":/\".esc_url($params{'file_name'});\n \t\t\t\tdelete $params{'hash'} if $params{'hash'} eq git_get_hash_by_path($params{'hash_base'},$params{'file_name'});\n@@ -694,7 +714,7 @@ sub href (%) {\n \t\t\t}\n \t\t\tdelete $params{'hash_base'};\n \t\t} elsif (defined $params{'hash'}) {\n-\t\t\t$href .= \"/\".esc_url($params{'hash'});\n+\t\t\t$href .= esc_url($params{'hash'});\n \t\t\tdelete $params{'hash'};\n \t\t}\n \t}\n-- \n1.5.6.5\n"},{"id":"89612","messageId":"1220435839-29360-6-git-send-email-giuseppe.bilotta@gmail.com","threadId":"15353","inReplyTo":"1220435839-29360-5-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH] gitweb: remove PATH_INFO from $my_url and $my_uri","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-09-03T09:57:19Z","receivedAt":"2008-09-03T09:57:19Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"This patch (combined with appropriate server configuration) allows usage\nof the gitweb CGI script as DirectoryIndex for the server root even when\nthe pathinfo feature is enabled.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n gitweb/gitweb.perl |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 573b416..99c891e 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -26,6 +26,10 @@ our $cgi = new CGI;\n our $version = \"++GIT_VERSION++\";\n our $my_url = $cgi->url();\n our $my_uri = $cgi->url(-absolute => 1);\n+if (my $path_info = $ENV{\"PATH_INFO\"}) {\n+\t$my_url =~ s,$path_info$,,;\n+\t$my_uri =~ s,$path_info$,,;\n+}\n \n # core git executable to use\n # this can just be \"git\" if your webserver has a sensible PATH\n-- \n1.5.6.5\n"},{"id":"89613","messageId":"g9lncf$182$1@ger.gmane.org","threadId":"15353","inReplyTo":"1220435839-29360-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: gitweb pathinfo improvements","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-09-03T10:02:54Z","receivedAt":"2008-09-03T10:02:54Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Damn, I forgot the patch sequence numbers.\n\nOn Wednesday 03 September 2008 11:57, Giuseppe Bilotta wrote:\n> The following patchset improves on gitweb's support for PATH_INFO\n> by supporting paths in the form project/action/[parent..]hash,\n> both in generating them and in accepting them. The old path info\n> style project/hash is still supported as long as it doesn't\n> conflict with the new style\n> \n> For those that prefer git trees to patch bombs, my git tree is\n> available for gitweb browsing at http://git.oblomov.eu/git and for\n> git cloning at git://git.oblomov.eu/git/git\n> \n> The changes are very local to the PATH_INFO parsing and creation,\n> so I hope they don't conflict with Lea's cache work.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"89778","messageId":"7v63pb4tv6.fsf@gitster.siamese.dyndns.org","threadId":"15353","inReplyTo":"g9lncf$182$1@ger.gmane.org","subject":"Re: gitweb pathinfo improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-04T19:02:37Z","receivedAt":"2008-09-04T19:02:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Please don't top post ;-).\n\nGiuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:\n\n> Damn, I forgot the patch sequence numbers.\n\nAnd please don't send a message without useful contents.  Instead of\ncursing alone, after that sentence you could have said:\n\n    Each of them is a reply to the previous one in the sequence so if your\n    MUA threads correctly that is the order of the patches in the sequence,\n    but in case yours doesn't, they should apply in this order:\n\n        [PATCH 1/5] gitweb: action in path with use_pathinfo\n        [PATCH 2/5] gitweb: use_pathinfo filenames start with /\n        [PATCH 3/5] gitweb: parse parent..current syntax from pathinfo\n        [PATCH 4/5] gitweb: use_pathinfo creates parent..current paths\n        [PATCH 5/5] gitweb: remove PATH_INFO from $my_url and $my_uri\n"},{"id":"89900","messageId":"200809061322.31094.jnareb@gmail.com","threadId":"15353","inReplyTo":"1220435839-29360-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: gitweb pathinfo improvements","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-09-06T11:22:29Z","receivedAt":"2008-09-06T11:22:29Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Hi, Giuseppe!\n\nI'm sorry for the delay in reviewing this series\n\nOn Wed, 3 September 2008, Giuseppe Bilotta wrote:\n> \n> The following patchset improves on gitweb's support for PATH_INFO\n> by supporting paths in the form project/action/[parent..]hash,\n> both in generating them and in accepting them. The old path info\n> style project/hash is still supported as long as it doesn't\n> conflict with the new style\n> \n> For those that prefer git trees to patch bombs, my git tree is\n> available for gitweb browsing at http://git.oblomov.eu/git and for\n> git cloning at git://git.oblomov.eu/git/git\n> \n> The changes are very local to the PATH_INFO parsing and creation,\n> so I hope they don't conflict with Lea's cache work.\n\nLet me comment first on _intent_ of the patches, and on series as\na whole, before examining individual patches in detail.\n\nBelow there is table of contents / shortlog of this series, which I \nthink is a good practice to include in cover letter describing the \nseries:\n\nTable of contents:\n==================\n * [PATCH 1/5] gitweb: action in path with use_pathinfo\n * [PATCH 2/5] gitweb: use_pathinfo filenames start with /\n * [PATCH 3/5] gitweb: parse parent..current syntax from pathinfo\n * [PATCH 4/5] gitweb: use_pathinfo creates parent..current paths\n * [PATCH 5/5] gitweb: remove PATH_INFO from $my_url and $my_uri\n\nThis series of patches deal with moving more parameters from CGI query \nto pathinfo (beside 2nd and 5th, which are about relative navigation in \nHTML files ('blob_plain' view) and ability to act as DirectoryIndex, \nwhich means that they are about using gitweb as web server).\n\nThere are two sides of those pathinfo improvement: first is to be able \nto get more parameters from pathinfo, second is to use pathinfo-URLs\nin gitweb links if requested/configured.\n\nThe problem with fitting more parameters in pathinfo is first backwards \ncompatibility (this is not strict requirement, but we would rather not \nmake existing bookmarked pathinfo URLs invalid), and second with \nordering those parameters and detecting when one parameter ends and \nnext one begins (which is made more complicated by the fact that some \nparameters, like action or hash/hash_base can be skipped).\n\nThis trouble with fitting parameters in pathinfo creates some \nlimitations and tradeoffs. For example (optionally!) embedding\nthe action in the pathinfo, by putting it after project and before \nhash/hash_base (usually refname) and filename makes old-style \n$project/$branch lead to incorrect view. This also means that we have \nto be careful about creating pathinfo links, either by always putting \nan action, or using full ref name (which I think we do anyway to avoid \ntag/head ambiguities); or doing it only in the case of possible \nconflict i.e. branch named like one of gitweb actions.\n\nUsing ':' or ':/' to separate branch name (hash or hash_base) from \nfilename doesn't lead to problems, as pathinfo is split on _first_ \noccurrence of ':', and refnames cannot contain ':'. Using '..' to \nseparate $hash_parent_base:$file_parent from $hash_base:$filename\nis IMVHO a very good idea... but when creating pathinfo links we have\nto consider filenames with '..' in them; an example is there in git \nrepository: \"t/t5515/refs.master_.._.git\" file. Then we probably want \nto fallback on CGI query/CGI parameters. NOTE: I have not read the \npatch yet, perhaps it does this.\n\n\nBy the way, this is perhaps slightly outside the issue of this series, \nbut having a..b syntax would tempt to handcraft gitweb URLs for \nequivalents of \"git log a..b\", \"git log a...b\" and \"git diff a...b\",\nneither of which works yet.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"89901","messageId":"cb7bb73a0809060455n78145ccdw99beeebbd7e0439a@mail.gmail.com","threadId":"15353","inReplyTo":"200809061322.31094.jnareb@gmail.com","subject":"Re: gitweb pathinfo improvements","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-09-06T11:55:44Z","receivedAt":"2008-09-06T11:55:44Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Sat, Sep 6, 2008 at 1:22 PM, Jakub Narebski <jnareb@gmail.com> wrote:\n> Below there is table of contents / shortlog of this series, which I\n> think is a good practice to include in cover letter describing the\n> series:\n>\n> Table of contents:\n> ==================\n>  * [PATCH 1/5] gitweb: action in path with use_pathinfo\n>  * [PATCH 2/5] gitweb: use_pathinfo filenames start with /\n>  * [PATCH 3/5] gitweb: parse parent..current syntax from pathinfo\n>  * [PATCH 4/5] gitweb: use_pathinfo creates parent..current paths\n>  * [PATCH 5/5] gitweb: remove PATH_INFO from $my_url and $my_uri\n\nYes, I really have to learn proper behaviour when sending a patchest.\nI'll make treasure of yours and Junio's hints on the matter 8-)\n\nI'll probably have to resend this patch series anyway, since I've\nalready found a quirk for which an additional patch is ready, and the\ndouble-dot-filename thing you mention below needs fixing as well.\n\nBTW, is there a way to automate this summary generation when using\nformat-patch or send-email?\n\n> The problem with fitting more parameters in pathinfo is first backwards\n> compatibility (this is not strict requirement, but we would rather not\n> make existing bookmarked pathinfo URLs invalid), and second with\n> ordering those parameters and detecting when one parameter ends and\n> next one begins (which is made more complicated by the fact that some\n> parameters, like action or hash/hash_base can be skipped).\n>\n> This trouble with fitting parameters in pathinfo creates some\n> limitations and tradeoffs. For example (optionally!) embedding\n> the action in the pathinfo, by putting it after project and before\n> hash/hash_base (usually refname) and filename makes old-style\n> $project/$branch lead to incorrect view. This also means that we have\n> to be careful about creating pathinfo links, either by always putting\n> an action, or using full ref name (which I think we do anyway to avoid\n> tag/head ambiguities); or doing it only in the case of possible\n> conflict i.e. branch named like one of gitweb actions.\n\nYes, this was something that got me thinking for a while. I've tried\nas hard as possible to preserve backwards compatibility, and old-style\npaths should still work correctly except for projects whose branched\nare named exactly like gitweb actions.\n\n> Using ':' or ':/' to separate branch name (hash or hash_base) from\n> filename doesn't lead to problems, as pathinfo is split on _first_\n> occurrence of ':', and refnames cannot contain ':'. Using '..' to\n> separate $hash_parent_base:$file_parent from $hash_base:$filename\n> is IMVHO a very good idea... but when creating pathinfo links we have\n> to consider filenames with '..' in them; an example is there in git\n> repository: \"t/t5515/refs.master_.._.git\" file. Then we probably want\n> to fallback on CGI query/CGI parameters. NOTE: I have not read the\n> patch yet, perhaps it does this.\n\nActually, this was not a case I had taken into consideration (a\nfilename with two dots). It should be straightforward to change the\nlink-creation code to switch to CGI parameters in this case. Should I\nchange the corresponding patch, or would it be enough to add a patch\nto the series clearing this issue?\n\n> By the way, this is perhaps slightly outside the issue of this series,\n> but having a..b syntax would tempt to handcraft gitweb URLs for\n> equivalents of \"git log a..b\", \"git log a...b\" and \"git diff a...b\",\n> neither of which works yet.\n\nI do have a patch to that effect for the shortlog action:\nhttp://git.oblomov.eu/git/commitdiff/refs/heads/gitweb/shortlog and\nyou can see it in effect on my gitweb with links such as\nhttp://git.oblomov.eu/git/master..gitweb/pathinfo.\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"89904","messageId":"200809061447.40459.jnareb@gmail.com","threadId":"15353","inReplyTo":"cb7bb73a0809060455n78145ccdw99beeebbd7e0439a@mail.gmail.com","subject":"Re: gitweb pathinfo improvements","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-09-06T12:47:39Z","receivedAt":"2008-09-06T12:47:39Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 6 Sep 2008, Giuseppe Bilotta wrote:\n> On Sat, Sep 6, 2008, Jakub Narebski <jnareb@gmail.com> wrote:\n\n> > Below there is table of contents / shortlog of this series, which I\n> > think is a good practice to include in cover letter describing the\n> > series:\n> >\n> > Table of contents:\n> > ==================\n> >  * [PATCH 1/5] gitweb: action in path with use_pathinfo\n> >  * [PATCH 2/5] gitweb: use_pathinfo filenames start with /\n> >  * [PATCH 3/5] gitweb: parse parent..current syntax from pathinfo\n> >  * [PATCH 4/5] gitweb: use_pathinfo creates parent..current paths\n> >  * [PATCH 5/5] gitweb: remove PATH_INFO from $my_url and $my_uri\n> \n> Yes, I really have to learn proper behaviour when sending a patchest.\n> I'll make treasure of yours and Junio's hints on the matter 8-)\n> \n> I'll probably have to resend this patch series anyway, since I've\n> already found a quirk for which an additional patch is ready, and the\n> double-dot-filename thing you mention below needs fixing as well.\n> \n> BTW, is there a way to automate this summary generation when using\n> format-patch or send-email?\n\nYes, if you have new enough git.\n\nFirst, to have numbered patches (just in case mail threading screws up,\nfor example if emails come out of order, or if you don't use chain\nreply to), you can use -n / --numbered option, or format.numbered=auto\n(I think that now auto numbering is the default).\n\nTo generate template for cover letter you can use new --cover-letter\noption. I don't think that there is configuration option for that.\n\nAdditionally when sending series of patches I usually output patches to \nseparate directory (mdir.1, mdir.2,...) using -o <directory> option,\nbut that is just my workflow.\n\n> > Using ':' or ':/' to separate branch name (hash or hash_base) from\n> > filename doesn't lead to problems, as pathinfo is split on _first_\n> > occurrence of ':', and refnames cannot contain ':'. Using '..' to\n> > separate $hash_parent_base:$file_parent from $hash_base:$filename\n> > is IMVHO a very good idea... but when creating pathinfo links we\n> > have to consider filenames with '..' in them; an example is there\n> > in git repository: \"t/t5515/refs.master_.._.git\" file. Then we\n> > probably want to fallback on CGI query/CGI parameters. \n> > NOTE: I have not read the patch yet, perhaps it does this.\n> \n> Actually, this was not a case I had taken into consideration (a\n> filename with two dots). It should be straightforward to change the\n> link-creation code to switch to CGI parameters in this case. Should I\n> change the corresponding patch, or would it be enough to add a patch\n> to the series clearing this issue?\n\nI think it would be better to have it correct the first time;\nespecially if you are resending series anyway.\n\n> > By the way, this is perhs slightly outside the issue of this series,\n> > but having a..b syntax would tempt to handcraft gitweb URLs for\n> > equivalents of \"git log a..b\", \"git log a...b\" and \"git diff a...b\",\n> > neither of which works yet.\n> \n> I do have a patch to that effect for the shortlog action:\n> http://git.oblomov.eu/git/commitdiff/refs/heads/gitweb/shortlog and\n> you can see it in effect on my gitweb with links such as\n> http://git.oblomov.eu/git/master..gitweb/pathinfo.\n\nI think that it would be good idea to first refactor generation of \nlog-like views in git (log, shortlog, history, search, feed) to \nconsolidate them somehow, so revision limiting would work on _all_ \nlog-like views.\n\nThis is in my TODO list, but I didn't even began implementing it.\n\n-- \nJakub Narebski\nPoland\n"}]}