WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#9004 closed defect (bug) (fixed)

More filtering in get_terms() and wp_generate_tag_cloud()

Reported by: jhodgdon Owned by: ryan
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.7
Component: Taxonomy Keywords: taxonomy, filter, has-patch
Focuses: Cc:

Description

I maintain a plugin that enables WordPress to switch languages at the click of a link.

One issue that my users have is that lists of categories and tags that are supposed to be sorted by name end up not in alphabetical order if you switch languages. The reason for this is that the term name field contains something like this:

[lang_en]book[/lang_en][lang_es]libro[/lang_es]

(the language mumbo-jumbo and the non-current language info is filtered out upon display). While this tag would belong in letter "B" (book) if you are reading in English, if you switch to Spanish, it should be in "L" (libro), and obviously sorting by the term name field is not going to get that right.

I looked into resolving this issue, and I was able to fix the sorting by adding a couple of new filters in get_terms() in wp-includes/taxonomy.php, and one in wp_generate_tag_cloud() in wp-includes/category-template.php (see patch I am about to add to this ticket).

I've tested the added filters patch against both 2.7 (I added the new filter lines manually, actually, not sure if the patch will apply) and 2.8-bleeding [10469] (the patch is against this version). I don't think it would do / could do / does anything bad, and it did allow me to add a few lines to my plugin and get everything sorted correctly. There's no straightforward way to get the sort working without this patch, aside from completely copying those two functions (and many others that depend on them) into the plugin, which would be a royal pain and difficult to maintain as well.

So I'd appreciate it if you could add this in 2.7.1, or barring that, 2.8 would be OK. I even added to the doc headers. :)

Attachments (2)

term-sort.diff (2.2 KB) - added by jhodgdon 5 years ago.
Patch that adds filters for custom term sorting (patch is relative to the wp-includes directory)
get_terms_fields_as_array.9004.diff (2.6 KB) - added by filosofo 5 years ago.

Download all attachments as: .zip

Change History (12)

jhodgdon5 years ago

Patch that adds filters for custom term sorting (patch is relative to the wp-includes directory)

comment:1 FFEMTcJ5 years ago

  • Milestone changed from 2.7.2 to 2.8

comment:2 jhodgdon5 years ago

  • Milestone changed from 2.8 to 2.7.2

Why up the milestone, and why not just commit this patch?

This is a simple request to add some filters to a couple of functions that lack them. The change affects 3 lines of code (plus the comment lines).

Can you please put it in 2.7.2 as well as 2.8?

comment:3 FFEMTcJ5 years ago

  • Milestone changed from 2.7.2 to 2.8

Since it's not a bug, it probably will not make it into 2.7.x. That will be up to the devs, who can look into it when they commit, but I'm moving it back to 2.8 where it belongs.

comment:4 jhodgdon5 years ago

This patch is so simple (adds 3 filtering lines of code), and I have tested it... please committ...

comment:5 jhodgdon5 years ago

  • Type changed from enhancement to defect (bug)

I'm changing this to a bug rather than enhancement. For my plugin's users, it's a bug that there is no way for a filter to change the sorting in get_terms and wp_generate_tag_cloud.

Please can you commit this patch? Thanks...

comment:6 filosofo5 years ago

I like this, but I think it would be better to pass an array to get_terms_fields, as in my version of the patch.

comment:7 follow-up: jhodgdon5 years ago

I'm not sure why it's better to pass an array, really... If you are worried about a particular string being duplicated, in your filter implementation, you could do a regexp match to make sure it isn't already there.

Anyway, I don't feel too strongly one way or the other, and if your version gets committed, I should be able to do a test in my current filter implementation to see if someone is using 2.7 with my version of the patch (I currently distribute it with my plugin, in case someone wants their categories/tags to be sorted correctly), or 2.8 with your version, by checking to see if the input is an array or a string.

That said, I haven't tested the patch from filosofo yet. It does change how the core WP function get_terms() generates its query, so it should definitely be tested to make sure all the get_terms stuff still works (even without any filters actually defined by plugins). The patch I originally submitted doesn't make any changes to the core function's query generation.

comment:8 in reply to: ↑ 7 filosofo5 years ago

Replying to jhodgdon:

I'm not sure why it's better to pass an array, really...

The PHP array functions are quicker and potentially more robust in filtering the fields than would be a regex replace. Most people using that filter would probably split the string into an array, do their business, and join it back together as a string, anyways. Other comma-delimited things-to-be-filtered are handled similarly throughout WP. I'm just suggesting that if we're going to introduce a new filter it should be consistent with other filters and convenient for developers, but yeah, it doesn't really matter that much.

comment:9 jhodgdon5 years ago

I see your point, but I'm not sure if most plugins will need to convert to an array and re-join.

The one example use so far is in my language plugin, where I just added a new field at the end of the string (knowing there was no way the core function would ever have my specific calculated field, so no duplication is possible). I don't really think plugins would want to remove fields, but mostly just add fields, as removing fields would break get_terms() most likely (it's used all over the place, and all 4 of its fields are probably necessary to have present).

My plugin may be the only user of this filter anyway. :)

All of that said, either version is fine with me.

comment:10 ryan5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11037]) get_terms_orderby, get_terms_fields, and tag_clooud-sort filters. Props jhodgdon, filosofo. fixes #9004

Note: See TracTickets for help on using tickets.