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.

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

Check spacing of function declarations #417

Merged
merged 5 commits into from
Aug 25, 2015
Merged

Check spacing of function declarations #417

merged 5 commits into from
Aug 25, 2015

Conversation

JDGrimes
Copy link
Contributor

This adds functions and closures to the control structure spacing
sniff. The sniff checks for proper spacing on each side of each of the
parentheses. It also checks that the opening brace is on the same line.

I’ve actually improved the sniff while I was at it, to check for excess
space in addition to insufficient space.

Because of these improvements, and due to the non-configurability of
the upstream version of this sniff, this sniff is really not a
duplicate, and so #385 can be closed.

I’ve also removed our implementation of the FunctionDeclarationArgumentSpacing sniff in favor of the upstream version. This is some overlap between that sniff and this one, but that one only checks the spacing of the arguments.

Fixes #412, #385

JDGrimes added 2 commits July 29, 2015 18:07
The upstream version is now configurable to meet our needs, and also
contains improvements, like being fixable.

See #412, #385
This adds functions and closures to the control structure spacing
sniff. The sniff checks for proper spacing on each side of each of the
parentheses. It also checks that the opening brace is on the same line.

I’ve actually improved the sniff while I was at it, to check for excess
space in addition to insufficient space.

Because of these improvements, and due to the non-configurability of
the upstream version of this sniff, this sniff is really not a
duplicate, and so #385 can be closed.

Fixes #412, #385
}
}

if (
Copy link
Member

Choose a reason for hiding this comment

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

This block seems to be indented one level too many?

Adds the `spaces_before_closure_open_paren` setting to the sniff. It
determines how many spaces come between the `function` keyword in a
closure and the opening parenthesis.
@JDGrimes JDGrimes added this to the 0.7.0 milestone Aug 25, 2015
@JDGrimes
Copy link
Contributor Author

I've updated this:

  • Handling for use statements in 7d19272.
  • Allow closure spacing to be configured to be either 1 or 0 spaces before the opening parenthesis (c7276e3).
  • I left the indentation, just because I didn't feel like fixing it. It's a tabs vs spaces thing, and there are so many places where it is messed up that the best thing would be to just convert the whole file to tabs. But that is for another pull request.

GaryJones added a commit that referenced this pull request Aug 25, 2015
@GaryJones GaryJones merged commit b20a852 into develop Aug 25, 2015
@GaryJones
Copy link
Member

Excellent work, thanks @JDGrimes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parameters in functions declarations are not forced to be padded with space
2 participants