Closed Bug 262967 Opened 20 years ago Closed 20 years ago

RSS feeds for revision histories

Categories

(Webtools Graveyard :: Bonsai, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: myk, Assigned: myk)

Details

(Whiteboard: [fixed on cls-bonsai-20041001-branch])

Attachments

(2 files, 1 obsolete file)

Bonsai should produce RSS feeds for CVS files where the items in the feed are
the revisions on the files.  Then users could track revisions to files by
subscribing to the the feeds in a feed reader.
Attached patch patch v1: implements feature (obsolete) — Splinter Review
Here's an implementation that hacks cvslog.cgi to generate an RSS version of
the page.  It bundles the code that generates the HTML version into a
"display_html" function (but doesn't indent it to make the patch easier to
read), puts the code for the RSS version into a "display_rss" function,
differentiates between them via the "ctype" URL parameter, adds a link from the
HTML to the RSS version, factors out some code for generating links to specific
revisions, and removes the unused "revision_pad" function.

It adds Date::Parse and Date::Format dependencies (both modules are available
from CPAN and come standard with Perl 5.8).  It generates valid RSS 1.0 per
http://feedvalidator.org/ and follows recommendations in the RSS 1.0 spec for
backwards compatibility with RSS 0.9x/2.0.
Attachment #161135 - Attachment is patch: false
Attachment #161135 - Attachment mime type: text/plain → application/rss+xml
Comment on attachment 161134 [details] [diff] [review]
patch v1: implements feature

Chris, you've been hacking Bonsai lately.  Can you review this patch?
Attachment #161134 - Flags: review?(cls)
See also bug 253245.
Attachment #161135 - Attachment mime type: application/rss+xml → text/plain
Comment on attachment 161134 [details] [diff] [review]
patch v1: implements feature

+	  <A
HREF="cvslog.cgi?file=$url_filename&root=$root&author=$author_arg&ctype=rss">RS
S</A>&nbsp;

that doesn't support branches
Attachment #161134 - Flags: review?(cls) → review-
Comment on attachment 161134 [details] [diff] [review]
patch v1: implements feature

>Index: cvslog.cgi

>+my $ctype = $::FORM{ctype} || "html";
>+if    ($ctype eq "html") { display_html(); }
>+elsif ($ctype eq "rss")  { display_rss(); }
>+else                     { display_html(); }
>+
>+## END of main script

Just a nit but can you put an 'exit;' here?

>+         <A HREF="cvslog.cgi?file=$url_filename&root=$root&author=$author_arg&ctype=rss">RSS</A>&nbsp;
>+        </TD><TD NOWRAP>

Like timeless pointed out, this seems to be an incomplete subset of the
possible cvslog.cgi options.  At a glance, the rev, mark & sort options are
missing.

>+        ."<A NAME=$revision><A HREF=".revision_link($revision).">$revision</A>"

The result of revision_link needs to be value_quoted.

>+    # The canonical URL of the file; we use a link to its revision history,
>+    # but arguably this should be a link to the LXR page or, for web pages,
>+    # a link to the actual web page.
>+    my $link = Param("urlbase") . "cvslog.cgi?file=$filename";

$filename should be url_encoded.

>+sub revision_link {
>+    my $revision = shift;
>+
>+    my $link = Param('urlbase') . "cvsview2.cgi";
>+    if (defined($::prev_revision{$revision})) {
>+        $link .= "?diff_mode=context&whitespace_mode=show&file=$url_file_tail"
>+                 . "&branch=$::opt_rev&root=$root&subdir=$rcs_path"
>+                 . "&command=DIFF_FRAMESET&rev1=$::prev_revision{$revision}"
>+                 . "&rev2=$revision";
>+    } else {
>+        $link .= "?files=$url_file_tail&root=$root&subdir=$rcs_path"
>+                 . "\&command=DIRECTORY\&rev2=$revision&branch=$::opt_rev";
>+        $link .= "&branch=$browse_revtag" unless $browse_revtag eq 'HEAD';
>+    }
>+    
>+    $link = url_encode3($link);

I know this code was just moved but what's the purpose of using url_encode3
here (or at all)?  The questionable args have already been url_encoded and the
entire result is usually value_quoted.

Any ideas on how bandwidth intensive these RSS feeds are?  We might want to
consider having an option to disable this functionality via admin.cgi.
>Just a nit but can you put an 'exit;' here?

Yup, done.


>Like timeless pointed out, this seems to be an incomplete subset of the
>possible cvslog.cgi options.  At a glance, the rev, mark & sort options
>are missing.

I've added "rev" so that branches are supported per timeless' comment.	"mark"
has no equivalent in RSS, which is after all a language for transmitting
structured data rather than marking it up for display.	"sort" seems similarly
unuseful, given that RSS readers tend to handle sorting themselves and the
order of items in an RSS file is generally considered to have either no
inherent meaning or be reverse chronological (which is how this patch orders
them).

Nonetheless, the RSS 1.0 spec does specify that items be listed in an RDF Seq
rather than an RDF Bag "to denote item order for rendering and reconstruction,"
and I'm loathe to take away options from users, so I've added back support for
sorting by author (the only kind of sort we do besides reverse date/revision).


>The result of revision_link needs to be value_quoted.

Done.


>$filename should be url_encoded.

Done.


>I know this code was just moved but what's the purpose of using url_encode3
>here (or at all)?

Good question.	It's only used in one other place, and it doesn't look so
useful there either, so I've removed the function and calls to it.


>Any ideas on how bandwidth intensive these RSS feeds are?  We might want to
>consider having an option to disable this functionality via admin.cgi.

RSS readers vary, but most reload feeds no more often than once every half hour
or hour, and RSS content is more compact than its HTML equivalent, so I don't
think this feature adds significant bandwidth or server load.

Additional changes in this version of the patch:
  * changed "revision n" -> "version n" in RSS item titles;
  * url_quote()d author_arg before using it in URLs;
  * added correct content type for RSS (application/rss+xml);
  * simplified logic for choosing output type; we don't have
    to check for ctype=html explicitly since it's the default;
Attachment #161134 - Attachment is obsolete: true
Attachment #161320 - Flags: review?(cls)
Attachment #161320 - Flags: review?(cls) → review+
Whiteboard: [fixed on cls-bonsai-20041001-branch]
Thanks Chris.

Checking in CGI.pl;
/cvsroot/mozilla/webtools/bonsai/CGI.pl,v  <--  CGI.pl
new revision: 1.18; previous revision: 1.17
done
Checking in cvslog.cgi;
/cvsroot/mozilla/webtools/bonsai/cvslog.cgi,v  <--  cvslog.cgi
new revision: 1.26; previous revision: 1.25
done
Checking in lloydcgi.pl;
/cvsroot/mozilla/webtools/bonsai/lloydcgi.pl,v  <--  lloydcgi.pl
new revision: 1.4; previous revision: 1.3
done
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Product: Webtools → Webtools Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: