[Update] Docs for array declaration spacing sniff #2593
Conversation
Update terminology from 'pair of key and value' to 'key/value pair'
- Add trailing commas after last array items - Align double arrows for better readability
jrfnl
changed the title
There was a problem hiding this comment.
Thanks for working on this PR @matt-galdino!
I left a comment about one of my original remarks and a few other things that I had missed in my original review.
| <standard> | ||
| <![CDATA[ | ||
| Associative arrays with multiple key-value pairs must also follow this rule. | ||
| ]]> | ||
| </standard> | ||
| <code_comparison> | ||
| <code title="Valid: Each key-value pair on its own line."> | ||
| <![CDATA[ | ||
| $settings = array( | ||
| <em>'width' => 300</em>, | ||
| <em>'height' => 200</em>, | ||
| <em>'color' => 'blue'</em>, | ||
| ); | ||
| ]]> | ||
| </code> | ||
| <code title="Invalid: Multiple key-value pairs on the same line in a multi-line array."> | ||
| <![CDATA[ | ||
| $settings = array( | ||
| <em>'width' => 300, 'height' => 200</em>, | ||
| <em>'color' => 'blue'</em>, | ||
| ); | ||
| ]]> | ||
| </code> | ||
| </code_comparison> |
There was a problem hiding this comment.
I'm sorry if my comment in the original PR was not clear, but in https://github.com/WordPress/WordPress-Coding-Standards/pull/2489/files#r1775277204, I was not suggesting to add a new <standard>/<code_comparison> block. I don't think this is necessary. Instead, I was suggesting adding a new valid/invalid example to the <code_comparison> block that already exists. <code_comparison> blocks can have multiple examples when needed. Or do you see a reason to have a separate <standard>/<code_comparison> block in this case?
| $args = array( | ||
| <em>'post_id' => 22, 'category' => 1</em>, | ||
| ); |
There was a problem hiding this comment.
One thing that I missed in my original review is that in order for this example to trigger the AssociativeArrayFound error, which I believe is the intention of this example, everything should be in the same line. Currently, this example triggers the ArrayItemNoNewLine error that is already covered in the <standard>/<code_comparison> block below. Could you please look into that? Let me know if you need any help. You might need to adjust the example so that it fits in a single line because of the 48 characters per line limit (not counting the <em> tags).
| > | ||
| <standard> | ||
| <![CDATA[ | ||
| When an array uses keys, each key/value pair must start on a new line. |
There was a problem hiding this comment.
There is something else that I missed in my original review. Using the default configuration of the sniff, which is what we want to document the XML file, the error ArrayItemNoNewLine is only triggered for multi-item single-line arrays, so it might be worth including this information in the description and in the code comparison titles.
| When an array uses keys, each key/value pair must start on a new line. | |
| When a multi-item array uses keys, each key/value pair must start on a new line. |
| ]]> | ||
| </standard> | ||
| <code_comparison> | ||
| <code title="Valid: There is only one key/value pair per line."> |
There was a problem hiding this comment.
| <code title="Valid: There is only one key/value pair per line."> | |
| <code title="Valid: Only one key/value pair per line on a multi-item array."> |
| ); | ||
| ]]> | ||
| </code> | ||
| <code title="Invalid: More than one key/value pair per line."> |
There was a problem hiding this comment.
| <code title="Invalid: More than one key/value pair per line."> | |
| <code title="Invalid: Single line multi-item array using keys."> |
Related to PR #1722
Surpasses RafaelFunchal's changes made here: #2489
Closes #2489