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 --noglob cli option #4286

Closed
wants to merge 2 commits into from
Closed

Add --noglob cli option #4286

wants to merge 2 commits into from

Conversation

AzazKamaz
Copy link

Closes #4193 and is non-breaking alternative for #4254 that can be accepted in minor release.
Tests and flow types have been implemented

system-tests/cli/cli.test.js Outdated Show resolved Hide resolved
vankop
vankop previously approved these changes Sep 20, 2019
Copy link
Member

@vankop vankop left a comment

Choose a reason for hiding this comment

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

I think this work is enough.

However there is some usage of globby options still, see https://github.com/stylelint/stylelint/blob/master/lib/standalone.js#L202

Maybe we need to use process.cwd() explicitly it such cases

@alexander-akait
Copy link
Member

I am 👎 on this, solution should be #4193 (comment), not new option for CLI

@@ -170,7 +170,7 @@ module.exports = function(
fileList = [fileList];
}

if (!options.disableDefaultIgnores) {
if (!options.disableDefaultIgnores && !options.noglob) {
Copy link
Member

Choose a reason for hiding this comment

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

In this case, ALWAYS_IGNORED_GLOBS is not working as expected with noglob, it is not good idea

@vankop
Copy link
Member

vankop commented Sep 23, 2019

@evilebottnawi So, if I understand you correctly, you suggest to use kind of heuristic:

  • if there is a file in file system using options.files as relative/absolute path, then we just lint this file
  • otherwise expect globby pattern

or there is another idea?

@vankop
Copy link
Member

vankop commented Sep 23, 2019

eslint use this code

@alexander-akait
Copy link
Member

Code from eslint looks good, i think we should use this logic too

@vankop
Copy link
Member

vankop commented Sep 24, 2019

Any other suggestions?

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

Successfully merging this pull request may close these issues.

Allow to use full file path without converting it to glob
4 participants