threads / patch / 15353

patchgitweb: action in path with use_pathinfo

Subject: [PATCH] gitweb: action in path with use_pathinfo

## tl;dr

11 messages between Sep 3, 2008 and Sep 6, 2008. Diffs are folded; open one to read it.

replies: 10people: 3as markdown or json

Giuseppe Bilotta· Sep 3, 2008, 09:57 UTC · lore

gitweb pathinfo improvements

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 Bilotta· Sep 3, 2008, 09:57 UTC · re: Giuseppe Bilotta · lore

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(-)
Show changes to gitweb/gitweb.perl +72 −37
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
Giuseppe Bilotta· Sep 3, 2008, 09:57 UTC · re: Giuseppe Bilotta · lore

[PATCH] gitweb: use_pathinfo filenames start with /

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(-)
Show changes to gitweb/gitweb.perl +1 −1
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
Giuseppe Bilotta· Sep 3, 2008, 09:57 UTC · re: Giuseppe Bilotta · lore

[PATCH] gitweb: parse parent..current syntax from pathinfo

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(-)
Show changes to gitweb/gitweb.perl +24 −2
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
Giuseppe Bilotta· Sep 3, 2008, 09:57 UTC · re: Giuseppe Bilotta · lore

[PATCH] gitweb: use_pathinfo creates parent..current paths

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(-)
Show changes to gitweb/gitweb.perl +23 −3
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
Giuseppe Bilotta· Sep 3, 2008, 09:57 UTC · re: Giuseppe Bilotta · lore

[PATCH] gitweb: remove PATH_INFO from $my_url and $my_uri

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(-)
Show changes to gitweb/gitweb.perl +4 −0
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
Giuseppe Bilotta· Sep 3, 2008, 10:02 UTC · re: Giuseppe Bilotta · lore

Re: gitweb pathinfo improvements

Damn, I forgot the patch sequence numbers.
On Wednesday 03 September 2008 11:57, Giuseppe Bilotta wrote:
Show 12 quoted lines
> 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
Junio C Hamano· Sep 4, 2008, 19:02 UTC · re: Giuseppe Bilotta · lore

Re: gitweb pathinfo improvements

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
Jakub Narebski· Sep 6, 2008, 11:22 UTC · re: Giuseppe Bilotta · lore

Re: gitweb pathinfo improvements

Hi, Giuseppe!
I'm sorry for the delay in reviewing this series
On Wed, 3 September 2008, Giuseppe Bilotta wrote:
Show 13 quoted lines
> 
> 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
Giuseppe Bilotta· Sep 6, 2008, 11:55 UTC · re: Jakub Narebski · lore

Re: gitweb pathinfo improvements

On Sat, Sep 6, 2008 at 1:22 PM, Jakub Narebski <jnareb@gmail.com> wrote:
Show 11 quoted lines
> 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?

Show 16 quoted lines
> 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.

Show 9 quoted lines
> 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
Jakub Narebski· Sep 6, 2008, 12:47 UTC · re: Giuseppe Bilotta · lore

Re: gitweb pathinfo improvements

On Sat, 6 Sep 2008, Giuseppe Bilotta wrote:
> On Sat, Sep 6, 2008, Jakub Narebski <jnareb@gmail.com> wrote:
Show 21 quoted lines
> > 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.

Show 15 quoted lines
> > 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.

Show 9 quoted lines
> > 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

← back to recent threads