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

no-multi-spaces conflicts with smart-tabs #2077

Closed
bluesmoon opened this issue Mar 16, 2015 · 18 comments
Closed

no-multi-spaces conflicts with smart-tabs #2077

bluesmoon opened this issue Mar 16, 2015 · 18 comments
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 enhancement This change enhances an existing feature of ESLint good first issue Good for people who haven't worked on ESLint before help wanted The team would welcome a contribution from the community for this issue rule Relates to ESLint's core rules

Comments

@bluesmoon
Copy link

If I have "no-mixed-spaces-and-tabs":[2, "smart-tabs"], then no-multi-spaces should not throw an error/warning for lines that use smart-tabs.

test case, try with the following code:

/*eslint no-mixed-spaces-and-tabs:[2, "smart-tabs"], no-multi-spaces:2 */
var i=10,
    j=20;

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@nzakas
Copy link
Member

nzakas commented Mar 16, 2015

What version of ESLint are you using? I don't see any errors with this code.

In the future, please also include the errors you're seeing on the console when reporting an issue. It really helps us triage.

@nzakas nzakas added the triage An ESLint team member will look at this issue soon label Mar 16, 2015
@bluesmoon
Copy link
Author

0.17.0

   441:7   warning  Multiple spaces found before 'i'                  no-multi-spaces
   442:7   warning  Multiple spaces found before 'gotcookies'         no-multi-spaces
   443:7   warning  Multiple spaces found before 'cookies'            no-multi-spaces

code:

            var cookies_a,
                i, l, kv,
                gotcookies=false,
                cookies={};

(3 tabs, and then 4 spaces to align variable names)

/*eslint-env browser*/
/*global BOOMR:true, BOOMR_start:true, BOOMR_lstart:true, console:false*/
/*eslint no-mixed-spaces-and-tabs:[2, true], console:0, camelcase:0, strict:0, quotes:[2, "double", "avoid-escape"], new-cap:0*/
/*eslint space-infix-ops:0, no-console:0, no-delete-var:0, no-space-before-semi:0, no-multi-spaces:1, space-unary-ops: 0, key-spacing: 0, dot-notation: [2, {"allowKeywords": true }]*/

@nzakas
Copy link
Member

nzakas commented Mar 16, 2015

Ah okay, GitHub was converting those to spaces.

Yeah, we can probably just ignore spaces that are in indent position.

@mourner
Copy link
Contributor

mourner commented May 11, 2015

Came here to open the same issue. 👍

@gyandeeps
Copy link
Member

@nzakas @ilyavolodin Based on the discussion, I think a decision has been made but the labels are not yet updated.

@ilyavolodin ilyavolodin added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jun 7, 2015
@nzakas
Copy link
Member

nzakas commented Jun 8, 2015

Yup, thanks!

@message
Copy link
Contributor

message commented Jul 6, 2015

+1

1 similar comment
@ohshhh
Copy link

ohshhh commented Jul 6, 2015

+1

@nzakas
Copy link
Member

nzakas commented Jul 6, 2015

@message @tape88 thanks for commenting. This issue is marked as "accepted", so it's just waiting for someone to volunteer to work on it.

@satya164
Copy link

Have the same issue. Anyone working on it yet?

@IanVS
Copy link
Member

IanVS commented Jul 27, 2015

@satya164 It's all yours if you want it. Looks like some tweaking is needed in: https://github.com/eslint/eslint/blob/master/lib/rules/no-multi-spaces.js#L73

@satya164
Copy link

@IanVS I can work on it, can you point me to documentation for writing rules?

@IanVS
Copy link
Member

IanVS commented Jul 27, 2015

Yep, rule development docs are at: http://eslint.org/docs/developer-guide/working-with-rules

I would suggest to create a test case first which fails, and then update the regex I linked to so that the test passes. Feel free to hop in the Gitter room if you need more guidance.

@IanVS
Copy link
Member

IanVS commented Nov 2, 2015

@satya164, do you still plan to work on this? I verified it is still an issue in 1.8.0.

@IanVS IanVS added the good first issue Good for people who haven't worked on ESLint before label Nov 2, 2015
@oh-ren
Copy link

oh-ren commented Nov 17, 2015

+1 :) (Still an issue in 1.9.0)

Btw, although somewhat a different discussion, when using smart-tabs, I also have to set VariableDeclarator to 0 in my indent options, otherwise I get complaints from that :)
It's my feeling when using smart-tabs, this should be handled in a smart way too. Setting to 0 is of course undesirable, as the linter then also just accepts no 'smart' indenting/alignment at all...

[edit] It could make sense / clear things up to house all this under the indent rule and remove the no-mixed-spaces-and-tabs rule?

@nzakas nzakas added the help wanted The team would welcome a contribution from the community for this issue label Nov 17, 2015
@nzakas
Copy link
Member

nzakas commented Nov 17, 2015

@oh-ren please open a separate issue for that discussion.

@oh-ren
Copy link

oh-ren commented Nov 17, 2015

@nzakas Will do, was already thinking about that myself as well.

@afahim
Copy link

afahim commented Feb 16, 2016

I've taken a stab at this, let me know if you have any comments / suggestions.

afahim pushed a commit to afahim/eslint that referenced this issue Feb 16, 2016
afahim pushed a commit to afahim/eslint that referenced this issue Feb 16, 2016
afahim pushed a commit to afahim/eslint that referenced this issue Feb 18, 2016
@nzakas nzakas closed this as completed in 211eb8f Feb 19, 2016
nzakas added a commit that referenced this issue Feb 19, 2016
Fix: no-multi-spaces conflicts with smart tabs (fixes #2077)
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 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 7, 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 enhancement This change enhances an existing feature of ESLint good first issue Good for people who haven't worked on ESLint before help wanted The team would welcome a contribution from the community for this issue rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests