SinkExt::feed

[mzabaluev]

Add SinkExt methods feed and feed_all. These are like send and send_all, except that the sink is not flushed at the end, allowing a sequence of output operations to be performed in async code without intermittent flushing. As a downside, the user is responsible for flushing or closing the sink to ensure completion.

Resolves #2059

[Ekleog]

@mzabaluev I've come here hoping for the same thing, but it looks like the CI build failed due to clippy warnings. Maybe that's why no one reviewed this yet?

If so, it looks like the clippy issues are probably not related to this PR, so hopefully just rebasing on latest master would fix the CI? :)

[mzabaluev]

@Ekleog I have rebased and pushed the branch after some cleanup, but the CI situation is the same.

[Ekleog]

Hmm that's weird, there's nothing in the diff that looks like it could be causing the raised issue… let's hope that someone with more knowledge of this repo comes and has a look, then, I guess :)

[taiki-e]

CI failure is due to (new) clippy bug: rust-lang/rust-clippy#5704

EDIT: will be fIxed in #2178

[taiki-e]

Sorry for the delay reviewing.

I think it makes sense to add feed method, but I'm not sure why feed_all method is needed. Could you explain the use case of feed_all? (IIUC, most of its use cases can be resolved with feed + loop or send_all, right?)

[mzabaluev]

@taiki-e

Could you explain the use case of feed_all?

1. For convenience of not having to read the source stream in a loop, in cases you want to send something more after the stream items without intermittent flushing;
2. It's easy to extract from the code of SendAll, maintaining symmetry with Send/Feed.

[taiki-e]

Sorry for the delayed reply.

• For convenience of not having to read the source stream in a loop, in cases you want to send something more after the stream items without intermittent flushing;
• It's easy to extract from the code of SendAll, maintaining symmetry with Send/Feed.

These do not seem to benefit more than the confusion of providing similar methods with subtle differences. (Also, what this patch is trying to add is subtly different from feed + loop.)
We can consider accepting these implementation changes as a patch to send_all, in 0.4, but I don't think it's worth adding a new API (feed_all) for this.

Could you remove the changes related to feed_all? (If you want to discuss the addition of feed_all to 0.3, please open a new issue.)

[mzabaluev]

Could you remove the changes related to feed_all? (If you want to discuss the addition of feed_all to 0.3, please open a new issue.)

I have pushed a rebased branch with just the changes for feed.

We can consider accepting these implementation changes as a patch to send_all, in 0.4

Is the plan to change the specification of send_all to not perform a complete flush at the end?
I think, for ease of use, it makes sense to retain the all-in-one method that's guaranteed to complete sending of all source items to the sink when the future it returns is resolved.

Also, what this patch is trying to add is subtly different from feed + loop.

This is actually part of why it makes sense to add as a method, just like send_all doing a more clever thing than send in a loop.

I will submit the send_all part in a separate PR.

[taiki-e]

Is the plan to change the specification of send_all to not perform a complete flush at the end?
I think, for ease of use, it makes sense to retain the all-in-one method that's guaranteed to complete sending of all source items to the sink when the future it returns is resolved.

I don't have any specific plans at this time, but I'm starting to think send_all doesn't necessarily have to do flush, given that write_all doesn't do flush.

(In any case, there are some issues in the current Sink trait, and in 0.4 the Sink trait itself may change.)

[taiki-e]

Thanks!