{"thread":{"id":"30128","subject":"[PATCH] Gitweb: Fix unintended \"--no-merges\" for regular Atom feed","startedAt":"2012-04-02T16:44:29Z","lastAt":"2012-04-11T16:48:48Z","messageCount":6,"participants":["Sebastian Pipping","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"188328","messageId":"4F79D76D.80805@pipping.org","threadId":"30128","inReplyTo":null,"subject":"[PATCH] Gitweb: Fix unintended \"--no-merges\" for regular Atom feed","fromName":"Sebastian Pipping","fromEmail":"sebastian@pipping.org","sentAt":"2012-04-02T16:44:29Z","receivedAt":"2012-04-02T16:44:29Z","isPatch":true,"sender":{"key":"sebastian@pipping.org","avatar":null},"body":"Hello!\n\n\nPlease excuse that I send the patch as an attachment and consider\napplication.  Thanks!\n\nBest,\n\n\n\nSebastian\n\n\n>From 03bed689671df47040c4a0ad158c0c9b039a50bf Mon Sep 17 00:00:00 2001\nFrom: Sebastian Pipping <sebastian@pipping.org>\nDate: Mon, 2 Apr 2012 18:39:48 +0200\nSubject: [PATCH] Gitweb: Fix unintended \"--no-merges\" for regular Atom feed\n\nBefore:\n<link rel=\"alternate\" title=\"[..] - Atom feed\" href=\"/?p=.git;a=atom;opt=--no-merges\" type=\"application/atom+xml\" />\n<link rel=\"alternate\" title=\"[..] - Atom feed (no merges)\" href=\"/?p=.git;a=atom;opt=--no-merges\" type=\"application/atom+xml\" />\n\nAfter:\n<link rel=\"alternate\" title=\"[..] - Atom feed\" href=\"/?p=.git;a=atom;opt=\" type=\"application/atom+xml\" />\n<link rel=\"alternate\" title=\"[..] - Atom feed (no merges)\" href=\"/?p=.git;a=atom;opt=--no-merges\" type=\"application/atom+xml\" />\n---\n gitweb/gitweb.perl |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex a8b5fad..cc45ae3 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3886,6 +3886,7 @@ sub print_feed_meta {\n \t\t\t\t'-type' => \"application/$type+xml\"\n \t\t\t);\n \n+\t\t\t$href_params{'extra_options'} = '';\n \t\t\t$href_params{'action'} = $type;\n \t\t\t$link_attr{'-href'} = href(%href_params);\n \t\t\tprint \"<link \".\n-- \n1.7.8.5\n\n"},{"id":"188484","messageId":"1333542344-20421-1-git-send-email-jnareb@gmail.com","threadId":"30128","inReplyTo":"4F79D76D.80805@pipping.org","subject":"[PATCH (bugfix)] gitweb: Fix unintended \"--no-merges\" for regular Atom feed","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-04-04T12:25:44Z","receivedAt":"2012-04-04T12:25:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: Sebastian Pipping <sebastian@pipping.org>\n\nThe print_feed_meta() subroutine generates links for feeds with and\nwithout merges, in RSS and Atom formats.  However because %href_params\nwas not properly reset, it generated links with \"--no-merges\" for all\nexcept the very first link.\n\nBefore:\n<link rel=\"alternate\" title=\"[..] - Atom feed\" href=\"/?p=.git;a=atom;opt=--no-merges\" type=\"application/atom+xml\" />\n<link rel=\"alternate\" title=\"[..] - Atom feed (no merges)\" href=\"/?p=.git;a=atom;opt=--no-merges\" type=\"application/atom+xml\" />\n\nAfter:\n<link rel=\"alternate\" title=\"[..] - Atom feed\" href=\"/?p=.git;a=atom\" type=\"application/atom+xml\" />\n<link rel=\"alternate\" title=\"[..] - Atom feed (no merges)\" href=\"/?p=.git;a=atom;opt=--no-merges\" type=\"application/atom+xml\" />\n\nSigned-off-by: Sebastian Pipping <sebastian@pipping.org>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nSebastian Pipping wrote:\n\n> Please excuse that I send the patch as an attachment and consider\n> application.  Thanks!\n\nBeside sending this patch as an attachement instead of putting it\ninline (what was the cause of this?) it was also lacking\nSigned-off-by... which I have forged.\n\nI have added explanation of this error in the commit message, and\nchanged from using '' to undef to get rid of 'opt' / 'extra_options'\nparameter instead of having it empty.  It is a better way of doing the\nreset.\n\nJunio, the bug is very minor, so I don't know if it is worth fixing\nfor 1.7.10.\n\n gitweb/gitweb.perl |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex a8b5fad2..ca6f038 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3886,6 +3886,7 @@ sub print_feed_meta {\n \t\t\t\t'-type' => \"application/$type+xml\"\n \t\t\t);\n \n+\t\t\t$href_params{'extra_options'} = undef;\n \t\t\t$href_params{'action'} = $type;\n \t\t\t$link_attr{'-href'} = href(%href_params);\n \t\t\tprint \"<link \".\n-- \n1.7.9\n"},{"id":"188502","messageId":"7v62df9yo5.fsf@alter.siamese.dyndns.org","threadId":"30128","inReplyTo":"1333542344-20421-1-git-send-email-jnareb@gmail.com","subject":"Re: [PATCH (bugfix)] gitweb: Fix unintended \"--no-merges\" for regular Atom feed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-04T17:47:38Z","receivedAt":"2012-04-04T17:47:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Junio, the bug is very minor, so I don't know if it is worth fixing\n> for 1.7.10.\n\nDoes this exist in 1.7.9.x maintenance track?  If it is an old bug, I do\nnot think it should go to 1.7.10 proper (perhaps 1.7.10.1) this late, and\nif it is a bug in a new feature added for 1.7.10, we may want to fix it\nbefore the final, as the impact of the patch seems very minor.\n\n>  gitweb/gitweb.perl |    1 +\n>  1 files changed, 1 insertions(+), 0 deletions(-)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index a8b5fad2..ca6f038 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -3886,6 +3886,7 @@ sub print_feed_meta {\n>  \t\t\t\t'-type' => \"application/$type+xml\"\n>  \t\t\t);\n>  \n> +\t\t\t$href_params{'extra_options'} = undef;\n>  \t\t\t$href_params{'action'} = $type;\n>  \t\t\t$link_attr{'-href'} = href(%href_params);\n>  \t\t\tprint \"<link \".\n"},{"id":"188509","messageId":"201204042058.32549.jnareb@gmail.com","threadId":"30128","inReplyTo":"7v62df9yo5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH (bugfix)] gitweb: Fix unintended \"--no-merges\" for regular Atom feed","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-04-04T18:58:31Z","receivedAt":"2012-04-04T18:58:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > Junio, the bug is very minor, so I don't know if it is worth fixing\n> > for 1.7.10.\n> \n> Does this exist in 1.7.9.x maintenance track?  If it is an old bug, I do\n> not think it should go to 1.7.10 proper (perhaps 1.7.10.1) this late, and\n> if it is a bug in a new feature added for 1.7.10, we may want to fix it\n> before the final, as the impact of the patch seems very minor.\n\nIt is an old bug, from 05bb5a2 (gitweb: Include links to feeds in HTML\nheader only for '200 OK' response, 2010-12-18) which refactored feed link\ngeneration into print_feed_meta().  It is in gitweb since v1.7.4 I think.\n\nSo 1.7.10.1 it is...\n\n> >  gitweb/gitweb.perl |    1 +\n> >  1 files changed, 1 insertions(+), 0 deletions(-)\n> >\n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > index a8b5fad2..ca6f038 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -3886,6 +3886,7 @@ sub print_feed_meta {\n> >  \t\t\t\t'-type' => \"application/$type+xml\"\n> >  \t\t\t);\n> >  \n> > +\t\t\t$href_params{'extra_options'} = undef;\n> >  \t\t\t$href_params{'action'} = $type;\n> >  \t\t\t$link_attr{'-href'} = href(%href_params);\n> >  \t\t\tprint \"<link \".\n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"188970","messageId":"201204111739.07765.jnareb@gmail.com","threadId":"30128","inReplyTo":"201204042058.32549.jnareb@gmail.com","subject":"Re: [PATCH (bugfix)] gitweb: Fix unintended \"--no-merges\" for regular Atom feed","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-04-11T15:39:06Z","receivedAt":"2012-04-11T15:39:06Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 4 April 2012, Jakub Narebski wrote:\n> Junio C Hamano wrote:\n> > Jakub Narebski <jnareb@gmail.com> writes:\n> > \n> > > Junio, the bug is very minor, so I don't know if it is worth fixing\n> > > for 1.7.10.\n> > \n> > Does this exist in 1.7.9.x maintenance track?  If it is an old bug, I do\n> > not think it should go to 1.7.10 proper (perhaps 1.7.10.1) this late, and\n> > if it is a bug in a new feature added for 1.7.10, we may want to fix it\n> > before the final, as the impact of the patch seems very minor.\n> \n> It is an old bug, from 05bb5a2 (gitweb: Include links to feeds in HTML\n> header only for '200 OK' response, 2010-12-18) which refactored feed link\n> generation into print_feed_meta().  It is in gitweb since v1.7.4 I think.\n> \n> So 1.7.10.1 it is...\n\nPing!\n\nI don't see this trivial fix (admittedly for obscure bug) in \"What's\ncooking\", and it is not present in 'master'.\n \n> > >  gitweb/gitweb.perl |    1 +\n> > >  1 files changed, 1 insertions(+), 0 deletions(-)\n\n-- \nJakub Narebski\nPoland\n"},{"id":"188992","messageId":"7vd37emcy7.fsf@alter.siamese.dyndns.org","threadId":"30128","inReplyTo":"201204111739.07765.jnareb@gmail.com","subject":"Re: [PATCH (bugfix)] gitweb: Fix unintended \"--no-merges\" for regular Atom feed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-11T16:48:48Z","receivedAt":"2012-04-11T16:48:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n>> So 1.7.10.1 it is...\n>\n> Ping!\n\nYou are part of my distributed memory and the system is nicely\nworking ;-)\n\n> I don't see this trivial fix (admittedly for obscure bug) in \"What's\n> cooking\", and it is not present in 'master'.\n\nOf course not.  Now you reminded me, it will.\n\nThanks.\n"}]}