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

sourceComments option gets ignored if called by gulp.watch #234

Closed
hashworks opened this issue Apr 9, 2015 · 9 comments · May be fixed by #286
Closed

sourceComments option gets ignored if called by gulp.watch #234

hashworks opened this issue Apr 9, 2015 · 9 comments · May be fixed by #286
Milestone

Comments

@hashworks
Copy link

Example:

var gulp = require('gulp');
var sass = require('gulp-sass');

gulp.task('sass', function() {
    gulp.src('**/*.{scss,sass}')
        .pipe(sass({
            sourceComments: true,
            outputStyle: 'expanded',
            errLogToConsole: true
        }))
        .pipe(gulp.dest('screen.css'));
});

gulp.task('watch', ['sass'], function() {
    var sassWatcher = gulp.watch('**/*.{scss,sass}', ['sass']);
});

When I call gulp sass it generates source comments, as expected. But when I run gulp watch no source comments get generated.

@Snugug
Copy link
Collaborator

Snugug commented Apr 9, 2015

Please try with the latest version:

npm install gulp-sass@next --save-dev

@hashworks
Copy link
Author

Same result.

@Snugug Snugug added this to the 2.0.0 milestone Apr 9, 2015
@Snugug Snugug added the bug label Apr 9, 2015
@Snugug
Copy link
Collaborator

Snugug commented Apr 30, 2015

I cannot replicate your error using the latest version of gulp-sass, in fact. I very clearly see the comments, including on watch, but it brings up an interesting question we should resolve.

Because we're passing a string to node-sass and not a path, the sourceComment says it's coming from stdin for anything coming from the main file. Should we replace this with the path to that file before passing passing back along @dlmanning @Keats? process.cwd() replaced below for the absolute path to the Gulp file.

/* line 1, {{process.cwd()}}/sass/_foo.scss */
.banana {
  content: 'waldo'; }

/* line 3, stdin */
.foo {
  content: 'bar'; }

/* line 7, stdin */
.bar {
  content: 'baz'; }

/* line 11, stdin */
.baz {
  content: 'baz'; }

/* line 15, stdin */
.qux {
  content: 'qux'; }

@Snugug Snugug added enhancement and removed bug labels Apr 30, 2015
@Snugug
Copy link
Collaborator

Snugug commented Apr 30, 2015

I'm also happy to defer this to 2.1 as most people seem to be interested in Source Maps over Comments and I don't want to block the 2.0 release on this

@hashworks
Copy link
Author

I'm ok with moving this to 2.1. I'll take a look at this again, I could bet in my case there where no source comments at all.

@Keats
Copy link
Collaborator

Keats commented Apr 30, 2015

I didn't even know that was a thing (I use compressed output with no sourcemaps/anything).
We can definitely work on that for 2.1

@aphex
Copy link

aphex commented May 25, 2015

For those of you, like me, who are to impatient to wait for a fix you can hack around this by tapping into the stream with gulp-tap and just doing plain ol' string replacement. It will likely slow things down as we have to toString the buffer then convert it back but it gets you out of a bind.

gulp.src(config.files.scss)
.pipe(sass({
    sourceComments:true
}))
.pipe(tap(function(file, t) {
    file.contents = new Buffer(file.contents.toString().replace(/stdin \*\//g, file.path + ' */'));
}))
.pipe(concat('app.css'))
}))

@xzyfer
Copy link
Collaborator

xzyfer commented May 10, 2016

This appears to be fix in v2.3.1.

@xzyfer xzyfer closed this as completed May 10, 2016
@alehro
Copy link

alehro commented Aug 22, 2016

In v2.3.2 it still doesn't work.

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

Successfully merging a pull request may close this issue.

6 participants