Skip to content

Integrate upstream .NET patches for Console.ReadKey #617

Closed

Description

While I'd hoped this would be easy, I must track it because I am blocked (again) on dotnet/cli#1043.

This will resolve #571, #461, and #484. The patches were merged in dotnet/corefx#6488, and I've posted manually built patched assemblies to the #porting channel on Slack.

The ReadKey changes are likely to be incompatible with George Fleming (@palladia)'s changes in #613 to support intercepting Ctrl-C.

Lifetime of terminal attribute settings. Previously we were setting up the terminal's attributes (e.g. disabling echo) the first time the console was used, and leaving those settings for the duration of the process. This has the very bad effect of leaving such settings on the terminal while other code using that terminal runs. For example, if you launch a process with Process.Start and that process expects input, echo will still be disabled, and you won't be able to see the input. This commit changes how we do these attributes. Instead of setting them and leaving them, we only set them while a read operation is in progress, reverting the settings immediately after. (This does mean that typing prior to a call to Console.ReadKey(intecept: true) will yield visible text, an unfortunate tradeoff, but it seems to be the better of the options.)

(Emphasis mine. This trade-off is the only solution we have to a myriad of other problems.)

I believe this means that characters typed while ReadKey is not blocking will not get captured. However, the only way to know for sure is extensive testing.

Activity

  1. andyleejordan commented on Mar 3, 2016

    @andyleejordan
    MemberAuthor

    I believe this means that characters typed while ReadKey is not blocking will not get captured. However, the only way to know for sure is extensive testing.

    They get captured just fine. What we saw was Console.KeyAvailable returning true only for Enter and EOL.

  2. andyleejordan commented on Mar 4, 2016

    @andyleejordan
    MemberAuthor

    This is in progress on the new-packages branch. Waiting on a couple things:

  3. andyleejordan commented on Mar 5, 2016

    @andyleejordan
    MemberAuthor

    All right, now I'm just waiting on packages as the build 23904 of System.Reflection.TypeExtensions is no good.

  4. andyleejordan commented on Mar 7, 2016

    @andyleejordan
    MemberAuthor

    Build 23907 worked with some updated packages (TypeExtensions minor version was updated as well, among others). Also had to fix up the compatibility package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions