sindresorhus / gulp-filter

Filter files in a `vinyl` stream
MIT License
315 stars 37 forks source link

Make more logic the filter example in README #1

Closed MoOx closed 10 years ago

MoOx commented 10 years ago

It makes no sense to only call jscs on src/vendor. The filter() is filtering the pattern you give to him, not the opposite.

sindresorhus commented 10 years ago

I think you're reading it wrong. This is not ignore. Filter filters out a subset of the files using globbing. In the example I filter out everything the vendor files. Would be happy to make that clearer though.

MoOx commented 10 years ago

Sorry I've an issue with a weird behavior (at least unexpected for me). You are right, but I'm missing something.

Here is a part of my gulpfile.

var jshint = require("gulp-jshint")
  , jscs = require("gulp-jscs")
  , jsFiles = [
      "*.js"
    , "*.json"
    , ".jshintrc"
    , ".csslintrc"
    , "./src/js/**/*.js"
  ]

gulp.task("lint-scripts", function() {
  gulp.src(jsFiles)
    .pipe(plumber())

    // dont jscs json files
    .pipe(filter(["!*.json", "!*rc"]))
    .pipe(jscs())
    .pipe(filter.end())

    .pipe(jshint(".jshintrc"))
    .pipe(jshint.reporter("jshint-stylish"))
})

The filtering is not working as expected. What I'm missing ?

sindresorhus commented 10 years ago

What are you getting and what did you expect?

MoOx commented 10 years ago

jscs parse json files which return an error...

sindresorhus commented 10 years ago

@MoOx pushed a fix. mind trying out master?

MoOx commented 10 years ago

Yes sure.

MoOx commented 10 years ago

@sindresorhus just got the same issue (I double check, I've correctly retrieved the new version using through2. Do you need more information from me ?

sindresorhus commented 10 years ago

@MoOx sorry, about that, check master again. I forgot you need to explicitly allow files with leading dots. I added a testcase for your usage.

MoOx commented 10 years ago

Still having issue. I just take a look & add some stupid console.log() just to see. It seems the issue is related to the fact that the file.path you use in the multimatch() call is something like /blah/.jshintrc & not .jshintrc. I've check with path.basename(file.path) it works better, but not sure if it's a good idea since the pattern can cover the filename... Maybe I need to use a better pattern ?

sindresorhus commented 10 years ago

If you need to match subfolders you can use "**/.jshintrc" instead

MoOx commented 10 years ago

I got your point, but it's doesn't really make sense to me since my .jshintrc & other json files I want to filter are at the same level of the gulpfile.

sindresorhus commented 10 years ago

@MoOx can you run tree so I can see your dir structure?

MoOx commented 10 years ago
❯ tree -a
.
├── .DS_Store
├── .csslintrc
├── .editorconfig
├── .git
├── .gitignore
├── .jscs.json
├── .jshintrc
├── Makefile
├── README.md
├── bower.json
├── gulpfile.js
├── package.json
└── src
    ├── .DS_Store
    ├── css
    │   ├── .DS_Store
    │   ├── body
    │   │   ├── index.css
    │   │   ├── nav.css
    │   │   └── page.css
    │   ├── entries
    │   │   └── index.css
    │   ├── index.css
    │   ├── reset
    │   │   └── button.css
    │   ├── styleguide
    │   │   └── button.css
    │   └── vendor
    │       ├── .DS_Store
    │       ├── fontello
    │       │   ├── animation.css
    │       │   ├── fontello-codes.css
    │       │   ├── fontello-embedded.css
    │       │   ├── fontello-ie7-codes.css
    │       │   ├── fontello-ie7.css
    │       │   └── fontello.css
    │       └── normalize.css
    ├── glyphicons
    │   ├── .DS_Store
    │   ├── config.json
    │   ├── fire.svg
    │   ├── plus.svg
    │   └── stats.svg
    ├── html
    │   └── index.jade
    ├── js
    │   ├── .DS_Store
    │   ├── api.js
    │   ├── collections
    │   ├── dom.js
    │   ├── index.js
    │   ├── models
    │   ├── routers
    │   │   └── index.js
    │   ├── storage.js
    │   └── views
    │       ├── 404.jade
    │       ├── 404.js
    │       ├── index.js
    │       ├── nav.jade
    │       ├── nav.js
    │       ├── templates
    │       │   └── entry.jade
    │       └── websites
    │           ├── index.jade
    │           └── index.js
    └── static
        ├── .DS_Store
        ├── font
        │   ├── .DS_Store
        │   ├── fontello.eot
        │   ├── fontello.svg
        │   ├── fontello.ttf
        │   └── fontello.woff
        └── img
            └── ss-Logo.png

The thing is, gulp is giving the full path to the plugin, so you don't have .jshintrc or similar but /full/path/to/.jshintrc :s

sindresorhus commented 10 years ago

@MoOx alright, i think i nailed it. mind trying out 0.2.1?

MoOx commented 10 years ago

Hey @sindresorhus thanks for the work done, but this still don't work (I still got issue on bower.json for example: Error: Syntax error at bower.json: Line 2: Unexpected token :)

/blah/node_modules/gulp-jscs/node_modules/jscs/lib/string-checker.js:209
            throw new Error('Syntax error at ' + filename + ': ' + e.message);
                  ^
Error: Syntax error at bower.json: Line 2: Unexpected token :
    at StringChecker.checkString (/blah/node_modules/gulp-jscs/node_modules/jscs/lib/string-checker.js:209:19)
    at Transform._transform (/blah/node_modules/gulp-jscs/index.js:23:24)
    at Transform._read (/blah/node_modules/gulp-jscs/node_modules/through2/node_modules/readable-stream/lib/_stream_transform.js:179:10)
    at Transform._write (/blah/node_modules/gulp-jscs/node_modules/through2/node_modules/readable-stream/lib/_stream_transform.js:167:12)
    at doWrite (/blah/node_modules/gulp-jscs/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:221:10)
    at writeOrBuffer (/blah/node_modules/gulp-jscs/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:211:5)
    at Transform.Writable.write (/blah/node_modules/gulp-jscs/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:180:11)
    at write (/blah/node_modules/gulp-filter/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:586:24)
    at flow (/blah/node_modules/gulp-filter/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:595:7)
    at Transform.pipeOnReadable (/blah/node_modules/gulp-filter/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:627:5)

Note I've sucessfully installed 0.2.1

gulp-jscs@0.2.1 node_modules/gulp-jscs
├── through2@0.4.0 (readable-stream@1.0.24, xtend@2.1.2)
└── jscs@1.2.3 (colors@0.6.0-1, vow@0.3.9, esprima@1.0.3, minimatch@0.2.12, xmlbuilder@1.1.2, glob@3.2.7, commander@1.2.0, vow-fs@0.2.3)