FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Bump .NET core framework to 3.1-preview.2 by adityapatwardhan · Pull Request #10993 · PowerShell/PowerShell · GitHub

Bump .NET core framework to 3.1-preview.2 - #10993

Merged
Aditya Patwardhan (adityapatwardhan) merged 3 commits into
PowerShell:masterfrom
adityapatwardhan:UpdateDotNet3_1_p2
Nov 7, 2019
Merged

Bump .NET core framework to 3.1-preview.2#10993
Aditya Patwardhan (adityapatwardhan) merged 3 commits into
PowerShell:masterfrom
adityapatwardhan:UpdateDotNet3_1_p2

Conversation

Aditya Patwardhan (adityapatwardhan) commented Nov 5, 2019
edited
Loading

Copy link
Copy Markdown
Member

PR Summary

Move to .NET core 3.1-preview.2

PR Context

PR Checklist

Comment thread .devcontainer/Dockerfile Outdated

Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

// The change was made in https://github.com/dotnet/coreclr/pull/27229
// The recommendation from .NET team is to not check for 'completed' if 'flush' is false.
// Break out of the loop if all bytes have been read.
if (!flush && bytesRead == byteIndex)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Now that complete is useless, I think we'd better change this implementation a bit more. Like the following:

            while (true)
            {
                // Read at most the number of bytes that will fit in the input buffer. The
                // return value is the actual number of bytes read, or zero if no bytes remain.
                bytesRead = stream.Read(bytes, 0, useBufferSize * 4);
                if (bytesRead == 0)
                {
                    break;
                }

                bool completed = false;
                int byteIndex = 0;
                int bytesUsed;
                int charsUsed;

                while (bytesRead > byteIndex)
                {
                    // If this is the last input data, flush the decoder's internal buffer and state.
                    bool flush = (bytesRead == 0);
                    decoder.Convert(bytes, byteIndex, bytesRead - byteIndex,
                                    chars, 0, useBufferSize, flush,
                                    out bytesUsed, out charsUsed, out completed);
                    // The conversion produced the number of characters indicated by charsUsed. Write that number
                    // of characters to our result buffer
                    result.Append(chars, 0, charsUsed);

                    // Increment byteIndex to the next block of bytes in the input buffer, if any, to convert.
                    byteIndex += bytesUsed;
                }
            } while (bytesRead != 0);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Never mind. Change of the API makes it hard to reason about -- does decoder.Convert needs to be called again when bytesRead is 0? If it's needed, then will it produce any chars? If not, what's the purpose of doing so?
Without the understanding of those questions, let's keep the change as safe as possible, at least for now.

Copy link
Copy Markdown

🎉v7.0.0-preview.6 has been released which incorporates this pull request.:tada:

Handy links:

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL