lazd / gulp-csslint

CSSLint plugin for gulp
MIT License
74 stars 12 forks source link

What is wrong with v0.2.0? #35

Closed Igloczek closed 8 years ago

Igloczek commented 9 years ago

Hi, I just upgraded from 0.1.5 to 0.2.0... and my console blows up. Total mess, lots of CSS code instead of errors and warnings, all without any formating, just plain text, totaly unreadable.

I'm not using any special formaters, just bulk run without any parameters.

Also on some files I get this error

         output += reporter.formatResults(file.csslint.originalReport, file.p
                  ^
RangeError: Invalid string length

PS. I'm not only one, my teammates have same problems.

lazd commented 9 years ago

@Igloczek please paste your gulpfile.js.

cc @SimenB

SimenB commented 9 years ago

Plain text is by design, the only supported formatters are the ones built-in to CSSLint (https://github.com/lazd/gulp-csslint#using-reporters). Hopefully #29 should allow you to use others when merged

The error seems weird though... Do you have a big amount of css-files?

@lazd I suppose we could push to an array and join them later to avoid string concatenation. Think that might help?

Igloczek commented 9 years ago
var gulp = require('gulp');
var csslint = require('gulp-csslint');

var paths = {
    template: '../app/design/frontend/snowdog/v2/**',
    sass: '../skin/frontend/snowdog/v2/scss',
    css: '../skin/frontend/snowdog/v2/css',
    scripts: '../skin/frontend/snowdog/v2/js/dev',
    custom: '../skin/frontend/snowdog/v2/js/dev/*.js',
    libs: '../skin/frontend/snowdog/v2/js/dev/lib/*.js',
    build: '../skin/frontend/snowdog/v2/js/build'
};

...

// lint all css files
gulp.task('css-lint', function() {
    return gulp.src(paths.css + '/*.css')
        .pipe(csslint())
        .pipe(csslint.reporter());
});

https://gist.github.com/Igloczek/f4676f7337ec41ec0413 - here is whole setup (temporary with fallback to 0.1.5)

Sample run with full error - https://gist.github.com/Igloczek/3a26dfead9f5b7e5a17b

Sample run with single file and some weird output - https://gist.github.com/Igloczek/36120cc44f0912c3b38d

gist looks "ok", but when you get this in console... Looks like you are print whole line with error after message - killer feature when you have minified/compressed CSS :) https://www.dropbox.com/s/8vqo47ro3tukjzm/Zrzut%20ekranu%202015-09-10%2021.46.57.png?dl=0

0.2.0 compared with 0.1.5 looks awful - https://www.dropbox.com/s/7uqxsdq6jym9niz/Zrzut%20ekranu%202015-09-10%2021.49.51.png?dl=0

About CSS quantity - I have 12 files, 1.1MB.

SimenB commented 9 years ago

Could you try to chuck this in node_modules/gulp-csslint/index.js? https://github.com/SimenB/gulp-csslint/commit/b85012cf445605cb254cf61d2b8bc928b94d7999 Hopefully it should fix the error you're seeing.

Regarding outputting the line without the full line, that's what the text-formatter (default) of CSSLint does. https://github.com/CSSLint/csslint/blob/master/src/formatters/text.js#L67 Doesn't seem to provide an option either for limiting it. I suppose we could manually unset the evidence if provided an option. Seems hackish though

Igloczek commented 9 years ago

Little bit better (previously hangs on "home.css"), but still crashing :(

[.tools] gulp css-lint                                                                                                                                                     22:14:55  ☁  feature/15239 ☂ ⚡
[22:15:28] Using gulpfile ~/Sites/eobuwie/.tools/gulpfile.js
[22:15:28] Starting 'css-lint'...
[gulp] ../skin/frontend/snowdog/v2/css/404.css
[gulp] ../skin/frontend/snowdog/v2/css/account.css
[gulp] ../skin/frontend/snowdog/v2/css/category.css
[gulp] ../skin/frontend/snowdog/v2/css/checkout.css
[gulp] ../skin/frontend/snowdog/v2/css/cms.css
[gulp] ../skin/frontend/snowdog/v2/css/email-inline.css
[gulp] ../skin/frontend/snowdog/v2/css/email-non-inline.css
[gulp] ../skin/frontend/snowdog/v2/css/home.css
[gulp] ../skin/frontend/snowdog/v2/css/product.css
[gulp] ../skin/frontend/snowdog/v2/css/rma.css
[gulp] ../skin/frontend/snowdog/v2/css/styles.css
/Users/macbookair/Sites/eobuwie/.tools/node_modules/gulp-csslint/index.js:120
        gutil.log(output.join(''));
                         ^
RangeError: Invalid string length
    at Array.join (native)
    at DestroyableTransform._flush (/Users/macbookair/Sites/eobuwie/.tools/node_modules/gulp-csslint/index.js:120:26)
    at DestroyableTransform.<anonymous> (/Users/macbookair/Sites/eobuwie/.tools/node_modules/gulp-csslint/node_modules/through2/node_modules/readable-stream/lib/_stream_transform.js:123:12)
    at DestroyableTransform.g (events.js:199:16)
    at DestroyableTransform.emit (events.js:104:17)
    at prefinish (/Users/macbookair/Sites/eobuwie/.tools/node_modules/gulp-csslint/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:492:12)
    at finishMaybe (/Users/macbookair/Sites/eobuwie/.tools/node_modules/gulp-csslint/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:500:7)
    at endWritable (/Users/macbookair/Sites/eobuwie/.tools/node_modules/gulp-csslint/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:512:3)
    at DestroyableTransform.Writable.end (/Users/macbookair/Sites/eobuwie/.tools/node_modules/gulp-csslint/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:477:5)
    at DestroyableTransform.onend (/Users/macbookair/Sites/eobuwie/.tools/node_modules/gulp-csslint/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:539:10)
lazd commented 9 years ago

@Igloczek what are your machine specs and node version? That is basically an out of memory error...

Igloczek commented 9 years ago

I'm using MB Air mid 2012 i7 8GB OS X 10.10.5. Node v0.12.7. Currently activity monitor shows I'm using only 3.5GB.

lazd commented 9 years ago

@Igloczek you should be fine... This is a really odd one. @SimenB maybe we need to print the output as we get it instead of building a super long string.

SimenB commented 9 years ago

@Igloczek Could you try with https://github.com/SimenB/gulp-csslint/commit/6be5f312e3af50b9300e8065e4c6d5fe642f3025? Builds upon the last one

@lazd We do not necessarily get a string from each invocation of formatResults... Removing the evidence string might work, but that makes us differ from the standard output

Igloczek commented 9 years ago

Yeah, that's works fine. "compact" reporter format looks close to 0.1.5 output. Is there a way to get same/comparable formating?

SimenB commented 9 years ago

If you provide a function to reporter instead of a string, it's passed the report object from CSSLint per file. Then you could look at the old reporter https://github.com/lazd/gulp-csslint/commit/f6dc009c46658aff32253e4bfda9bee468ea632c#diff-168726dbe96b3ce427e7fedce31bb0bcL82

Currently working on allowing custom formatters, like grunt allows now

Igloczek commented 9 years ago

Thanks for help! With custom formatter work like a charm.

ffoodd commented 8 years ago

I got the same error but am not able to follow your conversation, would it be possible to get an example of a custom formatter that works?

Thanks :)

SimenB commented 8 years ago

@ffoodd https://github.com/lazd/gulp-csslint/blob/master/README.md#custom-reporters

Igloczek commented 8 years ago

@ffoodd You can use mine :)

