Comments

jhodgdon’s picture

Issue tags: +Novice, +Needs backport to D7

OK, that seems like a reasonable idea. That would be a good project for a novice contributor. It just needs to use @see to link to the "themeable" group. See http://drupal.org/node/1354 (links section).

joachim’s picture

Adding that I think it would be a good idea to put this link fairly high up in the (long and pretty technical in parts) docs for this function. Catch the less technical users before they get scared! :)

jhodgdon’s picture

Our standards call for putting @see links at the bottom of function doc.

joachim’s picture

Ah. In which case, could this be linked in the body of the documentation, as a descriptive sentence?

Eg, 'The theme function is used to allow [LINK]themeable functions[LINK] to be overridden in a site theme.'

Or even use one that's already there:

All requests for themed output must go through this function. It examines the request and routes it to the appropriate [LINK]theme function[LINK] or template, by checking the theme registry.

jhodgdon’s picture

Yes, that could be done. Sounds like a good idea to turn that sentence that is already there into a link. I think it would still also be beneficial to have the @see at the end.

drupal_was_my_past’s picture

Assigned: Unassigned » drupal_was_my_past
Status: Active » Needs review
StatusFileSize
new966 bytes

Patch attached based on #5.

jhodgdon’s picture

Status: Needs review » Needs work

That's the right idea!

+ * the request and routes it to the appropriate @link themeable theme function
+ * @endlink or template, by checking the theme registry.

I think:
- we usually like to put @link ... @endlink all on one line (you can put it all on its own line so it will not go over the 80-character limit ) - reason being that the API module doesn't do well with multi-line @links sometimes (or at least it used to not do well, maybe that has been fixed).
- The "or template" can be part of the link text, since the themeable group is used for both.

drupal_was_my_past’s picture

Status: Needs work » Needs review
StatusFileSize
new970 bytes

Thanks! Here's the re-rolled patch.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Excellent, thanks!

catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Makes sense, took me a couple of reads to realise @link themeable blah blah @endlink automatically links to the themeable group, but that's just me I think. Committed/pushed to 8.x, moving back to 7.x.

drupal_was_my_past’s picture

Status: Patch (to be ported) » Needs review
StatusFileSize
new950 bytes

7.x ported patch attached for review.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Nope, it's me too. ;) But we can't exactly comment our comments. :D

Committed and pushed to 7.x. Thanks!

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