native/subscription/unfold: terminate stream when user returns None

[fengalin]

Context

While experimenting with iced for a WIP application, I ran into issues with the subscription::unfold behaviour. This MR proposes a change to the API which would solve my issues and which I believe would be preferable for iced in general.

Note: I read the contributing guidelines. Since this seemed like a small improvement to me, I didn't feel the need to introduce myself on the Discord server. In order to reflect on this I had to experiment to the extent of this MR anyway. Please tell me if I was wrong!

Analysis

futures::stream::unfold builds a Stream from the provided seed of type T and a Future returned by the f function. The resulting Stream produces values as long the Future returns Some tuple (Item, T). If the Future returns None, unfold is supposed to stop producing items.

In iced, the subscription::unfold function is based on the stream::unfold. However the Output of the Future is defined as (Option<Message>, T). The Future can return a None Message and the Stream will keep running.

The above behaviour is exemplified in examples/download_progress. When the download is finished (either because it is complete or if an error occurred), the Future is still being called. Adding a println statement shows that the Future is called repeatedly. The original code added a pending().await as a workaround. This means that the executor Tasks is kept in queues even if it will no longer make progress.

The first commit removes the pending().await workaround in download_progress and adds stdout & stderr traces so as to assess the impact of the second commit.

Proposal

One way to solve this would be to keep the existing signature for subscription::unfold and to map the Future output in such a way that it return None if the provided Option<Message> is None. However, this would be inconsistent with the unfold function from the futures crate and it wouldn't be suitable since the state is no longer usable after the Stream
completes.

This MR aligns the subscription::unfold function with its futures counterpart. In the download_progress example, after a download is finished, the Task in no longer kept in queues.

This changes a bit the implementation for the websocket example. Previous implementation returned from the Future with a None message and an unchanged state when the websocket or input were awaken but the result didn't need to be propagated. With this MR, in such cases the implementation loops on the wbesocket & input async select awaiting for the next message to be propagated.

The last commit is a small unrelated improvement to the download_progress example.

[n1ght-hunter]

couldn't you just put a simple if statement that when your subscription is finished you no longer return it?

 if self.continue_websocket {
    echo::connect().map(Message::Echo)
} else {
    iced::subscription::Subscription::none()
}

[fengalin]

Thanks for the suggestion @Night-Hunter-NF. I faced the issue addressed in this PR while already doing this.

[hecrj]

While the changes at first glance seem reasonable and consistent, the main issue here is that a Subscription should not be allowed to terminate. The Application is the only one capable of dictating which subscriptions are born and terminated.

Therefore, exposing an API that seems to indicate subscriptions can "end" by themselves seems misleading. It is true that we are already exposing Stream in Recipe and you can definitely return a Stream that terminates there; but that API is considered relatively internal (or "advanced").

[hecrj]

This MR proposes a change to the API which would solve my issues

I'd like to know more about these issues. Is there something you need to do that the current unfold helper doesn't allow?

[hecrj]

I believe the main issue here is that unfold should not have an Option at all to begin with. This makes it seem that returning None actually terminates the Subscription when all it does is loop.

I have made that change in #1786 and introduced a new helper channel that should simplify state management a bunch when compared to unfold.