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

Backport docs for System.IO.Compression.Brotli #46716

Merged
merged 7 commits into from
Feb 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Nullable>enable</Nullable>
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)</TargetFrameworks>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
</PropertyGroup>
<!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
<PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace System.IO.Compression
{
/// <summary>Provides methods and properties used to compress and decompress streams by using the Brotli data format specification.</summary>
public sealed partial class BrotliStream : Stream
{
private const int DefaultInternalBufferSize = (1 << 16) - 16; //65520;
Expand All @@ -16,7 +17,14 @@ public sealed partial class BrotliStream : Stream
private readonly bool _leaveOpen;
private readonly CompressionMode _mode;

/// <summary>Initializes a new instance of the <see cref="System.IO.Compression.BrotliStream" /> class by using the specified stream and compression mode.</summary>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Would be good to simplify the type name where possible:

-cref="System.IO.Compression.BrotliStream"
+cref="BrotliStream"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sharwell do you know how to get the prefix removed via Roslyn if the symbol can be resolved in the current file? I couldn't find any information on this, which is why I decided to leave their full names.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Would be good to omit the space before />

/// <param name="stream">The stream to compress.</param>
/// <param name="mode">One of the enumeration values that indicates whether to compress or decompress the stream.</param>
public BrotliStream(Stream stream, CompressionMode mode) : this(stream, mode, leaveOpen: false) { }
/// <summary>Initializes a new instance of the <see cref="System.IO.Compression.BrotliStream" /> class by using the specified stream and compression mode, and optionally leaves the stream open.</summary>
/// <param name="stream">The stream to compress.</param>
/// <param name="mode">One of the enumeration values that indicates whether to compress or decompress the stream.</param>
/// <param name="leaveOpen"><see langword="true" /> to leave the stream open after the <see cref="System.IO.Compression.BrotliStream" /> object is disposed; otherwise, <see langword="false" />.</param>
public BrotliStream(Stream stream, CompressionMode mode, bool leaveOpen)
{
if (stream == null)
Expand Down Expand Up @@ -48,6 +56,8 @@ private void EnsureNotDisposed()
throw new ObjectDisposedException(GetType().Name, SR.ObjectDisposed_StreamClosed);
}

/// <summary>Releases the unmanaged resources used by the <see cref="System.IO.Compression.BrotliStream" /> and optionally releases the managed resources.</summary>
/// <param name="disposing"><see langword="true" /> to release both managed and unmanaged resources; <see langword="false" /> to release only unmanaged resources.</param>
protected override void Dispose(bool disposing)
{
try
Expand All @@ -72,6 +82,11 @@ protected override void Dispose(bool disposing)
}
}

/// <summary>Asynchronously releases the unmanaged resources used by the <see cref="System.IO.Compression.BrotliStream" />.</summary>
/// <returns>A task that represents the asynchronous dispose operation.</returns>
/// <remarks>The `DisposeAsync` method lets you perform a resource-intensive dispose operation without blocking the main thread. This performance consideration is particularly important in a Windows 8.x Store app or desktop app where a time-consuming stream operation can block the UI thread and make your app appear as if it is not working. The async methods are used in conjunction with the <see langword="async" /> and <see langword="await" /> keywords in Visual Basic and C#.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❔ What form did `DisposeAsync` have in the original input? It should have been rendered either as a <see cref> or as a <c> element.

/// This method disposes the Brotli stream by writing any changes to the backing store and closing the stream to release resources.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❔ Is this all supposed to be one paragraph? If more than one paragraph is part of the documentation output, explicit <para> elements need to be used in the documentation XML.

/// Calling `DisposeAsync` allows the resources used by the <see cref="System.IO.Compression.BrotliStream" /> to be reallocated for other purposes. For more information, see [Cleaning Up Unmanaged Resources](/dotnet/standard/garbage-collection/unmanaged).</remarks>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 The link was not rendered correctly.

-[Cleaning Up Unmanaged Resources](/dotnet/standard/garbage-collection/unmanaged)
+<see href="https://docs.microsoft.com/dotnet/standard/garbage-collection/unmanaged">Cleaning Up Unmanaged Resources</see>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be <a href=""></a>? I couldn't find examples or official guidance on this.

public override async ValueTask DisposeAsync()
{
try
Expand Down Expand Up @@ -112,17 +127,39 @@ private void ReleaseStateForDispose()
}
}

/// <summary>Gets a reference to the underlying stream.</summary>
/// <value>A stream object that represents the underlying stream.</value>
/// <exception cref="System.ObjectDisposedException">The underlying stream is closed.</exception>
public Stream BaseStream => _stream;
/// <summary>Gets a value indicating whether the stream supports reading while decompressing a file.</summary>
/// <value><see langword="true" /> if the <see cref="System.IO.Compression.CompressionMode" /> value is <see langword="Decompress," /> and the underlying stream supports reading and is not closed; otherwise, <see langword="false" />.</value>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<see langword="Decompress," />

Not sure what this was supposed to be, but it didn't turn out as expected.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I manually edited this, it's a typo. My fault.

