-
-
Notifications
You must be signed in to change notification settings - Fork 490
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.
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
Adding documentation for WordPress.Arrays.ArrayDeclarationSpacing #2489
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe that |
||
]]> | ||
</standard> | ||
<code_comparison> | ||
<code title="Valid: There is only one pair of key and value per line."> | ||
<![CDATA[ | ||
$args = array( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
This makes a small difference when generating the HTML version of the documentation ( Current version: Version with the suggested change: |
||
'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."> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: |
||
<![CDATA[ | ||
$args = array( | ||
<em>'post_id', 'comment_count', 'post_type'</em> | ||
); | ||
]]> | ||
</code> | ||
</code_comparison> | ||
</documentation> |
There was a problem hiding this comment.
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 justkeys
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). Sayingassociative 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?