threads / patch / 9362

patchgitweb: Provide RSS feeds for file history

Subject: [PATCH] gitweb: Provide RSS feeds for file history

## tl;dr

5 messages between Aug 3, 2007 and Aug 4, 2007. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Steven Walter· Aug 3, 2007, 02:05 UTC · lore
If git_feed is provided a file name, it ought to show only the history
affecting that file.  The title was already being set correctly, but all
commits from history were being shown anyway.
---
 gitweb/gitweb.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Show changes to gitweb/gitweb.perl +2 −1
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 498b936..26932a4 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -611,6 +611,7 @@ sub href(%) {
 	my %mapping = @mapping;
 
 	$params{'project'} = $project unless exists $params{'project'};
+	$params{'file_name'} = $file_name unless exists $params{'file_name'};
 
 	my ($use_pathinfo) = gitweb_check_feature('pathinfo');
 	if ($use_pathinfo) {
@@ -5365,7 +5366,7 @@ sub git_feed {
 
 	# log/feed of current (HEAD) branch, log of given branch, history of file/directory
 	my $head = $hash || 'HEAD';
-	my @commitlist = parse_commits($head, 150);
+	my @commitlist = parse_commits($head, 150, 0, "--full-history", $file_name);
 
 	my %latest_commit;
 	my %latest_date;
-- 
1.5.2.3


-- 
-Steven Walter <swalter@lexmark.com>
Jakub Narebski· Aug 3, 2007, 09:10 UTC · re: Steven Walter · lore

Re: [PATCH] gitweb: Provide RSS feeds for file history

Steven Walter wrote:
Nak. Explanation below. Corrected patch will follow.
> If git_feed is provided a file name, it ought to show only the history
> affecting that file.  The title was already being set correctly, but all
> commits from history were being shown anyway.

This is a bug introduced while changing gitweb (among others git_feed subroutine) to use parse_commits, in commit b6093a5c. Earlier it worked. So the explanation (in commit message) is not full.

By the way it affects not only RSS but also Atom feeds.
Documentation/SubmittingPatches:
  Checklist (and a short version for the impatient):
        Commits:
  [...]
        - if you want your work included in git.git, add a
          "Signed-off-by: Your Name <your@email.com>" line to the
          commit message (or just use the option "-s" when
          committing) to confirm that you agree to the Developer's
          Certificate of Origin
Show 16 quoted lines
> ---
>  gitweb/gitweb.perl |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 498b936..26932a4 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -611,6 +611,7 @@ sub href(%) {
>  	my %mapping = @mapping;
>  
>  	$params{'project'} = $project unless exists $params{'project'};
> +	$params{'file_name'} = $file_name unless exists $params{'file_name'};
>  
>  	my ($use_pathinfo) = gitweb_check_feature('pathinfo');
>  	if ($use_pathinfo) {

This is a big, intrusive change. It makes 'file_name' default argument, unless overriden. While it made sense for 'project' parameter, as almost all URLs in gitweb needed it, more than half URLs does not need 'file_name' parameter. And some of those URLs are present in a views which do use 'file_name'.

If you wanted alternative URLs for a feed preserve 'file_name' parameter, do it explicitely.

Show 9 quoted lines
> @@ -5365,7 +5366,7 @@ sub git_feed {
>  
>  	# log/feed of current (HEAD) branch, log of given branch, history of file/directory
>  	my $head = $hash || 'HEAD';
> -	my @commitlist = parse_commits($head, 150);
> +	my @commitlist = parse_commits($head, 150, 0, "--full-history", $file_name);
>  
>  	my %latest_commit;
>  	my %latest_date;

I'd rather not use "--full-history" for feeds. We use it in the 'history' view for backward compatibility reasons; I'd rather leave it for extra options in the feed.

-- 
Jakub Narebski
Poland
Jakub Narebski· Aug 3, 2007, 17:50 UTC · re: Jakub Narebski · lore

[PATCH] gitweb: Fix handling of $file_name in feed generation

>From 6ef05672bb1dd1fe1ded15707164eaac36772c21 Mon Sep 17 00:00:00 2001
From: Steven Walter <stevenrwalter@gmail.com>
From: Jakub Narebski <jnareb@gmail.com>
Date: Fri, 3 Aug 2007 19:35:00 +0200
Subject: [PATCH] gitweb: Fix handling of $file_name in feed generation
The commit b6093a5c, by Robert Fitzsimons:
  "gitweb: Change atom, rss actions to use parse_commits."
forgot to pass $file_name parameter to parse_commits subroutine.

If git_feed is provided a file name, it ought to show only the history affecting that file or a directory. The title was being set correctly, but all commits from history were being shown.

Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Three comments. First, authorship. Steven Walter didn't signoff his patch,
and the contents differs a bit from second chunk of his patch (see
comment below) and I have added infor about which commit introduced this
bug to the commit message. I have added my signoff, you can take this
patch as either mine or Steven authorship.

Second, I have discarded first chunk in Steven patch because it was too intrusive. As I have said, it makes 'file_name' default argument, unless overriden. While it made sense for 'project' parameter to be made default parameter in href(), as almost all URLs in gitweb needed it, more than half URLs does not need 'file_name' parameter. And some of those URLs are present in a views which do use 'file_name'.

So if we want alternative URLs for a feed preserve 'file_name' parameter, or we want RSS/Atom links for "file_name" kind of views, like 'tree', 'blob' or 'history' views, we should add 'file_name' parameter explicitely, and not change href() to do it implicitely.

But as we are in stabilization (freeze) stage, I'd rather not add any new features. This one just fixes a bug in gitweb.

Third, I'd rather not use "--full-history" for feeds. We use it in the 'history' view for backward compatibility reasons; I'd rather leave it for extra options in the feed. But this is also for after the release.

 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 498b936..4733728 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5365,7 +5365,7 @@ sub git_feed {
 
 	# log/feed of current (HEAD) branch, log of given branch, history of file/directory
 	my $head = $hash || 'HEAD';
-	my @commitlist = parse_commits($head, 150);
+	my @commitlist = parse_commits($head, 150, 0, undef, $file_name);
 
 	my %latest_commit;
 	my %latest_date;
-- 
1.5.2.4
Junio C Hamano· Aug 4, 2007, 00:25 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: Fix handling of $file_name in feed generation

Thanks. This came in while I already tagged -rc4 and was doing the final round of tests and packaging, so has to become the first thing post -rc4, unfortunately.

The format of the e-mail message you sent out could use much improvements but I'll let it pass this time.

Robert Fitzsimons· Aug 4, 2007, 00:27 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: Fix handling of $file_name in feed generation

Show 12 quoted lines
> Subject: [PATCH] gitweb: Fix handling of $file_name in feed generation
> 
> The commit b6093a5c, by Robert Fitzsimons:
>   "gitweb: Change atom, rss actions to use parse_commits."
> forgot to pass $file_name parameter to parse_commits subroutine.
> 
> If git_feed is provided a file name, it ought to show only the history
> affecting that file or a directory.  The title was being set
> correctly, but all commits from history were being shown.
> 
> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Silly me.  Jakub's change is the correct fix for the bug.
Robert

← back to recent threads