Skip to content

Conversation

davinci26
Copy link
Contributor

@davinci26 davinci26 commented Oct 27, 2019

PR Summary

Issue

Adds #10847

Implementation

  • Extend the PSHostUserInterface class with a virtual method ReadLineMaskedAsString. This method is in between ReadLine and ReadLineAsSecureString.
  • Implement the new method in the InternalHostUserInterface.cs
  • I provided a default implementation in the Interface which just throws an exception to make this change backward compatible.
  • Check at runtime if the user has provided both -MaskInput and -AsSecureString and if so throw an exception. Use parameter sets to make -MaskInput and -AsSecureString mutually exclusive (this is another application for Parameters Should Support Mutual Exclusion Natively #5175)

Happy Hacktoberfest!

PR Context

PR Checklist

  • Testing - New and feature
  • Tooling
    • I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
    • OR
    • I have considered the user experience from a tooling perspective and enumerated concerns in the summary. This may include:
      • Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode (which runs in a different PS Host).
      • Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
      • Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
      • Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).

@davinci26 davinci26 changed the title WiP: Adds Mask Input Parameter to Read-Host Adds Mask Input Parameter to Read-Host Oct 28, 2019
@iSazonov iSazonov requested a review from PaulHigin October 28, 2019 04:06
@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 29, 2019
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 29, 2019
@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 30, 2019
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 30, 2019
@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 30, 2019
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 31, 2019
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Oct 31, 2019
@iSazonov iSazonov added this to the 7.0.0-preview.6 milestone Oct 31, 2019
@TravisEz13
Copy link
Member

TravisEz13 commented Nov 5, 2019

My concern would be that this, https://github.com/PowerShell/PowerShell/pull/10908/files#diff-fff8012934f59c558d7de1adeca99bf1R208, is used in some existing remoting scenario.

GitHub
PR Summary Issue Adds #10847 Implementation

Extend the PSHostUserInterface class with a virtual method ReadLineMaskedAsString. This method is in between ReadLine and ReadLineAsSecureString.
Implem...

@SteveL-MSFT SteveL-MSFT added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Nov 20, 2019
@SteveL-MSFT
Copy link
Member

@PowerShell/powershell-committee reviewed this and is ok with the proposed switch name and functionality.

@SteveL-MSFT
Copy link
Member

@davinci26 can you resolve the merge conflict?

@davinci26
Copy link
Contributor Author

@PoshChan please retry windows and static

@PoshChan
Copy link
Collaborator

PoshChan commented Dec 7, 2019

@davinci26, you are not authorized to request a rebuild

@davinci26
Copy link
Contributor Author

@SteveL-MSFT FYI I have fixed all merge conflicts.

@ghost ghost added the Review - Needed The PR is being reviewed label May 27, 2020
@ghost
Copy link

ghost commented May 27, 2020

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

@iSazonov
Copy link
Collaborator

@TravisEz13 It seems the PR ready to merge.

@TravisEz13 TravisEz13 changed the title Adds Mask Input Parameter to Read-Host Adds Mask Input Parameter to Read-Host May 28, 2020
@TravisEz13 TravisEz13 merged commit d26e8c0 into PowerShell:master May 28, 2020
@iSazonov
Copy link
Collaborator

@davinci26 Thanks for your contribution!

@ghost ghost removed the Review - Needed The PR is being reviewed label May 29, 2020
@ghost
Copy link

ghost commented Jun 25, 2020

🎉v7.1.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log Committee-Reviewed PS-Committee has reviewed this and made a decision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants