- Notifications
You must be signed in to change notification settings - Fork7.7k
Make prompt colors configurable#18003
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I am happy with this though I am going to request reviews from others in the Interactive-UX workgroup too
Uh oh!
There was an error while loading.Please reload this page.
Hi@SeeminglyScience could you please review this PR, Thanks in advance! |
ghost commentedSep 9, 2022
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for7 days. |
@SteveL-MSFT@daxian-dbw@anmenaga@TravisEz13@SeeminglyScience Apologies for tag SPAM, but can I please get this PR reviewed? It had no reviewer activity for 2 weeks and it's blocking some other things I would like to work on. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Cool change, a few concerns
src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHostUserInterfacePromptForChoice.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
@TravisEz13 Resolved |
ghost commentedMay 17, 2023
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for7 days. |
Pinging@SteveL-MSFT for review/merge |
@dkaszews I've brought this up for a review with the Interactive-UX WG and I hope we can get this reviewed in time to add to the next preview release |
$PSStyle.Prompt.ChoiceHelp = "`e[47m" | ||
'@ | ||
# Only reasonable way to force "interactivity" and capture prompts is to fork |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
please take a look at the custom host, which may solve your problem here (and potentially speed up your tests). You can find the module here:test/tools/Modules/HelpersHostCS/HelpersHostCS.psm1
and an example may be found at:test/powershell/Modules/Microsoft.PowerShell.Utility/Read-Host.Tests.ps1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nope - it doesn't help. The test host would need the psstyle changes to be implemented in it as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Apologies, I'm not sure whether this means I should do something or not. To be frank, this PR has been stuck waiting for merge for over a year, so if you want me to add a bunch of code to test it, I'll just mark it as untestable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
to be clear, i think this is an interactive only scenario test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sure, what I have done is quite a dirty and fragile hack. You want me to remove this test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Overall, the changes seem fine to me, but we should wrap this as an ExperimentalFeature to get user feedback.
@SteveL-MSFT Not sure how to do that, as it would likely require wrapping all diff in an |
@daxian-dbw@kilasuit@iSazonov Do any of you have some ability to push this through so it can make 7.5.0? |
StevenBucher98 commentedFeb 12, 2024 • edited by unfurl-linksbot
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by unfurl-linksbot
Uh oh!
There was an error while loading.Please reload this page.
@dkaszews the docs to wrap things in experimental features can be found here:https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_experimental_features?view=powershell-7.5#the-experimental-attribute
|
@StevenBucher98 To confirm, you want me to duplicate all the related code inside a new class |
@kilasuit@SteveL-MSFT Please merge approved changes |
@kilasuit@SteveL-MSFT Please merge approved changes |
I don't have maintainer rights, so can't further I'm afraid. |
@adityapatwardhan@SeeminglyScience@TravisEz13 You seem to have merge rights, could you please help getting this ancient issue resolved? |
@theJasonHelmick can we add this to the next I-UX WG meeting please to be reviewed. |
floh96 commentedJul 18, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@kilasuit this was already reviewed by the WG see#17961 (comment) Also no response from@theJasonHelmick here#17961 (comment) |
@floh96 I know & I'm part of that Working Group & my hope is that this can be merged either before, during or just after we meet again, which is next week, & will allow us to also discuss a bit around why this has been stuck as is for so long too. |
It was approved over a year ago, just nobody clicked "merge". Not sure if it can still be applied cleanly or requires extensive rebase. |
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
Allows user to configure prompt colors (e.g. when removing non-empty directories or explicitly giving
-Confirm
flag). The configurable colors and the default values are as following (up to discussion):Changes in resource strings to avoid coloring trailing whitespace, which leads to visual artifacts for background colors.
Test had to be written by forking the powershell executable, as I could find no other way to both allow interactive prompt (disqualifies
[System.Management.Automation.PowerShell]
even with test hooks) and capture output fromWrite(Line)ToConsole
(disqualifies regular invocation, even with test hooks). Test is tagged slow because it takes up to 100 times longer than similar tests using[System.Management.Automation.PowerShell]
(80ms vs 8s), but if this method is too janky to use in CI, I can simply remove it and mark PR as "can only be tested interactively".Closes#17961
PR Context
Existing colors are hardcoded and are either yellow+white when on light background, or blue+black on white background. Because it is not really possible to reliably detect background colors, especially considering remoting over PSSession or ssh, or embedding sessions in screen or tmux, this leads to white text on white background, completely invisible.
Without PR:

With PR (defaults):

With PR (custom):

PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.(which runs in a different PS Host).