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

Add extensions option #149

Merged
merged 7 commits into from
Oct 7, 2016
Merged

Add extensions option #149

merged 7 commits into from
Oct 7, 2016

Conversation

abread
Copy link
Contributor

@abread abread commented Oct 4, 2016

Solves #117 (Allowing more file extensions)

Trying to detect whether the glob pattern restricts file extensions quickly gets out of hand. There are arrays of patterns, there can be negations, obscure regex syntax that may not be accounted for and that does limit what extensions can be used...

So, I added a new option: anyExtension (or --any-extension).
Set it to true, and the extension filter is disabled.

In my opinion, it would be better to just remove it altogether. Most people are probably just using the default expression and the ignores option, so impact should be minimal. A FAQ entry would be fine. Maybe this should happen for 1.0?

Turn it on and you can lint files with any extension.
That is, the filter ignoring all files that do not end
in .js or .jsx is disabled.
@sindresorhus
Copy link
Member

I think the correct solution would be to introduce an extensions option where users can specify additional extensions to include.

By default, xo filters out any non-js and non-jsx files. With this option you can add more extensions to the whitelist.
Just make sure ESLint supports that format, either directly or with a plugin.
@abread
Copy link
Contributor Author

abread commented Oct 5, 2016

Changed it to the extensions option. Should I change the default pattern to **/*? That way, someone could just enable more extensions and not need to change anything else. It shouldn't break anything since the filter has the usual .js/.jsx by default.

Copy link
Member

@sindresorhus sindresorhus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a test

@@ -42,6 +42,7 @@ const cli = meow(`
--extend Extend defaults with a custom config [Can be set multiple times]
--open Open files with issues in your editor
--quiet Show only errors and no warnings
--extension Additional extensions to lint [Can be set multiple times]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extensions -> extension

return ext === '.js' || ext === '.jsx';
// Remove dot before the actual extension
const ext = path.extname(x).replace('.', '');
return opts.extensions.indexOf(ext) > -1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Array#includes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered using it but it seems like it's only supported since Node 6 (or node 5 with a flag): http://node.green/#Array-prototype-includes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agh, right. Forgot that.

Use !== -1

@@ -197,6 +199,12 @@ Type: `Array`, `string`

Use one or more [shareable configs](http://eslint.org/docs/developer-guide/shareable-configs.html) to override any of the default rules (like `rules` above).

### extensions

Type: `Array`, `string`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should only be Array

@sindresorhus
Copy link
Member

Should I change the default pattern to */?

👍

@sindresorhus
Copy link
Member

And PR title needs to be updated

@abread abread changed the title Add anyExtension option (Allowing more file extensions) Add extensions option (Allowing more file extensions) Oct 5, 2016
extensions option takes in an array, not a string
--extension option adds one extension per use, not multiple
.indexOf(..) !== -1 instead of > -1
The filter removes the need to specify extensions manually.
@sindresorhus sindresorhus changed the title Add extensions option (Allowing more file extensions) Add extensions option Oct 7, 2016
@sindresorhus sindresorhus merged commit 3c42847 into xojs:master Oct 7, 2016
@sindresorhus
Copy link
Member

Looks great! Thanks @addobandre :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants