Skip to content

[API Proposal]: File.AppendAllBytes #84532

Closed

Description

@Symbai

Background and motivation

The File class has all methods for bytes which also exist for text except of appending data. Not only that it fits into the api surface by filling this gap is also saves us writing more than one line of code.

API Proposal

namespace System.IO;

public class File
{
+    public static void AppendAllBytes(string path, ReadOnlySpan<byte> data);
+    public static Task AppendAllBytesAsync(string path, ReadOnlyMemory<byte> data, CancellationToken cancellationToken = default);
     public static void AppendAllText(string path, string? contents);
     public static Task AppendAllTextAsync(string path, string? contents, CancellationToken cancellationToken = default);
}

API Usage

byte[] data = File.ReadAllBytes("C:\\1.bin");

File.AppendAllBytes("C:\\2.bin", data);

Alternative Designs

No response

Risks

No response

Activity

  1. added
    api-suggestionEarly API idea and discussion, it is NOT ready for implementation
    on Apr 8, 2023
  2. ghost added
    needs-area-labelAn area label is needed to ensure this gets routed to the appropriate area owners
    on Apr 8, 2023
  3. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Apr 8, 2023
  4. added and removed
    needs-area-labelAn area label is needed to ensure this gets routed to the appropriate area owners
    on Apr 8, 2023
  5. gfoidl commented on Apr 11, 2023

    @gfoidl
    Member

    The data argument should be of type ReadOnlySpan<byte> instead of byte[]??

  6. Symbai commented on Apr 11, 2023

    @Symbai
    Author

    The data argument should be of type ReadOnlySpan<byte> instead of byte[]??

    WriteAllBytes uses byte[] instead of span, thats why I used it here as well but yeah sure it can be a span.

  7. adamsitnik commented on May 19, 2023

    @adamsitnik
    Member

    Few comments from my side:

    • Why the API should allow for nulls? What is the point of appending nothing to a file? Creating it? If so, we have dedicated API for that.
    • We need an async overload too.
    • The mentioned APIs were introduced before Span and Memory, the new methods should accepts ReadOnlySpan and ReadOnlyMemory to accept both arrays and other types of inputs.
    namespace System.IO;
    
    public class File
    {
    +    public static void AppendAllBytes(string path, ReadOnlySpan<byte> data);
    +    public static Task AppendAllBytesAsync(string path, ReadOnlyMemory<byte> data, CancellationToken cancellationToken = default);
    }

    Having said that, I am going to update the proposal and make it as ready for review.

  8. added
    api-ready-for-reviewAPI is ready for review, it is NOT ready for implementation
    and removed
    api-suggestionEarly API idea and discussion, it is NOT ready for implementation
    untriagedNew issue has not been triaged by the area owner
    on May 19, 2023
  9. self-assigned this
    on May 19, 2023
  10. Symbai commented on May 19, 2023

    @Symbai
    Author
    • Why the API should allow for nulls? What is the point of appending nothing to a file?

    The AppendAllText allows a nullable string I only followed this choice.

    public static void AppendAllText(string path, string? contents)
    => AppendAllText(path, contents, UTF8NoBOM);

    Possibly because of that:

    using SafeFileHandle fileHandle = OpenHandle(path, mode, FileAccess.Write, FileShare.Read, FileOptions.None, GetPreallocationSize(mode, contents, encoding, preambleSize));
    long fileOffset = mode == FileMode.Append && fileHandle.CanSeek ? RandomAccess.GetLength(fileHandle) : 0;
    if (string.IsNullOrEmpty(contents))
    {
    if (preambleSize > 0 // even if the content is empty, we want to store the preamble
    && fileOffset == 0) // if we're appending to a file that already has data, don't write the preamble.
    {
    RandomAccess.WriteAtOffset(fileHandle, preamble, fileOffset);
    }
    return;
    }

  11. stephentoub commented on May 19, 2023

    @stephentoub
    Member

    Why the API should allow for nulls? What is the point of appending nothing to a file? Creating it? If so, we have dedicated API for that.

    There's value in being consistent with the existing APIs here. If someone is using WriteAllText with something that ends up being null and they decide to switch to appending rather than overriding, it'd be nice if that didn't then entail having to handle the arguments differently.

    The mentioned APIs were introduced before Span and Memory, the new methods should accepts ReadOnlySpan and ReadOnlyMemory to accept both arrays and other types of inputs.

    Do we want ReadOnly{Span/Memory}<byte> overloads then for the existing ones? What about ReadOnlySpan<char> for the ones currently only accepting strings?

  12. 9 remaining items

  13. MojtabaTajik commented on Jul 31, 2023

    @MojtabaTajik
    Contributor

    @adamsitnik @stephentoub Can I work on this API implementation?

  14. adamsitnik commented on Jul 31, 2023

    @adamsitnik
    Member

    @adamsitnik can you assign this issue to me?

    @AlexRadch please excuse me, for some reason I've missed the notification. Since you still have some actionable PRs opened (example: #88009 (review)), I am going to assign @MojtabaTajik. I hope you don't mind.

  15. adamsitnik commented on Jul 31, 2023

    @adamsitnik
    Member

    @adamsitnik @stephentoub Can I work on this API implementation?

    @MojtabaTajik I've assigned you, please let me know if you need some help.

  16. added this to the Future milestone on Jul 31, 2023
  17. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on Aug 11, 2023
  18. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Sep 19, 2023
  19. tcortega commented on Sep 30, 2023

    @tcortega
    Contributor

    Hi @adamsitnik and @MojtabaTajik,

    I noticed this PR has been quiet since August 19th. If it's okay, I'd like to help out or take it on.

  20. MojtabaTajik commented on Sep 30, 2023

    @MojtabaTajik
    Contributor

    Hi @adamsitnik and @MojtabaTajik,

    I noticed this PR has been quiet since August 19th. If it's okay, I'd like to help out or take it on.

    Hey @tcortega

    I was on vacation, but I hope to get it done this week.

  21. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on Oct 14, 2023
  22. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Oct 24, 2023
  23. modified the milestones: Future, 9.0.0 on Oct 25, 2023
  24. ghost locked as resolved and limited conversation to collaborators on Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

api-approvedAPI was approved in API review, it can be implementedarea-System.IOgood first issueIssue should be easy to implement, good for first-time contributorshelp wanted[up-for-grabs] Good issue for external contributors

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions