This issue has novice tasks. If you are an experienced core developer and have multiple commit mentions, please review novices' work on these tasks rather than doing them yourself. Feedback from experienced contributors is valued.

Problem/Motivation

Preprocess function documentation standards do not state that @ingroup themeable should be included in preprocess function docblocks. That group should only be included for functions or templates that actually produce markup and can be overridden in a theme.

A number of Twig conversions got in that converted a theme_ function to a template_preprocess_ function and left in the @ingroup themeable from the theme function.

This is messing up the listing on https://api.drupal.org/api/drupal/core%21modules%21system%21theme.api.ph... which should only show theme functions and template files.

Proposed resolution

Remove @ingroup themeable from all preprocess function docblocks.

Remaining tasks

User interface changes

n/a

API changes

n/a

Files: 
CommentFileSizeAuthor
#8 drupal.remove-ingroup-themeable-from-preprocess-function-docblocks.2219617-08.patch3.01 KBgalooph
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,813 pass(es). View
#5 drupal.remove-ingroup-themeable-from-preprocess-function-docblocks.2219617-05.patch2.58 KBvisabhishek
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,724 pass(es). View
#5 interdiff-2219617-02-05.txt448 bytesvisabhishek
#2 remove-ingroup-themeable-2219617-2.patch2.26 KBjoshtaylor
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,411 pass(es). View

Comments

joshtaylor’s picture

Assigned: Unassigned » joshtaylor

I will be writing a patch to fix this issue.

joshtaylor’s picture

FileSize
2.26 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,411 pass(es). View

This patch removes the @ingroup themeable from the following functions:

  • template_preprocess_details
  • template_preprocess_input
  • template_preprocess_form_element
  • template_preprocess_admin_block_content
  • template_preprocess_admin_page
  • template_preprocess_system_themes_page

I couldn't find the @ingroup themeable in the following functions, as per the https://api.drupal.org/api/drupal/core%21modules%21system%21theme.api.ph... page, as I think they might have been removed already?:

  • template_preprocess_container
  • template_preprocess_datetime
  • template_preprocess_feed_icon
  • template_preprocess_item_list
  • template_preprocess_links
  • template_preprocess_status_messages
  • template_preprocess_status_report
joshtaylor’s picture

Status: Active » Needs review
Cottser’s picture

Status: Needs review » Needs work

Thanks @joshtaylor! I think the only one that was missed is template_preprocess_status_report().

Edit: Which I see is in the list of ones you couldn't find but is there I promise :)

visabhishek’s picture

Status: Needs work » Needs review
FileSize
448 bytes
2.58 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,724 pass(es). View

updated patch as per #4. Please review

galooph’s picture

Patch from #5 looks good. Verified that @ingroup themeable was removed from:

  • template_preprocess_details()
  • template_preprocess_input()
  • template_preprocess_form_element()
  • template_preprocess_admin_block_content()
  • template_preprocess_admin_page()
  • template_preprocess_system_themes_page()
  • template_preprocess_status_report()

and that it was already missing from:

  • template_preprocess_container()
  • template_preprocess_datetime()
  • template_preprocess_feed_icon()
  • template_preprocess_item_list()
  • template_preprocess_links()
  • template_preprocess_status_messages()
Cottser’s picture

Status: Needs review » Needs work

Sorry I missed this because I just did grep before, but we need to remove the line above the @ingroup themeable as well.

So instead of:

+++ b/core/includes/form.inc
@@ -1068,7 +1068,6 @@ function template_preprocess_fieldset(&$variables) {
  *     Properties used: #attributes, #children, #open,
  *     #description, #id, #title, #value, #optional.
  *
- * @ingroup themeable
  */
 function template_preprocess_details(&$variables) {
   $element = $variables['element'];

Should be more like:

+++ b/core/includes/form.inc
@@ -1068,7 +1068,6 @@ function template_preprocess_fieldset(&$variables) {
  *     Properties used: #attributes, #children, #open,
  *     #description, #id, #title, #value, #optional.
- *
- * @ingroup themeable
  */
 function template_preprocess_details(&$variables) {
   $element = $variables['element'];
galooph’s picture

Status: Needs work » Needs review
FileSize
3.01 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,813 pass(es). View

Here's a revised patch as per the comments in #7.

Status: Needs review » Needs work
Cottser’s picture

Status: Needs work » Needs review
Cottser’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @galooph!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Nice work!

Committed and pushed to 8.x. Thanks!

  • Commit 627b7bb on 8.x by webchick:
    Issue #2219617 by visabhishek, galooph, joshtaylor, Cottser: Remove @...
webchick’s picture

Oh, also? Thanks for making our docs clearer. :)

Status: Fixed » Closed (fixed)

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