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

From: Jakub Narebski <jnareb@gmail.com>
Date: Aug 3, 2007, 09:10 UTC
Message-ID: <200708031110.55969.jnareb@gmail.com>
In-Reply-To: <20070803020555.GB8593@dervierte>

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