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

carlossanlop · 2021-01-08T00:24:52Z

The purpose of this PR is described in the issue #44969 - Pilot a new process that extracts API documentation from source code under the section "Bring documentation from Docs to triple slash".

Opened as draft to collect preliminary comments.

I wrote a tool that collects all the documentation published in MS Docs for a particular assembly, then inserts it as triple slash comments in source code using Roslyn.

The code for that tool is currently located in this ongoing PR:
dotnet/api-docs-sync#23

The tool was executed with this command:

DocsPortingTool.exe \
-CsProj D:\runtime\src\libraries\System.IO.Compression.Brotli\src\System.IO.Compression.Brotli.csproj \
-Docs D:\dotnet-api-docs\xml \
-Save true \
-Direction ToTripleSlash \
-IncludedAssemblies System.IO.Compression.Brotli \
-IncludedNamespaces System.IO.Compression

I decided to start with System.IO.Compression.Brotli because it is a relatively small and simple assembly.

There are 3 types in this assembly, and the original documentation is located in these files:

BrotliDecoder: dotnet-api-docs/xml/System.IO.Compression/BrotliDecoder.xml
BrotliEncoder: dotnet-api-docs/xml/System.IO.Compression/BrotliEncoder.xml
BrotliStream: dotnet-api-docs/xml/System.IO.Compression/BrotliStream.xml

cc @safern @jeffhandley @ericstj

carlossanlop · 2021-01-08T00:26:59Z

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs

@@ -72,6 +82,13 @@ 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><![CDATA[


The ## Remarks header is excluded to save lines of code. The MS Docs website will show the Remarks header whether it's explicitly mentioned in the remarks comments or not.

Double newlines were collapsed into one also to save lines of code. The MS Docs website should be able to render paragraph line height correctly without the need for two line break characters.

Could it be possible to avoid emitting ![CDATA[]]? Seems meaningless on VS' triple-slash comments.

It's necessary for invalid XML eg <xref:System.Buffers.OperationStatus.Done?displayProperty=nameWithType> which is unclosed. Perhaps we could close those tags so it could be removed. In this block they aren't present and it seems like valid XML.

I believe CDATA is also used if you want to preserve line breaks -- in our case if we're going to break these lines so they have some reasonable column width, we probably specifically don't want to preserve those line breaks. I'm not sure how this will work.

See my other comment.

...ibraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliStream.Compress.cs

...raries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliStream.Decompress.cs

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliEncoder.cs

...ibraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliStream.Compress.cs

danmoseley · 2021-01-08T03:19:08Z

Nice to see this.

Some of the line lengths are super long. Not good for developers. Should the tool be line breaking at a reasonable point, as a human would do?

danmoseley · 2021-01-08T03:24:09Z

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs

+        /// <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><![CDATA[
+        /// The `DisposeAsync` method lets you perform a resource-intensive dispose operation without blocking the main thread. This performance consideration is particularly important in a [!INCLUDE[win8_appname_long](~/includes/win8-appname-long-md.md)] app or [!INCLUDE[desktop_appname](~/includes/desktop-appname-md.md)] 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 `async` and `await` keywords in Visual Basic and C#.


Would devs know how to edit these INCLUDE markers?

No. We will have to publish new guidance for when devs want to add links to external repos.

Since our aim in the near future is to add comments in triple slash, then the Docs team will help review comments here, and they'll be able to provide guidance whenever necessary.

It might be good to clean up the dotnet-api-docs repo first, e.g. replacing all INCLUDEs and deleting multiple successive newlines. Also trailing and leading whitespace.

danmoseley · 2021-01-08T03:25:19Z

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliDecoder.cs

+        /// <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><![CDATA[
+        /// The return value can be as follows:


If the tool closed the xref tags could we lose the ugly CDATA?

Also, doesn't VS/the .NET world expect these to be <see cref="System.Buffers.OperationStatus.Done"/>? How can we satisfy those tools/VS and also the docs tools that want xref?

See my other comment.

jozkee · 2021-01-08T19:27:29Z

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs

+        /// <remarks><![CDATA[
+        /// The `DisposeAsync` method lets you perform a resource-intensive dispose operation without blocking the main thread. This performance consideration is particularly important in a [!INCLUDE[win8_appname_long](~/includes/win8-appname-long-md.md)] app or [!INCLUDE[desktop_appname](~/includes/desktop-appname-md.md)] 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 `async` and `await` keywords in Visual Basic and C#.
+        /// This method disposes the Brotli stream by writing any changes to the backing store and closing the stream to release resources.
+        /// Calling `DisposeAsync` allows the resources used by the <xref:System.IO.Compression.BrotliStream> to be reallocated for other purposes. For more information, see [Cleaning Up Unmanaged Resources](/dotnet/standard/garbage-collection/unmanaged).


xref links could be converted to <see cref=""> but then we probably have to find out how many other tags are worth converting.

Suggested change

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

/// 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).

If we convert xrefs to crefs, there are two problems to tackle:

We need to add the API type prefix (T: for types, P: for properties, F: for vields, E: for events, M: for methods, N: for namespaces). I need to check if Roslyn has a way to obtain the full DocID with this prefix.

We would lose the suffix ?displayProperty=nameWithType, which is invalid in crefs.

David and I discussed this earlier today - we may be able to remove the CDATA tags and leave the xref items in remarks unmodified, without causing a build failure due to unresolved API full names. The triple slash comments seem to treat xref items as plain text, not as xml items.

Same case with hyperlinks in markdown format: If we remove CDATA, they don't cause a failure. Seems to be the case with most markdown code.

I suspect that if we remove the CDATA, if we send the remarks without line breaks to the Docs system, it will render the text correctly as it looks right now. Each sentence is a full paragraph, and the CSS takes care of the line height, not the endline characters. I'll verify this with a test PR in dotnet-api-docs.

CC @danmosemsft

I like that you're doing just this one library so we can iterate a bit and find an approach that works really well (with respect to CDATA, line breaks, INCLUDE, and anything else) and that devs like before extending to other libraries.

(BTW as an aside we apparently have an analyzer that flags "T:" etc in our build -- I know because I had to remove them when porting System.Speech. We'd have to disable that.)

Presumably aspnetcore must have solved (or at least made decisions about) each of these things already?

We need to add the API type prefix (T: for types, P: for properties, F: for vields, E: for events, M: for methods, N: for namespaces). I need to check if Roslyn has a way to obtain the full DocID with this prefix.

Why? Roslyn emits those into the underlying XML automatically based on resolving the name. We should strongly prefer using the language-supported syntax.

We would lose the suffix ?displayProperty=nameWithType, which is invalid in crefs.

Why is that necessary? My preference is that we only force the different syntax when there's something critical (i.e. for whatever reason we couldn't accept a slightly less precise experience) it expresses that can't be done with the existing, well-supported / recognized-by-IntelliSense / etc. syntax.

Thanks for correcting me, @stephentoub. The prefixes are not needed, which would make it easier for remarks to be transformed to <see cref=""> items in triple slash.

I forgot to mention, but I also have to detect remarks items that are wrapped by backticks, which point to paramrefs or typeparamrefs. I would have to find out what it's referring to, and if nothing is found, then I have to transform it into a <see langword="">.

I did an experiment in MS Docs to verify point no. 5, and I was wrong: sentences are not handled as independent paragraphs by the MS Docs CSS. See the test PR results: https://github.com/dotnet/dotnet-api-docs/pull/5213/files

So if we want to preserve paragraphs, we will have to keep empty lines, which would make several remarks quite large.

So if we want to preserve paragraphs, we will have to keep empty lines, which would make several remarks quite large.

Preservation of paragraph boundaries because that's how they currently are isn't required. Preservation of desired paragraph boundaries is desirable. Such boundaries generally are valuable in large part to help with reading comprehension, which makes them valuable in comments as well.

danmoseley · 2021-01-08T21:20:00Z

With these docs added, is there some analyzer rule we can switch on so that the build of this library will from now on fail if someone adds a publicly visible member without docs?

carlossanlop · 2021-01-11T19:05:17Z

With these docs added, is there some analyzer rule we can switch on so that the build of this library will from now on fail if someone adds a publicly visible member without docs?

@danmosemsft It seems that adding <GenerateDocumentationFile>true</GenerateDocumentationFile> to the first <PropertyGroup> of the csproj would suffice to emit a warning during build if a public API does not have docs.

Pending adding that to this csproj.

gewarren · 2021-01-11T23:06:51Z

What should we do about APIs that have super lengthy remarks, e.g. String.Format()? I'd propose moving excess remarks to the dotnet/docs repo, to somewhere like this: https://docs.microsoft.com/en-us/dotnet/standard/runtime-libraries-overview. And then linking there from the triple slash remarks.

gewarren · 2021-01-11T23:15:02Z

...ibraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliStream.Compress.cs

@@ -132,6 +187,12 @@ public override void Flush()
            }
        }

+        /// <summary>Asynchronously clears all buffers for this Brotli stream, causes any buffered data to be written to the underlying device, and monitors cancellation requests.</summary>


It would be good to see an example of backporting from dotnet-api-docs when there are existing triple slash comments too.

Good point to bring up: since dotnet-api-docs is the source of truth currently, then it doesn't matter if existing triple slash comments get overwritten when backportin. If there's anything important we want to keep, then it can be discussed in the PR.

I'll have to add a unit test for this case in my tool anyway, because I think the spacing gets messed up when there are existing comments already.

carlossanlop · 2021-01-11T23:33:44Z

What should we do about APIs that have super lengthy remarks, e.g. String.Format()? I'd propose moving excess remarks to the dotnet/docs repo, to somewhere like this: https://docs.microsoft.com/en-us/dotnet/standard/runtime-libraries-overview. And then linking there from the triple slash remarks.

Moving lengthy remarks somewhere else is a good suggestion, @gewarren. We can consider it.

A similar suggestion would be having those remarks in *.md files, and link them like we link example files. Do you know if it's possible?

For example, if this is how we link multi-language example snippets:

 [!code-cpp[Name of the example](~/samples/snippets/cpp/VS_Snippets_Remoting/ExampleFolder/CPP/source.cpp#1)]
 [!code-csharp[Name of the example](~/samples/snippets/csharp/VS_Snippets_Remoting/ExampleFolder/CS/source.cs#1)]
 [!code-vb[Name of the example](~/samples/snippets/visualbasic/VS_Snippets_Remoting/ExampleFolder/VB/source.vb#1)]

Then I think remarks could be linked more or less like this:

[!md][Remarks](~/samples/remarks/md/Namespace.Type/remarks.md)

danmoseley · 2021-01-11T23:40:53Z

Moving lengthy remarks somewhere else is a good suggestion

Would we lose any build validation those remarks currently get in the docs repo? Eg., that they are valid XML, have valid links?

BTW, you also show links to samples. How are those currently validated today - is there a build process? Several folks have mentioned the value of building them, so they don't "rot". For brand new libraries, they could even be unit tests.

carlossanlop · 2021-01-12T00:03:15Z

Would we lose any build validation those remarks currently get in the docs repo? Eg., that they are valid XML, have valid links?

The validation would be done on the dotnet-api-docs side, when we send the build-generated xmls to the Docs system to update the MS Docs content.

BTW, you also show links to samples. How are those currently validated today - is there a build process? Several folks have mentioned the value of building them, so they don't "rot". For brand new libraries, they could even be unit tests.

The samples are not currently validated. We have an email thread with a conversation about this. We decided to leave this outside of the scope of this plan, but in the future it would be nice to have them built to make sure the code is valid.

One option we were considering was to move the code samples to their own repo for this sole purpose. They would continue being linked the same way, with a link to the file (but pointing to the new repo).

For now, the code samples will all remain in dotnet-api-docs, same location.

gewarren · 2021-01-12T03:22:16Z

The samples are not currently validated. We have an email thread with a conversation about this. We decided to leave this outside of the scope of this plan, but in the future it would be nice to have them built to make sure the code is valid.

One option we were considering was to move the code samples to their own repo for this sole purpose. They would continue being linked the same way, with a link to the file (but pointing to the new repo).

For now, the code samples will all remain in dotnet-api-docs, same location.

@adegeo has a sample validator called SNIPPETS 5000 that we use for building code snippets in dotnet/docs.

gewarren · 2021-01-15T17:01:06Z

Moving lengthy remarks somewhere else is a good suggestion, @gewarren. We can consider it.

A similar suggestion would be having those remarks in *.md files, and link them like we link example files. Do you know if it's possible?

I tried it out in this draft PR, and it works to have the content in a separate markdown file and then reference it using INCLUDE syntax.

## Remarks

[!INCLUDE[extra-remarks](~/includes/extra-remarks.md)]

carlossanlop · 2021-02-01T19:24:37Z

I think this PR is ready to merge, if all looks ok and I get an approval.

Here is a list of items to tackle in the next docs backporting PRs I plan to send:

Partial class implementations divided by platform will be the next problem to address in more complex assemblies (this wasn't an issue in this Brotli assembly). From my conversations with @safern, the best approach for this will be to copy and paste the exact same comments in all the implementations. This will simplify the process of generating the intellisense xmls when runtime builds, and we won't have to worry about having to join the docs back into one intellisense xml file before sending it to the Docs build system.
Developers will have to make sure to keep the exact same documentation in all the files from now on. @safern and I will talk about adding logic to our build system that will check if there are any diffs in the triple slash comments of an API with multiple implementations per platform, and we would throw a build warning to get them fixed. I need to add logic to the DocsPortingTool to detect multiple sites in the SyntaxTree and paste the descriptions in all the API locations.
@stephentoub - I am preserving the minimum required paragraph boundaries to keep the comments as compact as possible here in source. I'll work with the Docs team to make sure they show up correctly in MS Docs.
Code examples will remain in dotnet-api-docs. Som APIs have their examples written directly in the remarks section. For those cases, we will have to create a new file in dotnet-api-docs and link them as [INCLUDE[...]] in the remarks sections in triple slash comments.
@gewarren confirmed we can put lengthy remarks in separate md files. If we encounter any, I'll create their file in dotnet-api-docs. We will treat them exactly like examples.
@adegeo has a tool called SNIPPETS 5000 to validate code examples. We decided this problem is out of scope for the 6.0 documentation plan, but we will definitely talk about this afterwards.
@gewarren mentioned the case where we have triple slash comments and we have documentation in dotnet-api-docs. Since today we consider MS Docs the source of truth, then DocsPortingTool will overwrite whatever exists in triple slash. The PR will help to ensure we don't lose any additional information that's specific for devs. For those cases, my suggestion is that we preserve those comments as double slash comments, so they don't get ported back to MS Docs.
As soon as we get this PR merged, S.IO.Compression.Brotli will have its documentation source of truth in triple slash comments. I will keep a list of finished assemblies in the main Docs issue.
As @danmosemsft asked: with the addition of <GenerateDocumentationFile>true</GenerateDocumentationFile> to the Brotli.csproj, the build will now throw warnings for this assembly if any public APIs are missing docs.
Speaking of which, the <GenerateDocumentationFile> element will have to be manually added to the csproj. The tool could do it, but from what I have asked to the Roslyn team, it's not a straightforward task of just calling an existing API. I prefer to spend time addressing other problems because this could take too much time to investigate and it's probably not worth investing on it.
If we encounter markers like [!INCLUDE[win8_appname_long](~/includes/win8-appname-long-md.md)] when backporting docs, we can ask area owners if they prefer to keep them or not. In MS Docs, they have the purpose of ensuring that we use the official name of specific products. But if we make sure to convert them to the appropriate text, I think we can have them as plain text in triple slash, which will be less confusing for future maintainers.

safern · 2021-02-01T22:28:47Z

Next steps and the bullet points you mentioned above make sense to me.

danmoseley

LGTM. I tihnk it wasn't clear to me whether we expect to have markers like [!INCLUDE[win8_appname_long](~/includes/win8-appname-long-md.md)] long term or not. But this seems like a good first step that we're learning from. I like this measured approach.

carlossanlop · 2021-02-02T01:53:19Z

I missed that one, @danmosemsft , thanks for reminding me.

In my opinion, for existing docs, it doesn't hurt if we keep those INCLUDE Markers. For future docs, I am hoping the Docs style and language reviewers will be able to tell us to use them if/when they're needed.

Edit: @danmosemsft FYI I decided to replace them to plain text in this PR.

carlossanlop · 2021-02-02T03:24:44Z

@safern how do I determine if I need to run the tool against this assembly using RuntimeFlavor=mono like you mentioned here? Do I need to run it here too?

safern · 2021-02-02T19:02:38Z

That is only when it includes APIs that live in System.Private.CoreLib. Because System.Private.CoreLib might have different sources for coreclr and mono.

safern

🎉

sharwell · 2021-03-02T03:52:34Z

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs

@@ -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>


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

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

@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.

sharwell · 2021-03-02T03:53:06Z

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs

@@ -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>


💡 Would be good to omit the space before />

sharwell · 2021-03-02T03:54:29Z

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs

@@ -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#.


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

sharwell · 2021-03-02T03:55:54Z

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs

+        /// <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#.
+        /// This method disposes the Brotli stream by writing any changes to the backing store and closing the stream to release resources.
+        /// 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>


📝 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>

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

sharwell · 2021-03-02T03:56:30Z

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs

+        /// <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#.
+        /// This method disposes the Brotli stream by writing any changes to the backing store and closing the stream to release resources.


❔ 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.

sharwell · 2021-03-02T03:57:13Z

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs

        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>


<see langword="Decompress," />

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

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

sharwell · 2021-03-02T03:59:22Z

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliDecoder.cs

+        /// - <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>


📝 This should have been rendered as a definition list. Here's an example from Roslyn:

https://github.com/dotnet/roslyn/blob/b09625add4da1b872e8033d2775ea514786d86e1/src/Features/Core/Portable/DocumentationComments/AbstractDocumentationCommentFormattingService.cs#L43-L56

sharwell · 2021-03-02T04:00:29Z

...raries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliStream.Decompress.cs

+        /// </summary>
+        /// <returns>The unsigned byte cast to an <see cref="int"/>, or -1 if at the end of the stream.</returns>
+        /// <exception cref="InvalidOperationException">Cannot perform read operations on a <see cref="BrotliStream" /> constructed with <see cref="CompressionMode.Compress" />.
+        /// -or-


📝 This is likely missing <para> elements around each line

sharwell · 2021-03-02T04:01:25Z

...raries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliStream.Decompress.cs

+        /// <param name="buffer">A region of memory. When this method returns, the contents of this region are replaced by the bytes read from the current source.</param>
+        /// <returns>The total number of bytes read into the buffer. This can be less than the number of bytes allocated in the buffer if that many bytes are not currently available, or zero (0) if the end of the stream has been reached.</returns>
+        /// <remarks>Use the <see cref="System.IO.Compression.BrotliStream.CanRead" /> property to determine whether the current instance supports reading. Use the <see langword="System.IO.Compression.BrotliStream.ReadAsync" /> method to read asynchronously from the current stream.
+        /// This method read a maximum of `buffer.Length` bytes from the current stream and store them in <paramref name="buffer" />. The current position within the Brotli stream is advanced by the number of bytes read; however, if an exception occurs, the current position within the Brotli stream remains unchanged. This method will block until at least one byte of data can be read, in the event that no data is available. `Read` returns 0 only when there is no more data in the stream and no more is expected (such as a closed socket or end of file). The method is free to return fewer bytes than requested even if the end of the stream has not been reached.


`buffer.Length`

`Read`

Definitely want to update these to render correctly with <c>buffer.Length</c> instead of "Markdown".

carlossanlop added the documentation Documentation bug or enhancement, does not impact product or test code label Jan 8, 2021

carlossanlop self-assigned this Jan 8, 2021

Dotnet-GitSync-Bot added the area-System.IO.Compression label Jan 8, 2021

carlossanlop commented Jan 8, 2021

View reviewed changes

...ibraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliStream.Compress.cs Outdated Show resolved Hide resolved

carlossanlop commented Jan 8, 2021

View reviewed changes

...raries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliStream.Decompress.cs Show resolved Hide resolved

This was referenced Jan 8, 2021

Libraries APIs are fully documented using efficient workflows #44969

Open

Add missing BrotliStream methods to ref file #46726

Merged

carlossanlop commented Jan 8, 2021

View reviewed changes

...raries/System.IO.Compression.Brotli/src/System/IO/Compression/dec/BrotliStream.Decompress.cs Outdated Show resolved Hide resolved

carlossanlop commented Jan 8, 2021

View reviewed changes

src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliEncoder.cs Outdated Show resolved Hide resolved