var gutil = require('gulp-util');

...

function customReporter(file) {
  gutil.log(gutil.colors.cyan(file.csslint.errorCount) + ' errors in ' + gutil.colors.magenta(file.path));

  file.csslint.results.forEach(function(result) {
    if (result.error.type === 'warning') {
      gutil.log( gutil.colors.yellow.bold('[Warining]') + gutil.colors.green(' Line: ' + result.error.line) + gutil.colors.cyan(' Column: ' + result.error.col) + ' ' + gutil.colors.magenta(result.error.message) + ' ' +  gutil.colors.gray(result.error.rule.desc) + ' ' + gutil.colors.red('Browsers: ' + result.error.rule.browsers));
    }
    else {
      gutil.log( gutil.colors.red.bold('[' + result.error.type + ']') + gutil.colors.green(' Line: ' + result.error.line) + gutil.colors.cyan(' Column: ' + result.error.col) + ' ' + gutil.colors.magenta(result.error.message) + ' ' +  gutil.colors.gray(result.error.rule.desc) + ' ' + gutil.colors.red('Browsers: ' + result.error.rule.browsers));
    }
  });
}

...

// lint all css files
gulp.task('css-lint', function() {
  return gulp.src(paths.css)
    .pipe(csslint())
    .pipe(csslint.reporter(customReporter));
});
ffoodd commented 8 years ago

Thanks a lot, I'll try it :)

lazd commented 8 years ago

@Igloczek @ffoodd is this still an issue? Can we close this?

lazd commented 8 years ago

I think this is fixed in 0.2.2. Please test and reopen if not!

SimenB commented 8 years ago

@lazd If you refer to the fact that we now join the array, I tried that in https://github.com/lazd/gulp-csslint/issues/35#issuecomment-139366439, and it didn't help

ffoodd commented 8 years ago

Using @Igloczek 's custom reporter did the job for me so it's not an issue anymore :) Thanks.

SimenB commented 8 years ago

That'll break again after #29. I can probably just make a minimal reporter after it's merged, though

Igloczek commented 8 years ago

Minimal reporter is good way. For me, with custom formater, everything works fine.

lazd commented 8 years ago

Fix released in 0.3.0!