Skip to content

Fixing the clone method#9

Closed
nfroidure wants to merge 2 commits intogulpjs:masterfrom
nfroidure:master
Closed

Fixing the clone method#9
nfroidure wants to merge 2 commits intogulpjs:masterfrom
nfroidure:master

Conversation

@nfroidure
Copy link
Copy Markdown
Contributor

Unfortunately, when cloning the file objects, we cannot keep the same stream for them since streams are synchronous when consuming them synchronously.

The added test would fail is there weren't the related changes in index.js

@yocontra
Copy link
Copy Markdown
Member

@nfroidure Do you think you should be able to file.clone() after the stream has been consumed (partially or completely) and expect a fresh stream?

@nfroidure
Copy link
Copy Markdown
Contributor Author

I think that when i clone the file here nfroidure@313b02a#diff-e42bc92107d70f0ca365cd690d0406a5R208, no data is consumed yet.

It just consume the streams at different moments nfroidure@313b02a#diff-e42bc92107d70f0ca365cd690d0406a5R221 .

I think that consuming the stream of 2 different files at a different moment will be a common use case and users will expect it to work.

@yocontra
Copy link
Copy Markdown
Member

@nfroidure But I'm asking - should you be able to consume file.contents multiple times? If you try to consume it and it's already emptied we could start from the beginning

@nfroidure
Copy link
Copy Markdown
Contributor Author

Do you mean consuming the contents of the stream of a file two times disregarding it's cloned or not? If so, i don't think it isn't possible without buffering it all and i think it is not suitable.

@yocontra
Copy link
Copy Markdown
Member

nfroidure I think with fs streams you can start over by calling .read(0) on it

@nfroidure
Copy link
Copy Markdown
Contributor Author

Does it work even if the are multiple pipes? Like that:

fs.createReadStream('file.txt).pipe(stream1).pipe(stream2);
stream2.read(0);

Another problem is that when i consume a stream that just has been cloned, how can i know if the stream is consuming/consumed to be able to choose to replay it or just consume it?

In fact, I cannot figure out how this is related to the issue.

nfroidure added a commit to nfroidure/gulp-svg2ttf that referenced this pull request Jun 28, 2014
nfroidure added a commit to nfroidure/gulp-ttf2eot that referenced this pull request Jun 28, 2014
nfroidure added a commit to nfroidure/gulp-ttf2woff that referenced this pull request Jun 28, 2014
@popomore
Copy link
Copy Markdown
Contributor

@contra How about this PR, should I clone stream or set an option for this.

@yocontra
Copy link
Copy Markdown
Member

@popomore It should clone the stream via dereferencing (what this PR does). Just pipe to an empty passthrough stream and return the passthrough stream

@yocontra
Copy link
Copy Markdown
Member

I'll merge a PR for this on the current codebase. @popomore or @nfroidure whoever wants to submit one

@yocontra yocontra closed this Aug 29, 2014
@popomore
Copy link
Copy Markdown
Contributor

I'll rebase this PR for you 😄

@popomore popomore mentioned this pull request Aug 29, 2014
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.

3 participants