Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
dkaszews wants to merge17 commits intoPowerShell:master
base:master
Choose a base branch
Loading
fromdkaszews:dkaszews-psstyle-prompt

Conversation

dkaszews
Copy link
Contributor

@dkaszewsdkaszews commentedAug 31, 2022
edited
Loading

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):

  • Caption - bold yellow, same as warnings
  • Message - default (reset)
  • Help - default (reset)
  • Choice selected by default - white on blue
  • Choice not selected by default - blue
  • Choice for help - default (reset)

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:
image

With PR (defaults):
image

With PR (custom):
image

PR Checklist

@kilasuitkilasuit added the WG-Interactive-Consolethe console experience labelSep 1, 2022
@kilasuitkilasuit self-assigned thisSep 1, 2022
Copy link
Collaborator

@kilasuitkilasuit left a 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

SteveL-MSFT reacted with thumbs up emoji
@kilasuit
Copy link
Collaborator

Hi@SeeminglyScience could you please review this PR, Thanks in advance!

@dkaszewsdkaszews changed the titleWIP: Make prompt colors configurableMake prompt colors configurableSep 1, 2022
@dkaszewsdkaszews marked this pull request as ready for reviewSeptember 1, 2022 18:19
@ghostghost added the Review - NeededThe PR is being reviewed labelSep 9, 2022
@ghost
Copy link

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for7 days.
Maintainer, please provide feedback and/or mark it asWaiting on Author

@dkaszews
Copy link
ContributorAuthor

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

Copy link
Member

@SteveL-MSFTSteveL-MSFT left a 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

@dkaszews
Copy link
ContributorAuthor

@TravisEz13 Resolved

@ghostghost added the Review - NeededThe PR is being reviewed labelMay 17, 2023
@ghost
Copy link

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for7 days.
Maintainer, please provide feedback and/or mark it asWaiting on Author

@dkaszews
Copy link
ContributorAuthor

Pinging@SteveL-MSFT for review/merge

@kilasuit
Copy link
Collaborator

@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
Copy link
Collaborator

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

Copy link
Collaborator

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.

Copy link
ContributorAuthor

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.

Copy link
Collaborator

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.

Copy link
ContributorAuthor

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?

Copy link
Member

@SteveL-MSFTSteveL-MSFT left a 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.

@microsoft-github-policy-servicemicrosoft-github-policy-servicebot added Waiting on AuthorThe PR was reviewed and requires changes or comments from the author before being accept and removed Review - NeededThe PR is being reviewed labelsOct 2, 2023
@dkaszews
Copy link
ContributorAuthor

@SteveL-MSFT Not sure how to do that, as it would likely require wrapping all diff in anelse/if, to use old way of coloring output of disabled. Also, does it even make sense since the default style matches the original? Alternatively, I could always use the new method of ANSI escapes, but use old (broken) logic of background color detection to pick a theme.

@microsoft-github-policy-servicemicrosoft-github-policy-servicebot removed the Waiting on AuthorThe PR was reviewed and requires changes or comments from the author before being accept labelOct 2, 2023
@microsoft-github-policy-servicemicrosoft-github-policy-servicebot added the Review - NeededThe PR is being reviewed labelOct 9, 2023
@dkaszews
Copy link
ContributorAuthor

@daxian-dbw@kilasuit@iSazonov Do any of you have some ability to push this through so it can make 7.5.0?

@StevenBucher98
Copy link
Collaborator

StevenBucher98 commentedFeb 12, 2024
edited by unfurl-linksbot
Loading

@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

The Experimental Features support in PowerShell provides a mechanism for experimental features to coexist with existing stable features in PowerShell or PowerShell modules.

@dkaszews
Copy link
ContributorAuthor

@StevenBucher98 To confirm, you want me to duplicate all the related code inside a new classV2, as per "mutually exclusive features" section?

@dkaszews
Copy link
ContributorAuthor

@kilasuit@SteveL-MSFT Please merge approved changes

@dkaszews
Copy link
ContributorAuthor

@kilasuit@SteveL-MSFT Please merge approved changes

@kilasuit
Copy link
Collaborator

I don't have maintainer rights, so can't further I'm afraid.

dkaszews reacted with thumbs up emoji

@dkaszews
Copy link
ContributorAuthor

@adityapatwardhan@SeeminglyScience@TravisEz13 You seem to have merge rights, could you please help getting this ancient issue resolved?

@dkaszews
Copy link
ContributorAuthor

@kilasuitkilasuit added the WG-NeedsReviewNeeds a review by the labeled Working Group labelJul 18, 2025
@kilasuit
Copy link
Collaborator

@theJasonHelmick can we add this to the next I-UX WG meeting please to be reviewed.

@floh96
Copy link
Contributor

floh96 commentedJul 18, 2025
edited
Loading

@kilasuit this was already reviewed by the WG see#17961 (comment)
Why does it need to be reviewed again?

Also no response from@theJasonHelmick here#17961 (comment)

@kilasuit
Copy link
Collaborator

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

@dkaszews
Copy link
ContributorAuthor

It was approved over a year ago, just nobody clicked "merge". Not sure if it can still be applied cleanly or requires extensive rebase.

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

@JamesWTruherJamesWTruherJamesWTruher left review comments

@iSazonoviSazonoviSazonov left review comments

@kilasuitkilasuitkilasuit approved these changes

@SteveL-MSFTSteveL-MSFTSteveL-MSFT approved these changes

Labels
LargeReview - NeededThe PR is being reviewedWG-Interactive-Consolethe console experienceWG-NeedsReviewNeeds a review by the labeled Working Group
Projects
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Prompt colors not configurable, not visible on white background
9 participants
@dkaszews@kilasuit@TravisEz13@StevenBucher98@floh96@JamesWTruher@SteveL-MSFT@iSazonov@SeeminglyScience

[8]ページ先頭

©2009-2025 Movatter.jp