Skip to content

Add isOutputRedirected parameter to GetFormatStyleString() method #14397

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.

Sign up for GitHub

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

Jump to bottom
Merged
daxian-dbw merged 2 commits into from
Sep 9, 2021
Merged

Add isOutputRedirected parameter to GetFormatStyleString() method #14397

daxian-dbw merged 2 commits into from
Sep 9, 2021

Conversation

@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Dec 11, 2020 •
edited
Loading

PR Summary

Based on this feedback #13758 (review), instead of checking if output is redirected within the engine, made it a parameter so that the console host and pass that information (and other hosts can do the same).

Also fixed an error in $PSStyle.Formatting formatting.

PR Checklist

@ghost ghost assigned iSazonov Dec 11, 2020
@SteveL-MSFT SteveL-MSFT assigned daxian-dbw and unassigned iSazonov Dec 11, 2020
@SteveL-MSFT SteveL-MSFT mentioned this pull request Dec 11, 2020
Merged
14 tasks
@SteveL-MSFT SteveL-MSFT added this to the 7.2-Consider milestone Dec 16, 2020
@ghost ghost removed this from the 7.2-Consider milestone Dec 16, 2020
@ghost
Copy link

ghost commented Dec 16, 2020

Open PRs should not be assigned to milestone, so they are not assigned to the wrong milestone after they are merged. For backport consideration, use a backport label.

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

ghost commented Dec 24, 2020

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

@iSazonov
Copy link
Collaborator

iSazonov commented Mar 6, 2021

The PR should close #14387

@ghost ghost removed the Review - Needed The PR is being reviewed label Mar 6, 2021
@ghost ghost added the Review - Needed The PR is being reviewed label Mar 13, 2021
@ghost
Copy link

ghost commented Mar 13, 2021

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

daxian-dbw
daxian-dbw approved these changes Sep 8, 2021
View reviewed changes
Copy link
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

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

@SteveL-MSFT Sorry that I lost track of this PR. The changes look good to me, but it doesn't address all the issues tracked in #14387.

I think we need to make GetOutputString and GetFormatStyleString public, so that other host implementation can behave just like the console host. Otherwise, other hosts will have to copy the source code of those 2 methods. What do you think?

@daxian-dbw
Copy link
Member

daxian-dbw commented Sep 8, 2021

The markdown test failed due to a broken link, which has nothing to do with this PR.

@ghost ghost removed the Review - Needed The PR is being reviewed label Sep 8, 2021
@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Sep 8, 2021

@daxian-dbw I think we can defer making those APIs public util we have a host that needs it

@daxian-dbw
Copy link
Member

daxian-dbw commented Sep 9, 2021 •
edited
Loading

@SteveL-MSFT The host of PowerShell Jupyter kernel will need them. Once .NET interactive adopts PowerShell 7.2, it will be a matter of time for users to open the issue about $PSStyle not working in the PowerShell sub-kernel.

And, maybe the PSES host will need them too? @rjmholt can you please weigh in.

@daxian-dbw daxian-dbw merged commit 024f409 into PowerShell:master Sep 9, 2021
@daxian-dbw daxian-dbw added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Sep 9, 2021
@daxian-dbw daxian-dbw added this to the 7.2.0-rc.1 milestone Sep 9, 2021
@SteveL-MSFT SteveL-MSFT deleted the isredirected branch September 9, 2021 21:18
@daxian-dbw daxian-dbw mentioned this pull request Sep 10, 2021
Merged
22 tasks
TravisEz13 pushed a commit that referenced this pull request Sep 29, 2021
@SteveL-MSFT @adityapatwardhan
Add isOutputRedirected parameter to GetFormatStyleString() method (…
c140137 
…#14397)
TrapGodBrim pushed a commit to TrapGodBrim/PowerShell that referenced this pull request Jan 19, 2022
@SteveL-MSFT @TrapGodBrim
Add isOutputRedirected parameter to GetFormatStyleString() method (…
8297203 
…PowerShell#14397)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw daxian-dbw daxian-dbw approved these changes

@anmenaga anmenaga Awaiting requested review from anmenaga

Assignees

@daxian-dbw daxian-dbw

Labels

Backport-7.2.x-Done CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Milestone

7.2.0-preview.10

Development

Successfully merging this pull request may close these issues.

4 participants

@SteveL-MSFT @iSazonov @daxian-dbw @adityapatwardhan