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

[CLOSED] add ESLint as a default Brackets extension #10334

Open
core-ai-bot opened this issue Aug 30, 2021 · 18 comments
Open

[CLOSED] add ESLint as a default Brackets extension #10334

core-ai-bot opened this issue Aug 30, 2021 · 18 comments

Comments

@core-ai-bot
Copy link
Member

Issue by zaggino
Wednesday Dec 09, 2015 at 03:54 GMT
Originally opened as adobe/brackets#11988


ref: adobe/brackets#11984

mostly cut out from my extension https://github.com/zaggino/brackets-eslint


zaggino included the following code: https://github.com/adobe/brackets/pull/11988/commits

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Wednesday Dec 09, 2015 at 05:00 GMT


@petetnt@ficristo@abose I'll need help here with testing, right now I'm using this branch as my primary and everything seems to work fine for me but that might not be the case for all types of projects.

@core-ai-bot
Copy link
Member Author

Comment by petetnt
Wednesday Dec 09, 2015 at 14:00 GMT


I've been using this branch today and most things seem to work as expected. However, I am using eslint-plugin-react and when I launch Brackets, I get the following error:

Provider ESLint (async) failed: Error: Cannot find module 'eslint-plugin-react'

Despite this error, eslint and eslint-plugin-react work as expected after dismissing the error (by, for example, saving a .jsx file)

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Wednesday Dec 09, 2015 at 23:48 GMT


@petetnt adobe/brackets@634160c should fix that issue.

@core-ai-bot
Copy link
Member Author

Comment by petetnt
Thursday Dec 10, 2015 at 06:55 GMT


adobe/brackets@634160c fixed it, thanks!

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Thursday Dec 10, 2015 at 08:59 GMT


ESLint supports multiple ways to define en .eslintrc file:
http://eslint.org/docs/user-guide/configuring#configuration-file-formats
Does this PR supports all of them?

Style nit: I guess it should indented by 4 spaces.

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Monday Dec 14, 2015 at 23:08 GMT


@ficristo made a fix in adobe/brackets@7917a2d to consider all config file names

@ficristo fixed indentation to 4 spaces in adobe/brackets@d9faa73

@abose against my better judgement, committed the actual ESLint into the extension folder so npm install is not required after cloning the repository ...

let me guys know if you have any more issues with this

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Monday Dec 14, 2015 at 23:16 GMT


btw let me say again, we should really use npm install and not commit this to git, I get this error when switching branches on windows:

$ git checkout zaggino/eslint
fatal: cannot create directory at 'src/extensions/default/ESLint/node_modules/eslint/node_modules/handlebars/node_modules/uglify-js/node_modules/yargs/node_modules/cliui/node_modules/center-align/node_modules/align-text/node_modules/kind-of/node_modules/is-buffer/test': Filename too long

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Monday Dec 14, 2015 at 23:27 GMT


had to remove ESLint's code because of the issue above with long file names ... we should just use npm install

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Tuesday Dec 15, 2015 at 04:20 GMT


added this with npm3, no longer have the long file path problem ... node_modules occupy about 10MB of disk space this way ... that's 10MB more when cloning a repo which could be avoided by using npm

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Tuesday Dec 15, 2015 at 09:26 GMT


As shown by travis grunt eslint doesn't pass.
If I click on an error the cursor goes on the right row but on the wrong colum. (Should go on the previous one)
The dependencies in node_modules should be tracked in the package.json?

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Tuesday Dec 15, 2015 at 09:49 GMT


grunt eslint doesn't pass because this pr turns on indentation checks for 4 spaces and there're files outside of this PR not respecting this

@core-ai-bot
Copy link
Member Author

Comment by abose
Tuesday Dec 15, 2015 at 10:55 GMT


But the NPM install would also pull the same amount of code right. So if brackets have to be working, the node modules anyways need to be in place.
On second thought, wouldn't this add 10+mb to the installer size?

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Tuesday Dec 15, 2015 at 11:47 GMT


Yes, that's true. Installer size would definitely grow.

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Tuesday Dec 15, 2015 at 13:25 GMT


I don't see the point of the noop function, can you remove it?
Just leaving the comments // no action required should be enough.

It shouldn't matter much but maybe use regex.test instead of entry.match(/^\.eslintrc(\.(js|yaml|yml|json))?$/); since you aren't using the results.

It is safe this process.chdir(projectRoot); ? I assume it is since running in a node domain.

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Tuesday Dec 15, 2015 at 20:27 GMT


process.chdir(projectRoot); has been in the extension for months, never caused any problems ... so I assume it's safe, new version of eslint will support cwd parameter (eslint/eslint@eb1ef88) so this won't be required.

@core-ai-bot
Copy link
Member Author

Comment by ficristo
Wednesday Dec 16, 2015 at 11:00 GMT


I don't know if happened with JSLint too, but if I create a new project without a .eslintrc and then I add one I need to restart Brackets to make it take effect.

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Thursday Dec 17, 2015 at 01:34 GMT


@ficristo fixed in adobe/brackets@2b57d05

@core-ai-bot
Copy link
Member Author

Comment by zaggino
Tuesday Aug 16, 2016 at 04:02 GMT


This died due to lack of feedback.
Latest version currently available here https://github.com/zaggino/brackets-eslint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant