Skip to content

Generic/ArrayIndent: add XML documentation #432

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

Merged
merged 4 commits into from
Oct 31, 2024
Merged

Generic/ArrayIndent: add XML documentation #432

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
948b2fb
Generic/ArrayIndent: add XML documentation
rodrigoprimo Apr 4, 2024
498983d
Apply suggestions from code review
rodrigoprimo Oct 10, 2024
e4fb292
Implement code review suggestions
rodrigoprimo Oct 10, 2024
da6c78d
Generic/ArrayIndent: tweak the text of the docs some more
jrfnl Oct 31, 2024
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
101 changes: 101 additions & 0 deletions src/Standards/Generic/Docs/Arrays/ArrayIndentStandard.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
<documentation title="Array Indent">
<standard>
<![CDATA[
The opening square bracket/parenthesis of a multi-line array must be indented at least to the same level of the start of the statement.
Copy link
Member

Choose a reason for hiding this comment

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

As a possible alternative (to get away from the kind of braces):

Suggested change
The opening square bracket/parenthesis of a multi-line array must be indented at least to the same level of the start of the statement.
The start of a multi-line array must be indented at least to the same level as the start of the statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO, "opening/closing brace" is clearer than "start/end" so I'm inclined to use that. Please let me know if you disagree.

]]>
</standard>
<code_comparison>
<code title="Valid: Opening square bracket/parenthesis token of a multi-line array indented at least to the same level as the start of the statement.">
<![CDATA[
if ($condition) {
$a =
<em> [</em>
1,
2,
];
}
]]>
</code>
<code title="Invalid: Opening square bracket/parenthesis of a multi-line array not indented at least to the same level as the start of the statement.">
<![CDATA[
if ($condition) {
$a =
<em>[</em>
1,
2,
];
}
]]>
</code>
</code_comparison>
<standard>
<![CDATA[
The elements of an array must be indented four spaces beyond the indentation level of the opening square bracket/parenthesis.
Copy link
Member

Choose a reason for hiding this comment

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

Okay, not sure how to rephrase this, but there are some things here which don't ring true/could be confusing:

  • The indentation is only checked for the first line of each element (subsequent lines are not checked for indentation).
  • "Four spaces beyond"- not sure how much margin that give to interpret this as "at least four spaces", while the rule is "exactly four spaces".
  • "the indentation level of the opening square bracket/parenthesis" - it's not about the indentation/position of the brackets, but about the start of statement which includes the array opener.

Let's rethink the phrasing of this rule.

Note: this also means the Valid/Invalid phrasing may need updating.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with "The first line of each array element must be indented exactly four spaces more than the start of the statement". I also updated the valid/invalid sentences accordingly and changed the <em> tags to highlight only the spaces and not the array element. What do you think?

]]>
</standard>
<code_comparison>
<code title="Valid: Elements of an array indented four spaces beyond the indentation level of the opening square bracket/parenthesis.">
<![CDATA[
$a = array(
<em> 1</em>,
<em> 2</em>,
<em> 3</em>,
);
]]>
</code>
<code title="Invalid: Elements of an array not indented four spaces beyond the indentation level of the opening square bracket/parenthesis.">
<![CDATA[
$a = array(
<em> 1</em>,
2,
<em> 3</em>,
);
]]>
</code>
</code_comparison>
<standard>
<![CDATA[
There must be a newline before the array closing square bracket/parenthesis.
]]>
</standard>
<code_comparison>
<code title="Valid: Newline before the array closing square bracket/parenthesis.">
<![CDATA[
$a = [
1,
2,<em>
]</em>;
]]>
</code>
<code title="Invalid: No newline before the array closing square bracket/parenthesis.">
<![CDATA[
$a = [
1,
2,<em>]</em>;
]]>
</code>
</code_comparison>
<standard>
<![CDATA[
The closing square bracket/parenthesis must be indented four spaces less than the array elements.
Copy link
Member

Choose a reason for hiding this comment

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

Would this be clearer if the indentation was phrased to be related to "same level of indentation as the start of the statement containing the array opener" (or something along those lines) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I don't have a strong preference, but I went with your suggestion anyway because you have more experience with this than I do.

]]>
</standard>
<code_comparison>
<code title="Valid: Closing square bracket/parenthesis indented four spaces less than the array elements.">
<![CDATA[
$a = array(
1,
2,
<em>)</em>;
]]>
</code>
<code title="Invalid: Closing square bracket/parenthesis not indented four spaces less than the array elements.">
<![CDATA[
$a = array(
1,
2,
<em> )</em>;
]]>
</code>
</code_comparison>
</documentation>