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

Graduate PSReadLine feature and add UseLegacyReadLine #1076

Merged
merged 2 commits into from
Oct 31, 2019

Conversation

TylerLeonhardt
Copy link
Member

@TylerLeonhardt TylerLeonhardt commented Oct 30, 2019

This stops checking for PSReadLine feature flag in the feature flags passed in.

Instead, it plumbs a UseLegacyReadLine parameter which allows the ability to force using the Legacy ReadLine experience (for situations like accessibility that require removing PSRL).

I have a separate PR on the vscode-powershell side taking advantage of this here: PowerShell/vscode-powershell#2262

// We also want it if we are either:
// * On Windows on any version OR
// * On Linux or macOS on any version greater than or equal to 7
var shouldUsePSReadLine = enableConsoleRepl && !useLegacyReadLine
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var shouldUsePSReadLine = enableConsoleRepl && !useLegacyReadLine
bool shouldUsePSReadLine = enableConsoleRepl && !useLegacyReadLine

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed!

// * On Windows on any version OR
// * On Linux or macOS on any version greater than or equal to 7
var shouldUsePSReadLine = enableConsoleRepl && !useLegacyReadLine
&& (VersionUtils.IsWindows || (!VersionUtils.IsWindows && !VersionUtils.IsPS6));
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't (IsWindows || (!IsWindows && IsPS6) be the same as (IsWindows || !IsPS6)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call - fixed!

// Stdio server can't support an integrated console so we pass in false.
// Stdio server can't support an integrated console so we pass in false for
// enableConsoleRepl and useLegacyReadLine.
false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Possible to use named args here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

@rjmholt
Copy link
Contributor

rjmholt commented Oct 30, 2019

Hopefully we can slightly reduce the number of files a change like this touches in the binary cmdlet refactor!

@rjmholt rjmholt merged commit 259fe9a into PowerShell:master Oct 31, 2019
@TylerLeonhardt TylerLeonhardt deleted the graduate-psrl-feature branch April 11, 2020 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants