Skip to content

Console.Unix: make Console.OpenStandardInput Stream aware of terminal - #39192

Merged
adamsitnik merged 6 commits into
dotnet:masterfrom
tmds:console_echo
Aug 14, 2020
Merged

Console.Unix: make Console.OpenStandardInput Stream aware of terminal#39192
adamsitnik merged 6 commits into
dotnet:masterfrom
tmds:console_echo

Conversation

@tmds

@tmdstmds commented Jul 13, 2020

Copy link
Copy Markdown
Member

When performing OpenStandardInput against a terminal, perform Reads on a
line-by-line basis and perform appropriate processing and echoing.

Fixes#39008

@eiriktsarpalis@stephentoub ptal

@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @eiriktsarpalis
Notify danmosemsft if you want to be subscribed.

Comment threadsrc/libraries/System.Console/src/System/IO/StdInReader.cs Outdated
@eiriktsarpalis

Copy link
Copy Markdown
Member

Seems reasonable. Do you think you could also provide a manual test that highlights the functional changes introduced here?

Comment threadsrc/libraries/System.Console/src/System/ConsolePal.Unix.cs Outdated
@tmds

tmds commented Jul 14, 2020

Copy link
Copy Markdown
MemberAuthor

@eiriktsarpalis I've updated the PR based on your feedback. I will still look into adding a manual test.

Comment threadsrc/libraries/System.Console/tests/ManualTests/ManualTests.cs Outdated
@eiriktsarpaliseiriktsarpalis added this to the 5.0.0 milestone Aug 6, 2020
@eiriktsarpalis

Copy link
Copy Markdown
Member

Any progress on this PR? This is a bug, which we should address for 5.0.

@eiriktsarpaliseiriktsarpalis self-assigned this Aug 12, 2020
tmds added 5 commits August 12, 2020 18:19
When performing OpenStandardInput against a terminal, perform Reads on a
line-by-line basis and perform appropriate processing and echoing.
@adamsitnik

Copy link
Copy Markdown
Member

@eiriktsarpalis

image

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

LGTM, thank you @tmds

I have tested it manually and it works as expected. The CI is green, code looks good. Merging and including in 5.0

@adamsitnik
adamsitnik merged commit 1e6e8d9 into dotnet:masterAug 14, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StreamReader over standard input stream does not echo if Console.ReadLine has been run first

4 participants

@tmds@eiriktsarpalis@adamsitnik@Dotnet-GitSync-Bot