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

Update: add allow underscores options to camelcase rule. #10474

Conversation

tinymins
Copy link
Contributor

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[X] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Add options to control whether leading and trailing underscores are allowed in id naming.

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

The default value. This option should be enabled because it was enabled in previous version.

@jsf-clabot
Copy link

jsf-clabot commented Jun 13, 2018

CLA assistant check
All committers have signed the CLA.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Jun 13, 2018
@aladdin-add aladdin-add added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jun 13, 2018
@aladdin-add aladdin-add changed the title New: add underscores options to camelcase rule. Update: add underscores options to camelcase rule. Jun 13, 2018
@tinymins tinymins force-pushed the allow-leading-and-trailing-underscores branch from 09692e4 to eea37f6 Compare June 13, 2018 11:18
@tinymins tinymins changed the title Update: add underscores options to camelcase rule. Update: add allow underscores options to camelcase rule. Jun 13, 2018
@platinumazure
Copy link
Member

I'm not sure this is necessary because you could use id-match for this sort of scenario as well.

@tinymins
Copy link
Contributor Author

@platinumazure I've tried id-match. There're too much problems to use that, even Math.floor will be reported when use id-match.

@tinymins
Copy link
Contributor Author

image
@platinumazure Here's the screenshot. Use id-match in current version cannot limit lower-camelcase in property. Rule id-match also need to be updated to satisfy this situation.

@platinumazure
Copy link
Member

The reason I recommended id-match here is because the entire point of the camelcase rule is to forbid underscores. Maybe we could consider an enhancement for your specific use case, but id-match was created to allow users to specify their own identifier rules. (That said, the fact that you might need different rules for variables versus properties is probably something that id-match is not currently designed for. Not saying we can't enhance it, but rather that I hadn't been aware of your requirement before I had recommended that rule.)

Let's see what the rest of the team thinks.

@tinymins
Copy link
Contributor Author

@platinumazure What you said is also what I want. Actually camelcase allows leading underscores now, but in my project we want to disallow any underscores. Shall we remove the leading and trailing underscores ignore code in rule camelcase?

@tinymins
Copy link
Contributor Author

@tinymins
Copy link
Contributor Author

#10554 id-match rule improved.

@tinymins tinymins closed this Nov 15, 2018
@tinymins tinymins deleted the allow-leading-and-trailing-underscores branch November 15, 2018 10:37
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators May 15, 2019
@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 May 15, 2019
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 enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants