Modified REMOTERSS plugin

Creating and modifying plugins.
Post Reply
bahwi
Regular
Posts: 9
Joined: Wed Aug 11, 2004 10:55 pm

Modified REMOTERSS plugin

Post by bahwi » Wed Aug 11, 2004 11:01 pm

Hey, I just upgraded to the most recent CVS(Today, Aug. 11th) and had trouble getting the db updates to work(I'm postgresql). Got those working though and I think everything is great! I added some new features to the remoterss plugin, thought you might want to incorporate it into the main CVS. I added the option to display the date at all and to allow you to add a bullet image in front of each headline. You can see it in action at my blog at http://ynko.com/.

I've created a (hopefully-good) patchfile at http://josephguhlin.com/patch.p1. I tried to stay with naming conventions and tabs, but I don't use vim so I'm not sure if everything is correct. Feedback appreciated!

Thanks for the great program if you take these mods or not! [/url]

User avatar
garvinhicking
Core Developer
Posts: 30020
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

Re: Modified REMOTERSS plugin

Post by garvinhicking » Thu Aug 12, 2004 10:33 am

Hi Joey!

Sadly we have very little active postgreSQL developers. And I don't have a postgresql installation available at all. :(

So if you could maye give exact error message, we could try to fix it? Or if you already applied changes, you could tell those to us, so we will commit them.

But about your patch: First thing is that you seem to have appended your patches to a single file; there are at least 3 patches of earlier revisions inside that. Please create a clean file with only the latest patch. And I'm afraid the indenting doesn't seem right; we use 4 spaces (no Tabs!) for each indenting step, maybe you can change that? If you look at the patch file you created, you'll see that the lines are misaligned by one space...(at least in my mozilla browser)

And then, there's one line which disturbs me:

+ if (empty($item['title'])) {
+ ++$i;
+ continue;
+ }

Why do you increase the counter, if that item shouldn't be displayed?

I will commit your patch, when this is clear to me - thanks for the very useful improvements!

Regards,
Garvin
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/

bahwi
Regular
Posts: 9
Joined: Wed Aug 11, 2004 10:55 pm

Ok, _finally_ updated. =)

Post by bahwi » Fri Aug 20, 2004 7:37 am

Ok, finally got around to fixing the patch up. Sorry, work has been crazy lately!

Take a look at it here:
http://josephguhlin.com/s9y/remoterss.patch

It is still all three combined:
* display date toggle
* bullet image
* skip headlines that are blank(I've had this happen on a few I've tested)

I made the fix you suggested(not incrementing the counter if you're not displaying anything). I've learned the system a lot. Just to be sure I picked out the latest cvs file off the site: s9y_200408191337.tar.gz so it should be exact if not close.

If you want I can split these into three patches, but they change very similar parts of the file and can not be done in succession. Let me know if these work or if you need any other changes. I'm more than happy to oblige!

Thanks,
--Joey

User avatar
garvinhicking
Core Developer
Posts: 30020
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

Re: Ok, _finally_ updated. =)

Post by garvinhicking » Fri Aug 20, 2004 9:44 am

Hi Joey!

Thanks a lot, I just committed that to CVS!

Regards,
Garvin
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/

Post Reply