Add ability to pipe SassDoc into `stdin` #315

Merged
merged 4 commits into from Jan 9, 2015

Conversation

Projects
None yet
3 participants
@pascalduez
Member

pascalduez commented Jan 9, 2015

Introduce a new -s --stdin flag to make it clear/intentional ?

Example:
cat test/data/test.scss | bin/sassdoc -vs

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jan 9, 2015

Member

Awesome!

I would rather remove the --stdin option and use stdin when src is not given. I think it's intentional enough, I don't see a case where you unintentionally ommit the source directory, you at least know you want to documentize something.

Member

valeriangalliat commented Jan 9, 2015

Awesome!

I would rather remove the --stdin option and use stdin when src is not given. I think it's intentional enough, I don't see a case where you unintentionally ommit the source directory, you at least know you want to documentize something.

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jan 9, 2015

Member

Like @valeriangalliat said. We can assume that the omission of <src> is a clear sign for wanting to read from stdin.

Member

FWeinb commented Jan 9, 2015

Like @valeriangalliat said. We can assume that the omission of <src> is a clear sign for wanting to read from stdin.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Jan 9, 2015

Member

Okay for removing --stdin but I'm currently trying to push things even further with stdout.

No Gulp pipeline:

cat file.scss | bin/sassdoc -s | sass -s --scss --sourcemap=none test.css
Member

pascalduez commented Jan 9, 2015

Okay for removing --stdin but I'm currently trying to push things even further with stdout.

No Gulp pipeline:

cat file.scss | bin/sassdoc -s | sass -s --scss --sourcemap=none test.css
@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jan 9, 2015

Member

It's interesting, but I really don't believe it's a "normal" use case. It makes no sense to me to both documentize and compile SCSS in the same pipeline, and it gives the impression that SassDoc is filtering the stream, which is not the case.

Additionnally it don't allows parallel running, while defining this with, for example, two distinct makefile tasks could be parallelized.

Member

valeriangalliat commented Jan 9, 2015

It's interesting, but I really don't believe it's a "normal" use case. It makes no sense to me to both documentize and compile SCSS in the same pipeline, and it gives the impression that SassDoc is filtering the stream, which is not the case.

Additionnally it don't allows parallel running, while defining this with, for example, two distinct makefile tasks could be parallelized.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Jan 9, 2015

Member

If I'm not mistaken, it's similar to:

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

gulp.task('styles', function () {
  return gulp.src('./file.scss')
    .pipe(sassdoc({ verbose: true }))
    .pipe(sass());
});

It should also be able to:

sassdoc src | sass -s --scss --sourcemap=none src
Member

pascalduez commented Jan 9, 2015

If I'm not mistaken, it's similar to:

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

gulp.task('styles', function () {
  return gulp.src('./file.scss')
    .pipe(sassdoc({ verbose: true }))
    .pipe(sass());
});

It should also be able to:

sassdoc src | sass -s --scss --sourcemap=none src
@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jan 9, 2015

Member

Yep. I don't use Gulp so I don't know if it makes sense, but for sure it looks odd in a shell pipeline.

Member

valeriangalliat commented Jan 9, 2015

Yep. I don't use Gulp so I don't know if it makes sense, but for sure it looks odd in a shell pipeline.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Jan 9, 2015

Member

Not 100% sure it makes sense, but that was one of the goal to the whole stream refactor, being able to pipe SassDoc in a Gulp pipeline, without requiring a dedicated task. That's why we pass trough the files also.

Member

pascalduez commented Jan 9, 2015

Not 100% sure it makes sense, but that was one of the goal to the whole stream refactor, being able to pipe SassDoc in a Gulp pipeline, without requiring a dedicated task. That's why we pass trough the files also.

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jan 9, 2015

Member

@pascalduez and @valeriangalliat you are both right. In the context of gulp this is perfect but behaviour like this isn't expected on the shell pipe. Nevertheless it is not that bad to have it.

Member

FWeinb commented Jan 9, 2015

@pascalduez and @valeriangalliat you are both right. In the context of gulp this is perfect but behaviour like this isn't expected on the shell pipe. Nevertheless it is not that bad to have it.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Jan 9, 2015

Member

Okay guys, no stdout !!

Member

pascalduez commented Jan 9, 2015

Okay guys, no stdout !!

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jan 9, 2015

Member

Right, all good there then?

Member

valeriangalliat commented Jan 9, 2015

Right, all good there then?

pascalduez added a commit that referenced this pull request Jan 9, 2015

Merge pull request #315 from SassDoc/stdin-support
Add ability to pipe SassDoc into `stdin`

@pascalduez pascalduez merged commit 764c799 into develop Jan 9, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@pascalduez pascalduez deleted the stdin-support branch Jan 9, 2015

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