[WIP] Default to ./ when no <src> and support - as stdin

[valeriangalliat]

• See Change behavior when no src #347

Usage:

sassdoc # Document the CWD
cat file.scss | sassdoc - # Document `stdin`
cat file.scss | sassdoc - src/ # Document both `stdin` and `src/` (we still support multiple `<src>`

This implies streamlining the behavior of sassdoc(src) and .pipe(sassdoc()), especially supporting recurse, exclude and converter in the streaming version.

While I think it's good to have the same behavior in both usages, it's maybe harmful for plugins usage?

Anyway, we're about to release 2.0 and this feature have not been heavily tested, so it's not a good idea to merge it right away.

Though, we need to be concerned about not breaking the API, that is, if we plan to have this default ./ and - support in 2.0, we need to at least add the default directory and drop the stdin support before releasing 2.0.

But I'm afraid the behavior streamlining between sassdoc(src) and .pipe(sassdoc()) can be considered as a breaking change too, and if we want this, we might not be able to use it until 3.0.

Note: according to #347, this PR is missing a default exclude pattern when using the CWD (to not documentize node_modules and other vendor directories, and I'd also avoid documenting the destination directory if it's contained in CWD, even if it's not supposed to contain Sass files).

Opinions?

[coveralls]

Coverage increased (+0.12%) to 88.0% when pulling ac4de8b on behavior-no-src into 205a03e on develop.

[pascalduez]

I'm highly concerned about such a change:

1
It hasn't been tested at all, while the current code base is.
And not just unit tests, manual testing, testing with plugins, etc.
It is now returning a Duplex Stream instead of a Transform Stream. While this should not be a problem, I don't have the deep understanding of Streams needed to be able to be affirmative.

2
The changes it implies to the stream API and for integration with tasks runners is much significant than it seems and not desired IMO. Let me try to explain.

Default API, aka documentize, is the no-brainer, care about all possible cases and scenarios process. And it's great, we want the CLI to be as straightforward as possible (e.g. being able to pass one or more directories as source, or a glob pattern, or several glob patterns, etc). We also care about converting Sass to SCSS if needed. No-brainer.

On the other end the stream API, which was primarily thought to be used with tasks runners is different and much more lighter, but flexible and versatile. That's what the successive refactors where for !
Gulp is the best current example, but it is not the only one.
Provided users are using Streams and/or Gulp we can assume they have a mere understanding
of what they are doing. In such I don't want to cut on flexibility and performances by surcharging the stream task.
recurse, exclude, convert are steps the CLI users have no way to choose or intervene on. While Gulp users have. They have total control over their streams pipeline. Passing a directory to gulp.src is close to a total non-sense. (mostly because most plugins don't care about directories cases like we do).
So why would we penalize them ?

3
For me the feature expected introducing this refactor is not worst the lost for plugins.

4
I understand that if we ship the current state it won't be possible to reassign the behaviour or $ sassoc later. Removing the stdin feature feels also a bit of a pity at that stage.

Possible solution
Why not shipping 2.0 with no default behavior (print usage infos), and set the stdin to work with - or --stdin ? This way we will have more time to think of it and still have to ability to add a default behaviour later.

[valeriangalliat]

I totally agree with you @pascalduez.

After some time away from this issue, I have a new idea on how to integrate this to the CLI without messing with the streaming API (I should really have thought of this earlier). In the CLI:

let filter = handler(env);
filter.promise.then(cb);

pipe(
  sources,
  recurse(),
  exclude(env.exclude || []),
  converter({ from: 'sass', to: 'scss' }),
  filter
).resume();

This way, there's no change to be done in the streaming API, and - is integrated seamlessly in SassDoc <src>... interface.

I'm also okay with just having a special case where one single <src> is - for stdin, and regular documentize usage otherwise. And it would take less code.

[pascalduez]

pipe(
  sources,
  recurse(),
  exclude(env.exclude || []),
  converter({ from: 'sass', to: 'scss' }),
  filter
).resume();

Looks great !
That's a bit what I tried to suggest yesterday, but surely failed to be clear enough 😕

[valeriangalliat]

I think I was also a bit stubborn with my idea and didn't accorded enough attention to your suggestions. :/

Anyway, it's all much clear after leaving the issue for a night and coming with a fresh mind again!