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

New: func-call-spacing rule (fixes #6080) #6749

Merged
merged 1 commit into from Aug 9, 2016
Merged

New: func-call-spacing rule (fixes #6080) #6749

merged 1 commit into from Aug 9, 2016

Conversation

btmills
Copy link
Member

@btmills btmills commented Jul 23, 2016

What issue does this pull request address?
#6080 - support requireSpacesInCallExpression and disallowSpacesInCallExpression by replacing the no-spaced-func rule with a new func-call-spacing rule having "never" (default) or "always" modes.

What changes did you make? (Give an overview)

Copied the no-spaced-func rule, named the new rule func-call-spacing, and added support for "never" and "always" options.

Is there anything you'd like reviewers to focus on?

  1. I haven't touched no-spaced-func. What's the proper way to indicate that it's deprecated in favor of func-call-spacing?
  2. For compatibility with JSCS requireSpacesInCallExpression, "always" permits newlines within calls.

@mention-bot
Copy link

@btmills, thanks for your PR! By analyzing the annotation information on this pull request, we identified @nzakas, @alberto and @kaicataldo to be potential reviewers

@eslintbot
Copy link

LGTM

@eslintbot
Copy link

LGTM

@eslintbot
Copy link

LGTM

@eslintbot
Copy link

LGTM

@btmills
Copy link
Member Author

btmills commented Jul 23, 2016

I followed @alberto's example from #6746 and attempted to add deprecation notices and replace references to the old rule.

@nzakas
Copy link
Member

nzakas commented Jul 25, 2016

I think we need to come to some decision on the right way to mark rules as deprecated. See my note on his PR.

This rule has one string option:

- `"never"` (default) disallows space between the function name and the opening parenthesis
- `"always"` requires space between the function name and the opening parenthesis
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

what if i want a space but never a newline?

Copy link
Member

Choose a reason for hiding this comment

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

Disallowing always line breaks sounds good to me.
We can add an option such as allowLineBreaks later if people want.

@nzakas
Copy link
Member

nzakas commented Aug 3, 2016

@btmills are you going to finish this up?

@eslintbot
Copy link

LGTM

@eslintbot
Copy link

LGTM

@btmills
Copy link
Member Author

btmills commented Aug 4, 2016

I've added the allowNewlines boolean option, which is off by default. I also followed the example set in #6746 to deprecate no-spaced-func. This is ready for another round of review.

@ilyavolodin
Copy link
Member

LGTM

@ilyavolodin ilyavolodin merged commit f8ab8f1 into master Aug 9, 2016
@btmills btmills deleted the issue6080 branch August 16, 2016 13:38
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants