[CLOSED] ESLint: enabled no-trailing-spaces and eol-last rules

[core-ai-bot]

Issue by ficristo
Saturday Dec 12, 2015 at 10:36 GMT
Originally opened as adobe/brackets#11998

---

I wanted to enable the indent rule but it seemed to be influenced by the trailing whitespaces.
So I've enabled no-trailing-spaces rule and, since there were only a few warnings the eol-last rule too.
Sorry for the big PR but it's only whitespaces changes and so there should not be any change in behaviour.

@abose@zaggino This PR can bitrot easily: I appreciate a fast turnaround.

---

ficristo included the following code: https://github.com/adobe/brackets/pull/11998/commits

[core-ai-bot]

Comment by zaggino
Saturday Dec 12, 2015 at 17:45 GMT

---

LGTM from me

[core-ai-bot]

Comment by petetnt
Sunday Dec 13, 2015 at 13:42 GMT

---

👍 from me too

[core-ai-bot]

Comment by ficristo
Monday Dec 14, 2015 at 09:01 GMT

---

@zaggino@petetnt Thank you!

[core-ai-bot]

Comment by abose
Monday Dec 14, 2015 at 10:34 GMT

---

@ficristo@zaggino As part of PR https://github.com/adobe/brackets/pull/11693/files
.brackets.json file was edited from "linting.prefer": ["JSLint", "JSHint"], to "linting.prefer": ["ESLint"],
This has caused JSline to stop working from brackets code base when editing in brackets editor!
As we have not yet integrated eslint into the lint panel in brackets, brackets does not lint JS files now.

[core-ai-bot]

Comment by ficristo
Monday Dec 14, 2015 at 10:52 GMT

---

@abose You are right... I'm using the eslint plugin so I didn't saw it...
Meanwhile you can use grunt eslint which run already on travis.
For now it's a bit annoying, but adobe/brackets#11988 should fix this.
Waiting that PR is merged you could:

• reverting the two PR of ESLint: this seems more work than what it is worth
• reintroducing JSLint: this could interfere with ESLint rules
  What do you think?

[core-ai-bot]

Comment by abose
Monday Dec 14, 2015 at 11:17 GMT

---

We can add eslint also to the preferred linters for the time being. And have #11988 just have eslint alone. That would be a good transition.

[core-ai-bot]

Comment by ficristo
Monday Dec 14, 2015 at 11:34 GMT

---

We can add eslint also to the preferred linters for the time being

I'm not sure to have understood: do you mean changing the config to this: "linting.prefer": ["ESLint", "JSLint"] ?

[core-ai-bot]

Comment by abose
Monday Dec 14, 2015 at 11:41 GMT

---

I was thinking of something like that but now figured it doesn't work like that.
What will happen if we just have JSLint here for the time being till #11988 is merged?

[core-ai-bot]

Comment by ficristo
Monday Dec 14, 2015 at 11:48 GMT

---

When I written the PR to integrate ESLint I tried to use similar rules to the ones already there.
Using JSLint shouldn't create problems but there is a chance that they could interfere: JSLint could warn about something that's fine for ESLint or worse could not warn something that's not fine for ESLint.

[core-ai-bot]

Comment by abose
Monday Dec 14, 2015 at 13:07 GMT

---

What should we do in this case then?

[core-ai-bot]

Comment by zaggino
Monday Dec 14, 2015 at 19:35 GMT

---

I've merged@ficristo rollback PR and I'll add these changes to adobe/brackets#11988

[core-ai-bot]

Comment by peterflynn
Tuesday Dec 15, 2015 at 22:17 GMT

---

@abose@zaggino@ficristo Sorry for not speaking up sooner, but I have serious concerns about this change. Brackets itself (and CodeMirror in general) automatically generates whitespace on blank lines based on indent. This ESLint rule means we will be constantly fighting with Brackets's own output, which seems like a bad policy.

I would strongly suggest we change the ESLint setting to "no-trailing-spaces": [2, { "skipBlankLines": true }] so that we don't have that problem.

If I'd noticed this sooner I would have also suggested reverting all the changes that remove whitespace on blank lines, so that we don't have verbose diffs where people editing the code gradually start reintroducing all that indent whitespace... but I guess it's too late for that now :-/