WordPress.org

Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#12130 closed enhancement (wontfix)

Add Donate URI plugin header

Reported by: Otto42 Owned by: westi
Milestone: Priority: low
Severity: trivial Version:
Component: Plugins Keywords: has-patch
Focuses: Cc:

Description

Add support for a new "Donate URI:" field in the plugin header, which adds a "Donate" link in the plugin_row_meta area on the Manage Plugins page.

We already have a similar field in the readme.txt, for adding a donate link to the wp.org Extend area, which creates a "Donate to this plugin" link there.

Many plugins already hook into and add this themselves. It'd be quicker and more efficient to allow for this as a default header, adding the link automatically. Plus this way it'd get translated in the normal WordPress translations, instead of being in English most of the time on international versions.

Patch attached.

Attachments (5)

12130.diff (2.3 KB) - added by Otto42 12 years ago.
Updated patch for trunk
12130-read-from-readme.txt (2.3 KB) - added by westi 12 years ago.
Read in the donate uri from readme.txt and return is as part of plugin data
12130.2.diff (2.6 KB) - added by Denis-de-Bernardy 12 years ago.
best of both worlds.
12130.3.diff (3.9 KB) - added by Denis-de-Bernardy 12 years ago.
12130.4.diff (4.0 KB) - added by Denis-de-Bernardy 12 years ago.

Download all attachments as: .zip

Change History (53)

#1 @Otto42
12 years ago

  • Keywords has-patch added

#2 @ryan
12 years ago

Sounds good to me.

#3 @Gautam Gupta
12 years ago

  • Cc Gautam Gupta added
  • Milestone changed from Unassigned to 3.0

Nice idea. This would also motivate the people to donate more to the plugin authors (as they develop plugins free of charge). I think this should get into 3.0.

#4 @ozh
12 years ago

  • Cc ozh@… added

#5 @Alphawolf
12 years ago

  • Cc Alphawolf added

#6 @nacin
12 years ago

Patch would need the header added to _get_plugin_data_markup_translate()

#7 follow-up: @westi
12 years ago

  • Keywords Needs-patch added; has-patch removed
  • Status changed from new to accepted

Please don't add this as a plugin header

we should just read it from the readme.txt and merge the data using the file parsing code we have in trunk for the header lines

that way all existing plugins benifit from this and people don't have to specify in two places

#8 in reply to: ↑ 7 ; follow-up: @Otto42
12 years ago

  • Keywords has-patch added; Needs-patch removed

Replying to westi:

Please don't add this as a plugin header

we should just read it from the readme.txt and merge the data using the file parsing code we have in trunk for the header lines

that way all existing plugins benifit from this and people don't have to specify in two places

No, this needs to be separate, because not all users want those two different spaces to contain the same link.

Furthermore, some people may not want that link to appear on the Plugins Management page, but do want the link to appear in the Extend section (I myself fall into this category with a few of my plugins). Therefore it must be separate.

#9 @nacin
12 years ago

  • Keywords needs-patch added; has-patch removed

Patch would need the header added to _get_plugin_data_markup_translate()

Either way, still needs a patch.

@Otto42
12 years ago

Updated patch for trunk

#10 @Otto42
12 years ago

  • Keywords has-patch added; needs-patch removed

Patch updated to include addition to _get_plugin_data_markup_translate() function.

#11 @bueltge
12 years ago

  • Cc frank@… added

#12 in reply to: ↑ 8 @westi
12 years ago

  • Keywords reporter-feedback added

Replying to Otto42:

Replying to westi:

> > Please don't add this as a plugin header
> >
> > we should just read it from the readme.txt and merge the data using the file parsing code we have in trunk for the header lines
> >
> > that way all existing plugins benifit from this and people don't have to specify in two places
>
> No, this needs to be separate, because not all users want those two different spaces to contain the same link.
>
> Furthermore, some people may not want that link to appear on the Plugins Management page, but do want the link to appear in the Extend section (I myself fall into this category with a few of my plugins). Therefore it must be separate.

Can you explain the why of this in more detail.

Adding an extra header means all the plugin authors with an existing donate link which appears in extend have to do something extra (including releasing a new version of there plugin) just to take advantage of having it displayed.

I would rather we brought the behaviour of Extend and the plugin page closer together and moved closer to using the same data for both of them.

What are the reasons why you don't want the same behaviour for both?

#13 @chipbennett
12 years ago

  • Cc chip@… added

I strongly disagree with the assertion that plugin authors must provide a Donate link in two separate plugin files.

I believe Otto represents an extreme edge case, in which a plugin author only wants to display a donate link on either Extend or the Manage Plugins page (but not both). Further, I believe that the vast majority of plugin authors who provide a Donate link a) want to provide only one Donate link in only one plugin file (not in two separate files), and b) want their Donate link to appear on both Extend and the Manage Plugins page.

#14 @otto42
12 years ago

Actually, I'd prefer seperate urls for tracking purposes, so I can see where the most donations are coming from, and through which channels.

#15 @chipbennett
12 years ago

I think that's a legitimate need; however, tracking Donate link click-throughs shouldn't necessarily require separate URLs.

#16 follow-up: @Ozh
12 years ago

Replying to chipbennett:

I strongly disagree with the assertion that plugin authors must provide a Donate link in two separate plugin files.

They *must* not. They just *can*.

I think the donate link right from the Manage Plugin page would be a hugely cool thing (not to mention that would also please people not using the repository). (For the sake of consistency and fairness, a similar link could be added to Themes too)

#17 in reply to: ↑ 16 @chipbennett
12 years ago

Replying to Ozh:

Replying to chipbennett:

I strongly disagree with the assertion that plugin authors must provide a Donate link in two separate plugin files.

They *must* not. They just *can*.

I think the donate link right from the Manage Plugin page would be a hugely cool thing (not to mention that would also please people not using the repository). (For the sake of consistency and fairness, a similar link could be added to Themes too)

I should re-phrase: they *must* provide the link in two separate plugin files, *if* they want the link to appear both on Extend and on the Manage Plugins page (if this is implemented in the manner that Otto suggests).

I believe it would be far more useful for plugin authors to be able to provide the link in *one* file, and have the link appear in both places.

Perhaps a good compromise would have Extend look at the readme.txt file first for a Donate link, and if it doesn't find one, to look in the Plugin header?

I think that would cover all use cases:

Plugin authors who want to provide one link and have it appear on both Extend and Manage Plugins puts Donate Link in the plugin header.

Plugin authors who want separate links for Extend and Manage Plugins can put separate links in the Plugin header and in readme.txt.

Plugin authors who want a link to appear on Extend but not on Manage Plugins can put a link in readme.txt, and nothing in the Plugin header.

#18 follow-ups: @otto42
12 years ago

Why so complicated, and for so little benefit? I mean, all that just to avoid putting one extra line in a file?

The current code in core does not look at the readme file at all. Adding a parsing mechanism just to avoid having an extra line is overkill.

Plus, having a separate link makes more sense for tracking reasons as well as for ppl who may not want links in certain locations. It's the simplest and most straightforward approach. A whole complicated set of rules is not necessary.

#19 @otto42
12 years ago

Also, not every plugin in the repo is "singular". My SFC plugin is one entry, with one readme, but contains 11 different plugins in the same directory. Twitter Tools does something similar. Each separate plugin could have it's own link with the header approach, which some authors might want.

#20 in reply to: ↑ 18 @chipbennett
12 years ago

Replying to otto42:

Why so complicated, and for so little benefit? I mean, all that just to avoid putting one extra line in a file?

The current code in core does not look at the readme file at all. Adding a parsing mechanism just to avoid having an extra line is overkill.

Plus, having a separate link makes more sense for tracking reasons as well as for ppl who may not want links in certain locations. It's the simplest and most straightforward approach. A whole complicated set of rules is not necessary.

hat rules, and how is it complicated?

The vast majority of plugin authors would add PluginURI to their plugin header and be done with it.

For your two use cases, you can a) add a separate Donate link to readme.txt, or b) add Donate link to readme.txt and leave the plugin header PluginURI undefined.

Extend does all the rest, by looking at readme.txt and/or the plugin header.

Of course, I'm with Westi's original suggestion: just parse readme.txt for the existing Donate link. I'm just trying to suggest the most simple compromise, that will be easiest/most convenient for the vast majority of plugin authors.

#21 @strider72
12 years ago

I'm against this. While a donate link on the Extend page makes sense, are we really* going to clutter up the Manage Plugins screen with a donate link for each plugin? Some authors may prefer it on the Settings screen. Some not at all.

Authors can readily add such a link right now with one or two lines of code.

-1

#22 in reply to: ↑ 18 @westi
12 years ago

Replying to otto42:

Why so complicated, and for so little benefit? I mean, all that just to avoid putting one extra line in a file?

