Integration with the Token module

Cool, thanks for getting this started -- seems like a good thing overall. A few problems I spotted from a very quick skim of the patch:

A) $tokens[t('['. $pattern .']')] = t($description);
That's now how t() works. You can't t() a variable, only string literals.

B) "avaialable" is misspelled.

C) I don't like having the duplicate code to get the token help text HTML. At the very least, that could be generated in a private helper function and shared. It's kinda silly that token doesn't provide this itself, to be honest.

I'll look more closely when you post a patch that fixes these and I have time to actually test it.

Thanks again!
-Derek

I gave this a close review and some testing. I found some problems, all of which I've fixed with this patch. Please test it out and see what you think. For the record, here's what I did:

A) Token.module expects tokens in your message to be wrapped in [], such as [nid], so the help text needs to show that.

B) Even with the private method to create the help text HTML, the previous patch was still duplicating a fair bit of code to create the fieldsets. So, I added _signup_token_help() which takes a $form array by reference and a form element name (for the fieldset) as args, and adds the fieldset and help text.

C) I made _signup_build_token_help() statically cache its results in cases where it's called twice on the same page load.

D) The way you were just conditionally appending "Additional tokens provided by the token module are available for substitution." would make it impossible to translate this module into an RTL language. So, I reworked that stuff in a way that I think will still be RTL-friendly. It's a wee bit unwieldy, but I think it's the best we can do. ;) I also think it's nicer not to mention "token.module" in user facing text like this, so I think the end result is cleaner for users.

E) The call to token_replace() was wrong for the confirmation emails, since you were passing in $event->nid even though $event didn't exist. In fact, we've already got a fully loaded $node object in that case, so we just use that instead of calling node_load() again (even though it's cached, it's still wasteful).

F) Ditto the token_replace() call for the signup_broadcast thing -- we've already got a fully loaded node in $event, so we use that directly.

This is going to be very slick, and will solve lots of problems, such as #117203: Adding CCK fields to reminder email, so I can't wait to get this in.

Thanks!
-Derek

Note: until token.module provides a token for the URL of a node (see #127613-6: Provide tokens for node url and path (i.e. absolute and relative alias respectively)) we'll need that ourselves: #265222-3: Event URL in email with %eventurl string.

I'd really like to get this in ASAP, so I'm bumping the prio...

Tested with and without token module enabled. Works great!

Great, committed to HEAD. Thanks, everyone. Yay.

Oh, I was late :-( But, it works fine here anyway :-D

Actually, let's leave this as just "fixed" for now, so it's still visible in the signup issue queue for others who are looking. It'll get automatically closed in 2 weeks. But thanks for the other testing.