Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: Stream.CopyTo{Async} with count #102177

Open
virzak opened this issue May 13, 2024 · 3 comments
Open

[API Proposal]: Stream.CopyTo{Async} with count #102177

virzak opened this issue May 13, 2024 · 3 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.IO
Milestone

Comments

@virzak
Copy link
Contributor

virzak commented May 13, 2024

Background and motivation

I need to copy a part of a stream to another stream.

API Proposal

namespace System.IO;

public abstract class Stream
{
    public void CopyTo(Stream destination, long? count);
    public void CopyTo(Stream destination, long? count, int bufferSize);
    public Task CopyToAsync(Stream destination, long? count, CancellationToken cancel);
    public Task CopyToAsync(Stream destination, long? count, int bufferSize, CancellationToken cancel);
}

This functionality is already a part of dotnet org, being used by aspnet.core repo, but is currently internal:
https://github.com/dotnet/aspnetcore/blob/d731e85ab8961368a6c41453dcfa2ebfa251f01d/src/Http/Shared/StreamCopyOperationInternal.cs

It is also slightly different from the current Stream.CopyToAsync

API Usage

stream.CopyTo(dst, 23);

Alternative Designs

No response

Risks

None. The advantage is that aspnet.core would not need to have its own implementation.

BTW, the existence of such API seemed so obvious to me, that I just code searched GitHub for CopyToAsync in dotnet org and was not disappointed.

@virzak virzak added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 13, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 13, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

@adamsitnik
Copy link
Member

long?

Why would the argument be nullable if we have overloads that don't accept it for those who don't have the value?

public void CopyTo(Stream destination, long? count);
public Task CopyToAsync(Stream destination, long? count, CancellationToken cancel);

I am afraid that introducing these two overloads in the proposed form would cause a compiler error because the compiler would not know whether provided number is bufferSize or count:

source.CopyTo(dst, 100); // is 100 bufferSize or count?

public virtual void CopyTo(Stream! destination, int bufferSize);
public virtual Task! CopyToAsync(Stream! destination, int bufferSize, CancellationToken cancellationToken);

Nevertheless I see the problem you are trying to solve and I agree we should provide a solution for it.

@adamsitnik adamsitnik removed the untriaged New issue has not been triaged by the area owner label May 22, 2024
@adamsitnik adamsitnik added this to the Future milestone May 22, 2024
@virzak
Copy link
Contributor Author

virzak commented May 28, 2024

long? was copied from the internal aspnet implementation by mistake. Of course it should just be long.

A specific use case for this need was to copy a huge file while reporting IProgress back to the UI thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.IO
Projects
None yet
Development

No branches or pull requests

2 participants