[Documentation] Squiz: Superfluous Whitespace

[jaymcp]

Description

This PR adds documentation for the Squiz.WhiteSpace.SuperfluousWhitespace sniff.

Suggested changelog entry

Add documentation for the Squiz Superfluous Whitespace sniff

Related issues/external references

Part of #148

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
  • This change is only breaking for integrators, not for external standards or end-users.
• Documentation improvement

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.
• I have verified that the code complies with the projects coding standards.
• [Required for new sniffs] I have added XML documentation for the sniff.

[jrfnl]

@jonmcp Thank you for taking on this sniff !

Aside from the inline notes, I noticed that the "Additional whitespace found at end of file" error is not documented. Could you please add the documentation for that ?
It's basically about whitespace found after a PHP close tag (without inline HTML contents after).

Nitpick: it looks like the CDATA close tag for all code samples is not indented ?

[jaymcp]

Thanks for the review, @jrfnl; I'll address your feedback today.

it looks like the CDATA close tag for all code samples is not indented

I found that the existing documentation didn't seem to agree on whether the closing CDATA tag should be indented or not, so I chose to follow the format of most of the other Squiz sniffs . I should perhaps have asked prior to opening my PR, and am sorry for not doing so.
I will of course address the indentation here. Would it be helpful if I were to open a separate PR to normalise the indentation of closing CDATA tags across the existing Squiz documentation to meet the style of the other standards? I could perhaps open a PR for each standard where there are issues?

[jrfnl]

it looks like the CDATA close tag for all code samples is not indented

I found that the existing documentation didn't seem to agree on whether the closing CDATA tag should be indented or not, so I chose to follow the format of most of the other Squiz sniffs . I should perhaps have asked prior to opening my PR, and am sorry for not doing so. I will of course address the indentation here.

@jonmcp No worries about asking first, it's not a biggie, just something I noticed while reviewing.

Would it be helpful if I were to open a separate PR to normalise the indentation of closing CDATA tags across the existing Squiz documentation to meet the style of the other standards? I could perhaps open a PR for each standard where there are issues?

That would be amazing! ❤️ I'm sure there will be plenty of inconsistencies as the docs haven't gotten much love over the last decade or so... Anything you can do to make them more consistent/improve them would be appreciated!

[jaymcp]

Thanks @jrfnl!

Once I've addressed feedback in my existing PRs, I'll prioritise normalising the existing sniff docs over creating new docs, so that it's easier going forward.

I think I've addressed all of your current feedback in this PR, but I'm not sure what the correct etiquette is now. When I think a PR is ready for a re-review, would it be helpful for me to modify the labels on the PR, or is it best if I leave those alone? I did re-read the contribution guidelines, and couldn't see anything specific about labels or whatnot, but please forgive me if I've missed something there. I'd like to be as helpful as I can be without getting in the way! 🙂

[jrfnl]

I think I've addressed all of your current feedback in this PR, but I'm not sure what the correct etiquette is now. When I think a PR is ready for a re-review, would it be helpful for me to modify the labels on the PR, or is it best if I leave those alone? I did re-read the contribution guidelines, and couldn't see anything specific about labels or whatnot, but please forgive me if I've missed something there. I'd like to be as helpful as I can be without getting in the way! 🙂

@jonmcp I don't think you can adjust the labels, unless you have triage/commit access to the repo... or am I missing something ?

If you would be able to update the labels, then removing the "Status: awaiting feedback" label would be the way to go. For PRs, I use that label to filter out PRs which have been reviewed and are waiting for an update/reply.

I still mean to look into creating a GH Actions workflow to automatically remove that label when a PR has been updated and/or received a reply from the original author, but I haven't got round to that yet (well, I did look into it a bit and found that it probably wouldn't be easy to create, so I left it for now)

Other than that, there's nothing for you to do, I have email notifications turned on for this repo, so I automatically get notified of updates and will look at them when I have time.

[jrfnl]

@jonmcp Thanks for making that update!

We're close, but the new section still needs work. I've left two notes inline.

[jaymcp]

I don't think you can adjust the labels… Other than that, there's nothing for you to do…

You are indeed correct. I was hoping I might be able to help save a little time, but I ended up doing the opposite. Sorry about that, and thanks for your patience.

[jrfnl]

Thank you @jonmcp for working on this and making those updates!

It all looks good to me now, so the docs will be included in the next release.

For the record: on the command-line the code samples don't always show a difference. This is due to the sniff being about whitespace and cannot be helped.
The text strings used within the code samples stating what is happening, should still illustrate this well enough and are a good solution for the docs for this particular sniff.