Follow-up to #2452371: Remove @addtogroup themeable from theme.inc, only theme_indentation() needs it

Problem/Motivation

Currently the following preprocess functions are contained in the themeable documentation group:

Preprocess functions shouldn't be in the themeable documentation group. Preprocess functions that are mistakenly added as part of the themeable group show up on the themeable API page which we don't want.

Also, there are a few theme functions still in core that are missing the @ingroup themeable line, so we should add the lines to those theme functions so that all themeable output in core is linked to from the themeable API page:

Documentation standards references:

https://www.drupal.org/node/1354#themeable
https://www.drupal.org/node/1354#templates

https://www.drupal.org/node/1354#themepreprocess

Proposed resolution

  • Remove the @ingroup themeable lines (and preceding blank line) from the docblocks of the preprocess functions listed above.
  • Add @ingroup themeable lines (and preceding blank line) to the docblocks of the theme functions listed above.

Remaining tasks

  • Create patch
  • Review patch

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because it doesn't mesh with our existing coding standards for preprocess functions and theme functions
Issue priority Normal
Unfrozen changes Unfrozen because it only changes documentation
Prioritized changes This is not a prioritized change for the beta phase.
Disruption No disruption for core or contrib.

User interface changes

n/a

API changes

n/a

Files: 
CommentFileSizeAuthor
#6 interdiff.txt657 bytesrteijeiro
#6 2457271-6.patch3.57 KBrteijeiro
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 90,459 pass(es). View
#2 2457271-2.patch2.93 KBPalashvijay4O
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 90,124 pass(es). View

Comments

Cottser’s picture

Issue summary: View changes

Removing empty li.

Palashvijay4O’s picture

FileSize
2.93 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 90,124 pass(es). View

I found that theme_language_content_settings_table is already ingroup themeable. Attached a patch with other changes please review.

Palashvijay4O’s picture

Status: Active » Needs review
rteijeiro’s picture

Status: Needs review » Reviewed & tested by the community

Looks good for me!

jhodgdon’s picture

Status: Reviewed & tested by the community » Needs work

Ah. Well, in language.admin.inc:

 *
* @ingroup themable
*/
function theme_language_content_settings_table($variables) { 

The problem with this is that themeable is spelled wrong in the ingroup line, which is why this function is not actually added to the Themeable topic page. Can you fix this in the patch?

rteijeiro’s picture

Status: Needs work » Needs review
FileSize
3.57 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 90,459 pass(es). View
657 bytes

Nice catch @jhodgdon!! Thanks for the review!

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

And thanks @Cottser for the research that went into filing this issue. Committer: I suggest giving credit to Cottser as well as rteijeiro and Palashvijay40, since this research was integral to getting this resolved.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Great. Clarity++.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 03aed6a on 8.0.x
    Issue #2457271 by rteijeiro, Palashvijay4O, Cottser: More cleanup of the...

Status: Fixed » Closed (fixed)

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