The filters table is kind of unique that of the four columns in the table three of them are associated with the primary key leaving only one column that gets updated - the weight. As a result it seems redundant to add another function just to change the one column, so install_add_filter should be changed to update records if they already exist.

CommentFileSizeAuthor
#10 install_profile_api-n412042-filters-API_fix.patch615 bytesJames Andres
#1 install_profile_api-n412042-filters.patch1.28 KBdamienmckenna

Comments

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new install_profile_api-n412042-filters.patch1.28 KB

Patch attached that renames install_add_filter to install_set_filter.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Active » Needs review

Can has feedback? ^_^

James Andres’s picture

James Andres commented
Status: Needs review » Fixed

I agree, this is more useful. Committed.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Ack! In the future, can you please leave legacy wrappers around this stuff? 2.0 is a stable release. Sudden API changes are Not Nice. :(

James Andres’s picture

James Andres commented

Sorry, I wasn't aware 2.0 was tagged for release. Is there a discussion thread somewhere I should be following to keep informed?

Sorry,

James.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

James: What webchick means is that we should have left install_add_filter() intact so that the v6.x-2.x releases didn't loose any functions, i.e keeping its API stable.

James Andres’s picture

James Andres commented

No worries, should I roll it back?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Fixed » Active

No, I don't think you need to roll it back. Just maybe do something like:

/**
 * Add a filter to an input format. Note: This function is deprecated.
 *
 * @see install_set_filter()
 */
function install_add_filter($format_id, $module, $delta = 0, $weight = 0) {
  install_set_filter($format_id, $module, $delta, $weight);
}
James Andres’s picture

James Andres commented

Wow, I'm a moron. I thought I gave that patch a thorough inspection before committing, but I didn't notice the function name change.

Yes, I definitely see your point now. I'll re-roll in a non-API-breakage way.

Ooops,

James.

James Andres’s picture

James Andres commented
StatusFileSize
new install_profile_api-n412042-filters-API_fix.patch615 bytes

Here is the patch with the now deprecated install_add_filter($format_id, $module, $delta = 0, $weight = 0) included.

To expedite the fix for those dealing with a broken install_profile_api, I've committed this.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Yay! Thanks, James. :) And no worries. I'm fine with occasional wrinkles as long as this module's getting some committer love. :D

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Active » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.