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

Make more logic the filter example in README #1

Closed
wants to merge 1 commit into from
Closed

Make more logic the filter example in README #1

wants to merge 1 commit into from

Conversation

MoOx
Copy link

@MoOx MoOx commented Jan 14, 2014

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

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
Copy link
Owner

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
Copy link
Author

MoOx commented Jan 14, 2014

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
Copy link
Owner

What are you getting and what did you expect?

@MoOx
Copy link
Author

MoOx commented Jan 14, 2014

jscs parse json files which return an error...

@sindresorhus
Copy link
Owner

@MoOx pushed a fix. mind trying out master?

@MoOx MoOx deleted the patch-1 branch January 15, 2014 10:36
@MoOx
Copy link
Author

MoOx commented Jan 15, 2014

Yes sure.

@MoOx
Copy link
Author

MoOx commented Jan 15, 2014

@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
Copy link
Owner

@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
Copy link
Author

MoOx commented Jan 16, 2014

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
Copy link
Owner

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

@MoOx
Copy link
Author

MoOx commented Jan 18, 2014

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
Copy link
Owner

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

@MoOx
Copy link
Author

MoOx commented Jan 19, 2014

❯ 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 added a commit that referenced this pull request Jan 19, 2014
@sindresorhus
Copy link
Owner

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

@MoOx
Copy link
Author

MoOx commented Jan 20, 2014

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)

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