Customize: Further improve inline documentation

[westonruter]

We need to do another pass on the customizer code to ensure that inline documentation is added to both PHP and JS. This is building on #27534, #27065, #21303 among others.

For example, per @grapplerulrich:

I would like to see parts of the documentation improved. For example: In WP_Customize_Selective_Refresh::add_partial( $id, $args ) there is no inline documentation what values can be provided for teh args. Only by reading the code I am able to work out what the variables are but it is not so easy as the args are all of the public variables in the class.

​https://make.wordpress.org/design/2017/01/13/what-makes-a-great-customization-experience/#comment-24907

[mrahmadawais]

May I ask, what is the scope of this ticket. The amount of documentation needed is subjective. More info will help me manage my time if I adopt this one? @westonruter

[westonruter]

@mrahmadawais scope is phpdoc and jsdoc which are absent or lacking.

[mrahmadawais]

@westonruter I'll try and jump in this next week.

[4nickpick]

Improves inline documentation for add_partial

[4nickpick]

I took a stab at adding documentation to the method specifically complained about. Would appreciate some feedback to make sure I'm in the right direction.

[grapplerulrich]

@4nickpick This looks good. I see you copied the documentation of the properties in class-wp-customize-partial.php :)

For $settings I would mention what happens if undefined e.g. "IDs for settings tied to the partial. If undefined, $id will be used.". I am not sure if we can somehow explain how the array should be structured. From my understanding they a list of IDs.

We should also add that the default for $type is default, $container_inclusive is false and $fallback_refresh is true.

[grapplerulrich]

These methods also also in need to improved documentation.

• WP_Customize_Manager::add_panel()
• WP_Customize_Manager::add_section()
• WP_Customize_Manager::add_setting()
• WP_Customize_Manager::add_control()

[4nickpick]

Adds and expands inline docs for partials, settings, sections, controls and panels.

[4nickpick]

In this case, the arrays inside the $args parameters are relatively straightforward. I expanded on them a little bit in 39671-1.patch.

The scope of this ticket also includes jsdocs. I'll be digging into those next.

[grapplerulrich]

This looks really good. It is going to make live so much easier.

The documentation in core for $sanitize_js_callback has not been fully clear. Instead of "Callback to filter a Customize setting value for use in Javascript" perhaps "Callback to convert a Customize PHP setting value to a value that is JSON serializable."

[4nickpick]

tweak docs on Partial's sanitize_js_callback argument

[4nickpick]

Still digging into the js end of Customizer. If there's any particularly bad file/method/section, or somewhere you think would be a good starting point, that would be helpful.

I'm specifically looking into these files:
customize-base.js
customize-loader.js
customize-models.js
customize-preview.js
customize-preview-nav-menus.js
customize-preview-widgets.js
customize-selective-refresh.js
customize-views.js

A lot of methods missing docs completely, I'll be starting creating stubs for those.

[grapplerulrich]

I think inline documentation for these features would be good.

• Selective Refresh JavaScript Events
• Notifications
• Client-side Validation

​https://developer.wordpress.org/themes/customize-api/tools-for-improved-user-experience/
​https://developer.wordpress.org/themes/customize-api/the-customizer-javascript-api/

[swissspidy]

A quick note on 39671-2.patch​

• We don't use @see for URLs, but @link instead.
• Don't link to the codex. ​https://codex.wordpress.org/Theme_Customization_API already mentions that one should use the handbooks.
• @since annotations would be nice.

For reference: ​https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/

The other areas mentioned above would need (separate) patches as well.

[sagarprajapati]

Hi @swissspidy

I have updated patch with the above-listed suggestions. Please check it from your side and let me know your suggestions.

Thanks

[swissspidy]

A few observations:

• Indentation and alignment is not always correct.
• @since annotations are missing

Moving to 4.7.5 as we're not in a hurry to move this into the next release.

[ketuchetan]

Hello @swissspidy

I have added my patch with the proper indentation and added missing tag @since tag on the doc block

Thanks,

[ericdaams]

@sagarprajapati @ketuchetan Your patch is missing the changes to wp-includes/customize/class-wp-customize-selective-refresh.php that were in @4nickpick's patch. Was that intentional?

[ketuchetan]

Hi @ericdaams

I have included the wp-includes/customize/class-wp-customize-selective-refresh.php missing changes.

Thanks,

[westonruter]

The @since tags aren't correct. They need to reflect the version in which the aspect is added, not the version in which the @since tag is added. So much of them will be 3.4.0 when the Customizer was first introduced.

[BharatKambariya]

Hi @westonruter

I have updated patch with @since changes. Please check it.

Thanks

[mrahmadawais]

Enhancement: Improve inline documentation and fix several @access tags for Customizer

[mrahmadawais]

@westonruter I spent some time today on this ticket today and have added my patch that addresses the following:

• Add Inline Docs where missing
• Fix wrong @access tags (found private access tags for public functions)
• Consistent @access tags throughout these files
• Consistent order of DocBlock tags throughout the files as per ​WP Docs Standards

Kindly, have a look at it when possible :)

[westonruter]

There are changes to kses.php which seem to have snuck into the patch. Those need to be removed.

Looking through some of the @since tags, I can see that they're not all accurate. For example \WP_Customize_Control::$input_attrs was added in 4.0.0 not 3.4.0 (see #28477). Likewise, \WP_Customize_Setting::$validate_callback was added in 4.6.0 not 3.4.0 (see #34893). So some more QA is needed on the patch.

The changes to the phpdoc comments before each require_once in class-wp-customize-control.php shouldn't be necessary. In fact, I don't think phpdoc comments are even appropriate here.

[westonruter]

Fix a couple incorrect @since tags; revert changes to kses.php; improve a couple descriptions

[mrahmadawais]

Replying to westonruter:

The changes to the phpdoc comments before each require_once in class-wp-customize-control.php shouldn't be necessary. In fact, I don't think phpdoc comments are even appropriate here.

I made those changes in line with the Doc Standards for ​Requires and Includes. I hope that clarifies it.

Is this ticket now ready to be committed? I think it's a good addition — in the next major version we can also take a pass on JS files.

[westonruter]

@mrahmadawais you're right about:

I made those changes in line with the Doc Standards for Requires and Includes. I hope that clarifies it.

The changes to the @access tags on class methods however doesn't seem right. In looking at ​Functions & Class Methods it says:

access: Only use for standalone functions if private. If the function is private it will be output with a message stating its intention for internal use.

So I think the presence of @access for all of the class methods actually should be removed, which also makes sense because it's entirely redundant with the member visibility keywords which is part of the PHP language itself.

[westonruter]

It also seems the docs for use of @access on class member properties should also be updated to note that it shouldn't be used, as I think perhaps it could be a carryover from PHP 4 which used just plain var keywords and lacked any visibility at the PHP language level.

[westonruter]

Restore require docs; remove method access tags; correct additional since tags

[mrahmadawais]

Replying to westonruter:

It also seems the docs for use of @access on class member properties should also be updated to note that it shouldn't be used, as I think perhaps it could be a carryover from PHP 4 which used just plain var keywords and lacked any visibility at the PHP language level.

That makes a lot of sense. Since I find them quite redundant as well. Though, being part of the guidelines, I did add them anyway.

So I think the presence of @access for all of the class methods actually should be removed, which also makes sense because it's entirely redundant with the member visibility keywords which is part of the PHP language itself.

I missed that. Thanks for letting me know.

Is there anything else left for this patch? I did take a cursory look at the last patch and there are certain functions for which access is not defined E.g. class-wp-customize-control.php at line # 484 the render_content() has protected access. But that's not defined in the DocBlock. Should it be?

[westonruter]

Replying to mrahmadawais:

That makes a lot of sense. Since I find them quite redundant as well. Though, being part of the guidelines, I did add them anyway.

Actually, I interpret the guidelines to mean that @access should be used for functions not methods.

Is there anything else left for this patch? I did take a cursory look at the last patch and there are certain functions for which access is not defined E.g. class-wp-customize-control.php at line # 484 the render_content() has protected access. But that's not defined in the DocBlock. Should it be?

So I think essentially all of the @access tags actually should be removed from the docblocks for methods in these classes. I've tried to get confirmation on this from #docs: ​https://wordpress.slack.com/archives/C02RP4WU5/p1495151067056851

[westonruter]

Also just came across this:

Incidentally, the @access tag has no context with regard to a class itself. Visibility scope of public/protected/private only applies to class methods and class properties, not to a class itself. Further, it was added to phpDocumentor 1.x back in the PHP4 days, before such visibility scope was available in PHP at all (that was added in PHP5). Therefore, it's not actually useful anymore. Even in 1.x, if run using PHP5 against code written for PHP5, the code scope keywords would override whatever an @access tag said. I don't believe that phpDocumentor 2.x even bothered implementing the @access tag, and rightly so.

​http://stackoverflow.com/a/20148459/93579

I'm going to commit 39671.9.diff​.

[westonruter]

In 40804:

Docs: Improve phpdoc for WP_Customize_Manager, WP_Customize_Control, WP_Customize_Setting, and WP_Customize_Selective_Refresh.

Props 4nickpick, sagarprajapati, ketuchetan, BharatKambariya, mrahmadawais, westonruter.
Fixes #39671.

[mrahmadawais]

Replying to westonruter:

Also just came across this:

Incidentally, the @access tag has no context with regard to a class itself. Visibility scope of public/protected/private only applies to class methods and class properties, not to a class itself. Further, it was added to phpDocumentor 1.x back in the PHP4 days, before such visibility scope was available in PHP at all (that was added in PHP5). Therefore, it's not actually useful anymore. Even in 1.x, if run using PHP5 against code written for PHP5, the code scope keywords would override whatever an @access tag said. I don't believe that phpDocumentor 2.x even bothered implementing the @access tag, and rightly so.

​http://stackoverflow.com/a/20148459/93579

I'm going to commit 39671.9.diff​.

Great! I think we can address the @access tag along with JS files in WordPress 4.9 then.

[grapplerulrich]

@westonruter Is there a reason why @access was removed in a few places, left in others and added too?

Do we need to create a new issue for the JS documentation?

[westonruter]

Replying to grapplerulrich:

@westonruter Is there a reason why @access was removed in a few places, left in others and added too?

Yes, see discussion at ​https://wordpress.slack.com/archives/C02RP4WU5/p1495224872995687

Do we need to create a new issue for the JS documentation?

There will always be need for more docs tickets :-)

[grapplerulrich]

Yes, see discussion at ​https://wordpress.slack.com/archives/C02RP4WU5/p1495224872995687

I saw the discussion. I would have in this case not made any changes to @access as in https://core.trac.wordpress.org/changeset/40804 it is not consistent.

There will always be need for more docs tickets :-)

Here is the new ticket #40831

[westonruter]

In 42039:

Customize: Fix phpdoc for params in WP_Customize_Manager::add_section().

Amends [40804].
See #39671.
Fixes #42372.