We're listing tokens by doing theme('flag_token_help'). Henceforth I'll call this feature "tokens browser".

However, our tokens browser is broken for D7: it lists all tokens and not just the ones we ask it to.

From what I've read I gather that there won't be a tokens browser in D7's core.

There are three ways to solve our problem:

  1. Fix our tokens browser (I've actually done that already).
  2. Use the tokens browser of the Token module. It has a nifty new browser. If Token isn't installed, we'll tell the user he has to install it if he wants to browse the tokens.
  3. Fix our token browser and use it, but if the Token module is installed use its instead.

(Why is this issue important? When #871064: Making flaggings fieldable is in, it will be possibe to embed field tokens in messages, so we should provide a browser.)

CommentFileSizeAuthor
#3 fixtokens6.diff4.61 KBmooffie
#3 fixtokens7.diff8.15 KBmooffie

Comments

mooffie’s picture

mooffie commented
Assigned: Unassigned » mooffie

Nate,

Among the three possibilities, what's your preference?

(Well, possibility #1 can be ignored, so there's only #2 and #3.)

quicksketch’s picture

quicksketch
he/him
commented

#2 is my preference, considering this is the most likely approach for other modules to take as well.

mooffie’s picture

mooffie commented
Status: Active » Needs review
StatusFileSize
new fixtokens7.diff8.15 KB
new fixtokens6.diff4.61 KB

Here's the patch.

I ported my style changes back to D6, so there are two patches.

Changes:

  • Removed module_exists('token') calls.
  • Renamed 'theme_flag_token_help' to 'theme_flag_tokens_browser'.
  • The help message ("The above six options may contain the following wildcard [...]") had some odd things in it, so I revised it. I also formatted the examples as a list (because it was getting too long: I added an example, and when 'fieldable flaggings' is in I may have another one or two).

(Drupal's cache must be cleared after applying the patch.)

mooffie’s picture

mooffie commented

Nate, are you ok with this patch?

(I did consider porting it to the D61 branch as well, thinking that unless I do this the rename from 'theme_flag_token_help' to 'theme_flag_tokens_browser' might get out of sync code that is shared between D61 and D62, but it turns out D61 and D62 don't share such code. Additionally, D61 doesn't have flag-admin.css, which is needed here.)

quicksketch’s picture

quicksketch
he/him
commented

Looks fine to me. We don't need to backport any changes to the 1.x version of Flag. 2.x versions should be feature-equivalent (D6 and D7) and the 1.x versions should be feature-equivalent (D5 and D6). We're not adding any changes to 1.x at this point.

mooffie’s picture

mooffie commented
Status: Needs review » Fixed

Committed.
http://drupal.org/cvs?commit=408354
http://drupal.org/cvs?commit=408342

Status: Fixed » Closed (fixed)

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