Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding documentation for WordPress.Arrays.ArrayDeclarationSpacing #2489

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Adding documentation for WordPress.Arrays.ArrayDeclarationSpacing #2489

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions WordPress/Docs/Arrays/ArrayDeclarationSpacingStandard.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?xml version="1.0"?>
<documentation xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://phpcsstandards.github.io/PHPCSDevTools/phpcsdocs.xsd"
title="Array Declaration Spacing"
>
<standard>
<![CDATA[
When an array uses associative keys, each pair of key and value must start on a new line.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the sniff error message uses the term associative keys, but I'm inclined to think that just keys is better (and we can also consider updating the sniff error message on a separate PR, if the maintainers agree with the point that I'm raising here). Saying associative keys can be misleading as it might mean to some people that indexed arrays that explicitly declare their keys will not trigger this error when they trigger it. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that key/value pair is more commonly used than pair of key and value.

]]>
</standard>
<code_comparison>
<code title="Valid: There is only one pair of key and value per line.">
<![CDATA[
$args = array(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the issue the sniff is about, please check that the code in each code sample follows the WP Coding Standards. I believe this code example is missing a trailing comma after the last array item, and the alignment of the first double arrow is incorrect.

<em>'post_id' => 22,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, it is slightly better if the multi line examples use one pair of <em> tags per line (here and in the valid example below):

$args = array(
    <em>'post_id' => 22,</em>
    <em>'category' => 1</em>
);

This makes a small difference when generating the HTML version of the documentation (vendor/bin/phpcs --generator=HTML --standard=WordPress --sniffs=WordPress.Arrays.ArrayDeclarationSpacing.

Current version:

Screenshot from 2024-09-25 09-41-37

Version with the suggested change:

Screenshot from 2024-09-25 09-41-43

'category' => 1</em>
);
]]>
</code>
<code title="Invalid: More than one pair of key and value per line.">
<![CDATA[
$args = array(
<em>'post_id' => 22, 'category' => 1</em>
);
]]>
</code>
</code_comparison>
<standard>
<![CDATA[
Each item in a multi-line array must be on a new line.
]]>
</standard>
<code_comparison>
<code title="Valid: Only one array item per line.">
<![CDATA[
$args = array(
<em>'post_id',
'comment_count',
'post_type'</em>
);
]]>
</code>
<code title="Invalid: Multiple array items in the same line.">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should include an example of an associative array that triggers this error to make it explicitly that this one is not only about indexed arrays without declared keys?

The error above is only triggered for single-line arrays with declared keys.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should we include in the invalid phrase that this only happens on a multi-line array? Maybe something like: Invalid: More than one item per line in a multi-line array.

<![CDATA[
$args = array(
<em>'post_id', 'comment_count', 'post_type'</em>
);
]]>
</code>
</code_comparison>
</documentation>
Loading