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

Fix: no-multi-spaces to avoid reporting consecutive tabs (fixes #9079) #9087

Merged
merged 1 commit into from Aug 7, 2017

Conversation

not-an-aardvark
Copy link
Member

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

[x] Bug fix (#9079)

What changes did you make? (Give an overview)

An unintended side-effect of the refactor in 0f97279 caused no-multi-spaces to start reporting any consecutive whitespace characters between tokens, rather than just reporting consecutive spaces. This commit fixes the rule to only report consecutive spaces.

To be honest, reporting any consecutive whitespace seems like it would also be a reasonable behavior -- I'm not really convinced that it's better for the rule to only report spaces. But that would be a semver-major change.

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

Nothing in particular

An unintended side-effect of the refactor in 0f97279 caused `no-multi-spaces` to start reporting any consecutive whitespace characters between tokens, rather than just reporting consecutive spaces. This commit fixes the rule to only report consecutive spaces.
@not-an-aardvark not-an-aardvark added accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly regression Something broke rule Relates to ESLint's core rules labels Aug 7, 2017
@eslintbot
Copy link

LGTM

@mention-bot
Copy link

@not-an-aardvark, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alberto, @nzakas and @vegetableman to be potential reviewers.

@kaicataldo
Copy link
Member

kaicataldo commented Aug 7, 2017

To be honest, reporting any consecutive whitespace seems like it would also be a reasonable behavior -- I'm not really convinced that it's better for the rule to only report spaces. But that would be a semver-major change.

I agree with this.

@gyandeeps
Copy link
Member

To be honest, reporting any consecutive whitespace seems like it would also be a reasonable behavior -- I'm not really convinced that it's better for the rule to only report spaces. But that would be a semver-major change.

Totally agree. I think we should make an issue for this.

@kaicataldo kaicataldo merged commit ec93614 into master Aug 7, 2017
@gyandeeps gyandeeps deleted the no-multi-spaces-consecutive-tabs branch August 7, 2017 15:22
@platinumazure
Copy link
Member

To be honest, reporting any consecutive whitespace seems like it would also be a reasonable behavior -- I'm not really convinced that it's better for the rule to only report spaces. But that would be a semver-major change.

Could this be done behind an option to allow for semver-minor release and let people opt into it, and then switch the option default in a semver-major?

@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
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly regression Something broke rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants