Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Fix: 'Fix on Save' should run only in valid file types #545

Merged
merged 2 commits into from
Apr 21, 2016

Conversation

pvamshi
Copy link
Contributor

@pvamshi pvamshi commented Apr 20, 2016

We already have valid scopes listed as

this.scopes = ['source.js', 'source.jsx', 'source.js.jsx', 'source.babel', 'source.js-semantic']

I am using the same array and comparing if the file type belongs to one of them .
With this fix, it is not running in all file types.
Please validate.

@IanVS
Copy link
Member

IanVS commented Apr 20, 2016

It really seems like this should be more flexible, since we can't predict what file extensions everyone will use for javascript. This is a good start to address the immediate bug, but I think a more flexible solution would be to lint (and fix) only .js files by default and provide an option for the user to configure others.

@pvamshi
Copy link
Contributor Author

pvamshi commented Apr 20, 2016

Probably that should be valid for general linting. So that would directly change this.scopes array and this fix is still valid even in that case .
Is my understanding correct ?

@Arcanemagus
Copy link
Member

This looks correct to me at least.

@IanVS I'm not sure what you are talking about here, this linter (and the changes in this PR) are based on the scope as Atom sees it, not the extension on the file.

Or is there a limitation on eslint's side that only pure JavaScript files can be fixed?

@pvamshi
Copy link
Contributor Author

pvamshi commented Apr 21, 2016

Can we please merge this if the fix looks fine ?

@IanVS
Copy link
Member

IanVS commented Apr 21, 2016

Ah ok, I assumed the scope was determined by the extension. Looks good to me then.

@Arcanemagus Arcanemagus merged commit f4cc378 into AtomLinter:master Apr 21, 2016
@Arcanemagus
Copy link
Member

That's essentially true for JavaScript, as the only place where I know of that source.js is valid is a pure .js file, but it's however Atom determines the scope really, which is controlled by the language packages.

@Arcanemagus
Copy link
Member

Published in v7.2.1. 🎉

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

Successfully merging this pull request may close these issues.

3 participants