evanshortiss / lintspaces-cli

A CLI for the node-lintspaces module.
MIT License
18 stars 6 forks source link

Linting all files, ignore node_modules at any level #21

Open robwilkerson opened 7 years ago

robwilkerson commented 7 years ago

I have a project directory structure that looks a little like this:

/
|- client_module/
|   |- node_modules/
|   |- index.js
|   |_ etl.py
|- node_modules/
|- src/
|   |- tests/
|   |   |- integration/
|   |   |   |- my.test.js
|   |- routes/
|   |   |_ route1.js
|   |_ server.js
|_ test_client/
|   |- node_modules/
|   |_ index.js
|_ package.json

I'm trying to run the lintspacers-cli against all files that aren't in a node_modules directory at any level. So far, the closest I've been able to come is using the following:

lintspaces --editorconfig './.editorconfig' --ignores 'js-comments' .* * **/*

Unfortunately, that doesn't reach all the way down to src/routes/route1.js, much less to src/tests/integration/my.test.js. If I try to inject another globstar all of the node_modules directories are included.

lintspaces --editorconfig './.editorconfig' --ignores 'js-comments' .* * **/**/*

I've tried about a dozen other combinations and some included pattern negation, but in those attempts I get an error that either "!" or "(" is an unexpected token (I use zsh).

I'm hoping someone might be able to move me in the right direction with a pattern that will return everything I'm trying to lint and nothing that I'm not. So far I haven't had much luck on my own.

evanshortiss commented 7 years ago

Hi @robwilkerson I would have thought **/** would do it, but I don't use the glob patterns often.

How about something like this?

lintspaces --editorconfig './.editorconfig' --ignores 'js-comments' $(find . -name '*.js' | grep -v 'node_modules/')

This would find all JS files in your project and ignore anything in a node_modules directory. Alternatively you could do:

# find all .js files in the current folder and subfolders and pass to lintspaces. ignores node_modules, you will probably need to also pass "-not -path 'client_module/node_modules'"
lintspaces --editorconfig './.editorconfig' --ignores 'js-comments' $(find . -name '*.js' -not -path './node_modules/*')

As a sidenote, this seems like an unusual project structure? Shouldn't each module have a package.json if it has a node_modules directory?

/
|- client_module/
|   |- node_modules/
|   |- index.js
|   |_ etl.py
|- node_modules/
|- src/
|   |- tests/
|   |   |- integration/
|   |   |   |- my.test.js
|   |- routes/
|   |   |_ route1.js
|   |_ server.js
|_ test_client/
|   |- node_modules/
|   |_ index.js
|_ package.json
evanshortiss commented 7 years ago

@robwilkerson I misread your original intention. Basically, you're asking to make it a feature that we ignore node_modules?

This could be done I suppose, but I'm not convinced it's something the module should be concerned with. My main reasoning is that this module could be used for non JavaScript projects and that it's possible to ignore node_modules without changes to this module.

I like to use this structure below to keep my projects entirely self contained. Each has a package.json and it's own node_modules and package.json scripts to validate it.

eshortiss:/tmp/my-dummy-project$ tree -L 2
.
├── client
│   ├── index.js
│   ├── node_modules
│   ├── package.json
│   └── www
├── server
│   ├── index.js
│   ├── lib
│   ├── node_modules
│   └── package.json
└── tests
    ├── node_modules
    └── package.json

Each project here can have lintspaces installed in the package.json. Then in the package.json for each you could have something similar to this in the scripts:

"lint": "lintspaces --editorconfig './.editorconfig' index.js www/**"

This will use the locally installed lintspaces CLI (in node_modules/.bin/lintspaces) to run validations like so:

npm run lint
robwilkerson commented 7 years ago

@evanshortiss In this case, I was really just looking for thoughts on a pattern that might work. For some reason, no pattern I've tried seems to work. The problem with using find is...wait for it...some team members use Windows. sigh.

And yes, the structure is a bit unusual. I've inherited this code. In time, the client and test_module directories contain independent CLI tools and will be broken out, but I'd like to lint them until we're positioned to do so. At the moment, pure convenience keeps them with the main project.

I completely agree that this module shouldn't concern itself with the nature of any given project.

Thanks for your thoughts.

evanshortiss commented 7 years ago

Unlucky on the Windows front @robwilkerson 😛

So one thing I realised in all of this is that the way the glob module is used here is a little unusual. It's glob@7.X. When I used glob before you could do things like !node_modules/**, but it no longer supports that. Take a look here at the patterns.

Basically, to pass an advanced pattern to the internal glob module you'd need to surround it by quotes like so:

lintspaces -d tabs -s 4 '**/** !node_modules/**'

Using glob I had no luck since, as stated, it dropped ! support. So...I looked into an alternative glob module that actually supports ! functionality and found globby.

A quick test shows it working when I do the below it correctly ignores node_modules as one would expect:

# the quotes are necessary so we know this should be considered a full glob pattern!
# this pattern will ignore nested and root node_modules folders

lintspaces -s 6 -d tabs '* **/* !coverage/** !**/node_modules/**'

Perhaps I will switch the module to use globby. @jantimon added glob originally so I'd like his thoughts.

robwilkerson commented 7 years ago

If it doesn't create more problems than it solves, I think it'd be extremely useful to be able to negate patterns. I look forward to hearing what you all decide.

jantimon commented 7 years ago

globby is using glob internally and extends the desired features. I guess it would be a great improvement 👍