The current code in core does not look at the readme file at all. Adding a parsing mechanism just to avoid having an extra line is overkill.

We don't need a new parsing mechanism.

It is really easy to read in from the readme.txt patch incoming.

@westi
12 years ago

Read in the donate uri from readme.txt and return is as part of plugin data

#23 follow-up: @Otto42
12 years ago

westi: That method doesn't allow for people to have a different link on Extend vs. on the Manage Plugins page.

My implementation is simpler, faster, and more flexible for plugin authors.

#24 in reply to: ↑ 23 @chipbennett
12 years ago

Replying to Otto42:

westi: That method doesn't allow for people to have a different link on Extend vs. on the Manage Plugins page.

My implementation is simpler, faster, and more flexible for plugin authors.

But Westi's method benefits the most plugin authors with the least effort (i.e. plugin authors who currently have a defined Donate Link in their readme.txt don't have to do anything at all to take advantage of the plugin_row_meta Donate link).

Wouldn't your use case be a perfect example of modifying plugin_row_meta DonateURI via the plugin itself, if the author wants different links in Extend and Manage Plugins?

#25 follow-up: @Otto42
12 years ago

Chip, I have 60+ plugins on my site. That's 60+ extra files you want the thing to read in and parse. At a max of 8k each, that's 480k that it now has to go through.

I don't think the overhead of reading an extra file for every plugin is worth it just to eliminate the need for plugin authors to add one single little line.

Lots of people use lots of plugins, and adding all sorts of extra overhead for something as silly as one line in the plugin header doesn't make any sense to me.

Plugin authors are not idiots, and they can add a line to their own plugins to give a donate link if they want to do so. You don't have to treat them as if they are children.

#26 @Ozh
12 years ago

  • Cc ozh@… removed

#27 in reply to: ↑ 25 @chipbennett
12 years ago

Replying to Otto42:

Chip, I have 60+ plugins on my site. That's 60+ extra files you want the thing to read in and parse. At a max of 8k each, that's 480k that it now has to go through.

I don't think the overhead of reading an extra file for every plugin is worth it just to eliminate the need for plugin authors to add one single little line.

Lots of people use lots of plugins, and adding all sorts of extra overhead for something as silly as one line in the plugin header doesn't make any sense to me.

Plugin authors are not idiots, and they can add a line to their own plugins to give a donate link if they want to do so. You don't have to treat them as if they are children.

The reality is that the vast majority of plugin authors would neither need nor want to have two separate Donate links. Your use case is the extremely minor edge case - the exception to the rule.

Westi's method benefits the most plugin authors, with the least amount of effort.

#28 @otto42
12 years ago

I would actually rather not have the feature in core at all instead of using that approach. Because not only will that method slow down every site by unneccsarily reading extra files, but it also forces plugin authors to add code to their plugins in order to undo the misfeature, and correct the broken URL.

#29 @nacin
12 years ago

Is there any agreement on this?

#30 @Otto42
12 years ago

  • Keywords has-patch reporter-feedback removed
  • Milestone 3.0 deleted
  • Resolution set to wontfix
  • Status changed from accepted to closed

#31 follow-up: @nacin
12 years ago

  • Milestone set to 3.1
  • Resolution wontfix deleted
  • Status changed from closed to reopened

There's no consensus here on how to do it, but we all agreed on finding an easy way for 'Donate' links to get into core.

#32 in reply to: ↑ 31 @Otto42
12 years ago

  • Keywords has-patch added
  • Milestone changed from 3.1 to Future Release

Replying to nacin:

There's no consensus here on how to do it, but we all agreed on finding an easy way for 'Donate' links to get into core.

I've already given the patch to put the Donate links into core. Twice. I closed as wontfix because that seems to be the only option remaining.

The alternative being proposed for reading readme.txt files is simply unacceptable. It adds too much overhead by reading tons of extra files, and it is far less flexible and individually configurable on a per plugin basis. Like I said before, I'd rather not have it in core than do it that way.

And if there's no consensus, then this shouldn't be marked for 3.1.

#33 @Denis-de-Bernardy
12 years ago

  • Milestone changed from Future Release to 3.0

I'd like to see this checked into 3.0. Could we please all agree on the following?

  1. If we find a donate header in the plugin, we use it (including if it's purposely left blank)
  2. If not, we use the one for one from the readme file?

@Denis-de-Bernardy
12 years ago

best of both worlds.

#34 @Denis-de-Bernardy
12 years ago

  • Keywords commit added

#35 @Denis-de-Bernardy
12 years ago

the modified patch, which is based on Westi's allows to set a donate link in the plugin, fallbacks to the readme file's link if none is set.

the current api does not reliably allow to prevent any link from appearing. setting the link to 0 will make WP think it's empty, but it shouldn't be relied upon in the long term.

#36 @nacin
12 years ago

This is more or less what I assumed would be checked in eventually. Though, I know Otto had performance concerns on parsing the readme.txt files of possibly dozens of plugins, which seems potentially valid.

#37 @Otto42
12 years ago

Still do not want. I don't want this thing scanning all my readme's for no reason.

Add some way to disable the readme.txt scanning portion of it. A filter or something I can add to turn this heavy performance drag right the heck back off. Having my system read in an extra half a meg just to add a few links is just straight stupid.

Or better yet, forget trying to parse the readme in the first place. There's no point in babysitting plugin authors, if they want a donate link, then can quite easily add it to the plugin themselves.

#38 follow-up: @Denis-de-Bernardy
12 years ago

k... I've massaged it a bit more. in 12130.4.diff:

  • added an extra argument to get_file_data(), allowing to set the max amount of file read (1kb is probably way more than needed, and the same holds for themes)
  • there's a hook in get_file_data that allows to override the size. namely to zero, in which case it bails and returns the default headers.

can we say everyone is now happy with the approach? if so, @otto - be so kind to give it a whirl, including the parts you don't like/agree with.

#39 @TobiasBg
12 years ago

I agree with Otto, that this would just add overhead.

There are enough support threads about exhausted memory limits already. And scanning a whole lot of readme files doesn't exactly help those.

This is just not worth the gain, especially as the same is easily possible from within the plugin - and that with even more control about the link for the plugin developer.

If at all, this should be punted to 3.1 for further discussion, especially in relation to the possible UI changes.

#40 in reply to: ↑ 38 @Otto42
12 years ago

  • Keywords commit removed
  • Milestone changed from 3.0 to Future Release

Replying to Denis-de-Bernardy:

can we say everyone is now happy with the approach? if so, @otto - be so kind to give it a whirl, including the parts you don't like/agree with.

Don't like:

  • No off switch
  • Still reading my readme.txt files, eating memory for no good reason, and slowing my site down to a crawl

Any solution that involves making the code read extra files is a solution I can not agree with under any circumstances. Remove the readme.txt file reading code entirely, that's my vote.

#41 @mikeschinkel
12 years ago

  • Cc mikeschinkel@… added

#42 follow-up: @Denis-de-Bernardy
12 years ago

@Otto: I hope you realize that you've just sent this ticket to the rot grubs... I'm willing to take the bet that it won't get fixed before a few years now...

#43 in reply to: ↑ 42 @Otto42
12 years ago

Replying to Denis-de-Bernardy:

@Otto: I hope you realize that you've just sent this ticket to the rot grubs... I'm willing to take the bet that it won't get fixed before a few years now...

Good. As I stated before, I'd rather there be no patch than to have yours or westi's in the core.

I've already posted my patch and stated that I think it should have been in there. I disagree with your patches and think they should not be in there.

It's really as simple as that. If we can't come to a consensus, then I think that it's right and proper that the ticket should rot and no changes should be made. That's exactly how strongly I feel about this one.

#44 @chipbennett
12 years ago

That's fine.

If this isn't in core by the time 3.0 hits, then I'll just release the plugin that does it anyway. I've already got it written; I've just been sitting on it, pending the outcome of this ticket.

#45 @Denis-de-Bernardy
12 years ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

might as well close it then. considering the lack of core dev interest, just go ahead and release it.

#46 follow-up: @Otto42
12 years ago

Fine.

And BTW, I'm rather annoyed at you people usurping this ticket, which was quite clear in the implementation, into this line of conversation. If you wanted to implement something else entirely that was not in the original ticket, then you should have created your own ticket for that change instead of blocking my change.

#47 in reply to: ↑ 46 @westi
12 years ago

Replying to Otto42:

Fine.

And BTW, I'm rather annoyed at you people usurping this ticket, which was quite clear in the implementation, into this line of conversation. If you wanted to implement something else entirely that was not in the original ticket, then you should have created your own ticket for that change instead of blocking my change.

Tickets should be about problems not solutions.

The discussion on the ticket has resolved around the best solution to the problem - "Reliable and easy access to the Donate URI for my plugin"

#48 @hakre
11 years ago

Related: #13699

Note: See TracTickets for help on using tickets.