[PATCH] Gitweb: Fix unintended "--no-merges" for regular Atom feed

[PATCH] Gitweb: Fix unintended "--no-merges" for regular Atom feed

[Sebastian Pipping]

[-- Attachment #1: Type: text/plain, Size: 117 bytes --]

Hello!


Please excuse that I send the patch as an attachment and consider
application.  Thanks!

Best,



Sebastian

[-- Attachment #2: 0001-Gitweb-Fix-unintended-no-merges-for-regular-Atom-fee.patch --]
[-- Type: text/x-patch, Size: 1196 bytes --]

>From 03bed689671df47040c4a0ad158c0c9b039a50bf Mon Sep 17 00:00:00 2001
From: Sebastian Pipping <sebastian@pipping.org>
Date: Mon, 2 Apr 2012 18:39:48 +0200
Subject: [PATCH] Gitweb: Fix unintended "--no-merges" for regular Atom feed

Before:
<link rel="alternate" title="[..] - Atom feed" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" />
<link rel="alternate" title="[..] - Atom feed (no merges)" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" />

After:
<link rel="alternate" title="[..] - Atom feed" href="/?p=.git;a=atom;opt=" type="application/atom+xml" />
<link rel="alternate" title="[..] - Atom feed (no merges)" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" />
---
 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a8b5fad..cc45ae3 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3886,6 +3886,7 @@ sub print_feed_meta {
 				'-type' => "application/$type+xml"
 			);
 
+			$href_params{'extra_options'} = '';
 			$href_params{'action'} = $type;
 			$link_attr{'-href'} = href(%href_params);
 			print "<link ".
-- 
1.7.8.5

[PATCH (bugfix)] gitweb: Fix unintended "--no-merges" for regular Atom feed

[Jakub Narebski]

From: Sebastian Pipping <sebastian@pipping.org>

The print_feed_meta() subroutine generates links for feeds with and
without merges, in RSS and Atom formats.  However because %href_params
was not properly reset, it generated links with "--no-merges" for all
except the very first link.

Before:
<link rel="alternate" title="[..] - Atom feed" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" />
<link rel="alternate" title="[..] - Atom feed (no merges)" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" />

After:
<link rel="alternate" title="[..] - Atom feed" href="/?p=.git;a=atom" type="application/atom+xml" />
<link rel="alternate" title="[..] - Atom feed (no merges)" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" />

Signed-off-by: Sebastian Pipping <sebastian@pipping.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Sebastian Pipping wrote:

> Please excuse that I send the patch as an attachment and consider
> application.  Thanks!

Beside sending this patch as an attachement instead of putting it
inline (what was the cause of this?) it was also lacking
Signed-off-by... which I have forged.

I have added explanation of this error in the commit message, and
changed from using '' to undef to get rid of 'opt' / 'extra_options'
parameter instead of having it empty.  It is a better way of doing the
reset.

Junio, the bug is very minor, so I don't know if it is worth fixing
for 1.7.10.

 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a8b5fad2..ca6f038 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3886,6 +3886,7 @@ sub print_feed_meta {
 				'-type' => "application/$type+xml"
 			);
 
+			$href_params{'extra_options'} = undef;
 			$href_params{'action'} = $type;
 			$link_attr{'-href'} = href(%href_params);
 			print "<link ".
-- 
1.7.9

Re: [PATCH (bugfix)] gitweb: Fix unintended "--no-merges" for regular Atom feed

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Junio, the bug is very minor, so I don't know if it is worth fixing
> for 1.7.10.

Does this exist in 1.7.9.x maintenance track?  If it is an old bug, I do
not think it should go to 1.7.10 proper (perhaps 1.7.10.1) this late, and
if it is a bug in a new feature added for 1.7.10, we may want to fix it
before the final, as the impact of the patch seems very minor.

>  gitweb/gitweb.perl |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index a8b5fad2..ca6f038 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3886,6 +3886,7 @@ sub print_feed_meta {
>  				'-type' => "application/$type+xml"
>  			);
>  
> +			$href_params{'extra_options'} = undef;
>  			$href_params{'action'} = $type;
>  			$link_attr{'-href'} = href(%href_params);
>  			print "<link ".

Re: [PATCH (bugfix)] gitweb: Fix unintended "--no-merges" for regular Atom feed

[Jakub Narebski]

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Junio, the bug is very minor, so I don't know if it is worth fixing
> > for 1.7.10.
> 
> Does this exist in 1.7.9.x maintenance track?  If it is an old bug, I do
> not think it should go to 1.7.10 proper (perhaps 1.7.10.1) this late, and
> if it is a bug in a new feature added for 1.7.10, we may want to fix it
> before the final, as the impact of the patch seems very minor.

It is an old bug, from 05bb5a2 (gitweb: Include links to feeds in HTML
header only for '200 OK' response, 2010-12-18) which refactored feed link
generation into print_feed_meta().  It is in gitweb since v1.7.4 I think.

So 1.7.10.1 it is...

> >  gitweb/gitweb.perl |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index a8b5fad2..ca6f038 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -3886,6 +3886,7 @@ sub print_feed_meta {
> >  				'-type' => "application/$type+xml"
> >  			);
> >  
> > +			$href_params{'extra_options'} = undef;
> >  			$href_params{'action'} = $type;
> >  			$link_attr{'-href'} = href(%href_params);
> >  			print "<link ".
> 

-- 
Jakub Narebski
Poland

Re: [PATCH (bugfix)] gitweb: Fix unintended "--no-merges" for regular Atom feed

[Jakub Narebski]

On Wed, 4 April 2012, Jakub Narebski wrote:
> Junio C Hamano wrote:
> > Jakub Narebski <jnareb@gmail.com> writes:
> > 
> > > Junio, the bug is very minor, so I don't know if it is worth fixing
> > > for 1.7.10.
> > 
> > Does this exist in 1.7.9.x maintenance track?  If it is an old bug, I do
> > not think it should go to 1.7.10 proper (perhaps 1.7.10.1) this late, and
> > if it is a bug in a new feature added for 1.7.10, we may want to fix it
> > before the final, as the impact of the patch seems very minor.
> 
> It is an old bug, from 05bb5a2 (gitweb: Include links to feeds in HTML
> header only for '200 OK' response, 2010-12-18) which refactored feed link
> generation into print_feed_meta().  It is in gitweb since v1.7.4 I think.
> 
> So 1.7.10.1 it is...

Ping!

I don't see this trivial fix (admittedly for obscure bug) in "What's
cooking", and it is not present in 'master'.
 
> > >  gitweb/gitweb.perl |    1 +
> > >  1 files changed, 1 insertions(+), 0 deletions(-)

-- 
Jakub Narebski
Poland

Re: [PATCH (bugfix)] gitweb: Fix unintended "--no-merges" for regular Atom feed

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

>> So 1.7.10.1 it is...
>
> Ping!

You are part of my distributed memory and the system is nicely
working ;-)

> I don't see this trivial fix (admittedly for obscure bug) in "What's
> cooking", and it is not present in 'master'.

Of course not.  Now you reminded me, it will.

Thanks.