public override bool CanRead => _mode == CompressionMode.Decompress && _stream != null && _stream.CanRead;
/// <summary>Gets a value indicating whether the stream supports writing.</summary>
/// <value><see langword="true" /> if the <see cref="System.IO.Compression.CompressionMode" /> value is <see langword="Compress" />, and the underlying stream supports writing and is not closed; otherwise, <see langword="false" />.</value>
public override bool CanWrite => _mode == CompressionMode.Compress && _stream != null && _stream.CanWrite;
/// <summary>Gets a value indicating whether the stream supports seeking.</summary>
/// <value><see langword="false" /> in all cases.</value>
public override bool CanSeek => false;
/// <summary>This property is not supported and always throws a <see cref="System.NotSupportedException" />.</summary>
/// <param name="offset">The location in the stream.</param>
/// <param name="origin">One of the <see cref="System.IO.SeekOrigin" /> values.</param>
/// <returns>A long value.</returns>
/// <exception cref="System.NotSupportedException">This property is not supported on this stream.</exception>
public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
/// <summary>This property is not supported and always throws a <see cref="System.NotSupportedException" />.</summary>
/// <value>A long value.</value>
/// <exception cref="System.NotSupportedException">This property is not supported on this stream.</exception>
public override long Length => throw new NotSupportedException();
/// <summary>This property is not supported and always throws a <see cref="System.NotSupportedException" />.</summary>
/// <value>A long value.</value>
/// <exception cref="System.NotSupportedException">This property is not supported on this stream.</exception>
public override long Position
{
get => throw new NotSupportedException();
set => throw new NotSupportedException();
}
/// <summary>This property is not supported and always throws a <see cref="System.NotSupportedException" />.</summary>
/// <param name="value">The length of the stream.</param>
public override void SetLength(long value) => throw new NotSupportedException();

private int _activeAsyncOperation; // 1 == true, 0 == false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace System.IO.Compression
{
/// <summary>Provides non-allocating, performant Brotli decompression methods. The methods decompress in a single pass without using a <see cref="System.IO.Compression.BrotliStream" /> instance.</summary>
public struct BrotliDecoder : IDisposable
{
private SafeBrotliDecoderHandle? _state;
Expand All @@ -27,6 +28,7 @@ internal void EnsureInitialized()
InitializeDecoder();
}

/// <summary>Releases all resources used by the current Brotli decoder instance.</summary>
public void Dispose()
{
_disposed = true;
Expand All @@ -39,6 +41,17 @@ private void EnsureNotDisposed()
throw new ObjectDisposedException(nameof(BrotliDecoder), SR.BrotliDecoder_Disposed);
}

/// <summary>Decompresses data that was compressed using the Brotli algorithm.</summary>
/// <param name="source">A buffer containing the compressed data.</param>
/// <param name="destination">When this method returns, a byte span containing the decompressed data.</param>
/// <param name="bytesConsumed">The total number of bytes that were read from <paramref name="source" />.</param>
/// <param name="bytesWritten">The total number of bytes that were written in the <paramref name="destination" />.</param>
/// <returns>One of the enumeration values that indicates the status of the decompression operation.</returns>
/// <remarks>The return value can be as follows:
/// - <see cref="System.Buffers.OperationStatus.Done" />: <paramref name="source" /> was successfully and completely decompressed into <paramref name="destination" />.
/// - <see cref="System.Buffers.OperationStatus.DestinationTooSmall" />: There is not enough space in <paramref name="destination" /> to decompress <paramref name="source" />.
/// - <see cref="System.Buffers.OperationStatus.NeedMoreData" />: The decompression action is partially done at least one more byte is required to complete the decompression task. This method should be called again with more input to decompress.
/// - <see cref="System.Buffers.OperationStatus.InvalidData" />: The data in <paramref name="source" /> is invalid and could not be decompressed.</remarks>
Comment on lines +51 to +54
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public OperationStatus Decompress(ReadOnlySpan<byte> source, Span<byte> destination, out int bytesConsumed, out int bytesWritten)
{
EnsureInitialized();
Expand Down Expand Up @@ -92,6 +105,12 @@ public OperationStatus Decompress(ReadOnlySpan<byte> source, Span<byte> destinat
}
}

/// <summary>Attempts to decompress data that was compressed with the Brotli algorithm.</summary>
/// <param name="source">A buffer containing the compressed data.</param>
/// <param name="destination">When this method returns, a byte span containing the decompressed data.</param>
/// <param name="bytesWritten">The total number of bytes that were written in the <paramref name="destination" />.</param>
/// <returns><see langword="true" /> on success; <see langword="false" /> otherwise.</returns>
/// <remarks>If this method returns <see langword="false" />, <paramref name="destination" /> may be empty or contain partially decompressed data, with <paramref name="bytesWritten" /> being zero or greater than zero but less than the expected total.</remarks>
public static unsafe bool TryDecompress(ReadOnlySpan<byte> source, Span<byte> destination, out int bytesWritten)
{
fixed (byte* inBytes = &MemoryMarshal.GetReference(source))
Expand Down
Loading