Add customize output color enhancement #654

KeroroLulu · 2018-04-20T00:55:11Z

With this, if the user change his PrivateData's color properties it will be print with the color that the user choose.

Edit

Sorry I touched .travis.ymlbecause I had some problem with travis but finally it didn't change. To resolve this problem I just put XML comment, that was from the copy/past from PowerShell repositorie.

fixes #651

msftclas · 2018-04-20T00:55:22Z

All CLA requirements met.

Supress Warnings in travis Supress Warning in Travis PowerShell#2 Test for travis Add Comment XML for travis Add Comment for Testing

TylerLeonhardt · 2018-04-20T07:08:51Z

Nice work @KeroroLulu 🎉 looking forward to reviewing this!

SeeminglyScience

Thank you so much for taking this one @KeroroLulu! A couple minor requests due to some differences in this project versus Core.

SeeminglyScience · 2018-04-20T11:25:58Z

src/PowerShellEditorServices/Session/Host/EditorServicesPSHost.cs

-        ///  
+        ///
+        /// </summary>
+        public class ConsoleColorProxy


This should be internal class ConsoleColorProxy since the only intended way to access it is via the PSObject. The empty documentation comment can then be removed as well

@SeeminglyScience Sorry I delete my comments in your review, afterwards I notice that the error I get with travis was from public members. I didn't notice that it work with internal members. And when I pushed that change AppVeyor worked I don't know why it was stuck for 20 minute.

AppVeyor and Travis sometimes have issues (not our issues) and it's just a matter of restarting the build and it works. 😀

SeeminglyScience · 2018-04-20T11:27:26Z

src/PowerShellEditorServices/Session/Host/EditorServicesPSHost.cs

+            /// <summary>
+            ///
+            /// </summary>
+            public ConsoleColorProxy(EditorServicesPSHostUserInterface hostUserInterface)


This should be internal ConsoleColorProxy(EditorServicesPSHostUserInterface hostUserInterface) , the empty documentation comment can then be removed as well

SeeminglyScience · 2018-04-20T11:29:16Z

src/PowerShellEditorServices/Session/Host/EditorServicesPSHost.cs

+            /// </summary>
+            public ConsoleColor ErrorForegroundColor
+            {
+                [SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")]


All of the diagnostic suppressions (SuppressMessage) can be removed since we don't have the same analyzers on the project

SeeminglyScience · 2018-04-20T11:30:56Z

src/PowerShellEditorServices/Session/Host/EditorServicesPSHostUserInterface.cs

+        /// <summary>
+        ///
+        /// </summary>
+        public static ConsoleColor BackgroundColor


All of the color properties here can be internal, the empty documentation comments can then be removed as well.

SeeminglyScience · 2018-04-20T11:31:46Z

src/PowerShellEditorServices/Session/Host/EditorServicesPSHostUserInterface.cs

+        {
+            get;
+            set;
+        }


This should follow the same style as below (e.g. public static ConsoleColor BackgroundColor { get; set; })

SeeminglyScience · 2018-04-20T11:33:07Z

src/PowerShellEditorServices/Session/Host/EditorServicesPSHost.cs

@@ -6,6 +6,7 @@
 using Microsoft.PowerShell.EditorServices.Session;
 using Microsoft.PowerShell.EditorServices.Utility;
 using System;
+using System.Diagnostics.CodeAnalysis;


This can be removed after the SuppressMessage attributes are removed.

I added all internal class and suppress all the SuppressMessage. But I can't remove the empty documentation, the analysis of PowerShell Editor Services doesn't let it pass. I tried to add the suppress warning in it but it doesn't work as well. So I had to add all documentation.

Sorry I didn't notice that was only for public members until I read the error again.

TylerLeonhardt · 2018-04-20T15:13:23Z

@KeroroLulu do you think you could upload a picture of it working? I'm really curious to see it 😃

KeroroLulu · 2018-04-20T15:26:39Z

@tylerl0706 Here the screenshot :

I changed the value of PrivateData and then print the error and it uses the new color.

Edit

I did it from Visual Studio Code.

TylerLeonhardt

LGTM!! 🎉

Awesome work @KeroroLulu! Thank you so much for contributing 😀

You're going to make a lot of folks happy with this change because VSCode needed a way to change colors of errors so that when people give presentations there's enough contrast.

I will wait to hear what @SeeminglyScience has to say about your revisions 😁

TylerLeonhardt · 2018-04-20T18:21:12Z

src/PowerShellEditorServices/Session/Host/EditorServicesPSHost.cs

+
+            /// <summary>
+            ///
+            /// </summary>


Can you either remove these summaries or add like

"The ForegroundColor for Errors"
etc.

I will add text, I think that your configuration of travis doesn't accept warning from XML documentation for public members

sounds good!

rjmholt · 2018-04-20T23:18:40Z

~~I think we need to make sure this compiles on .NET Core~~ Just realised Travis does that. I was under the impression that ConsoleColor was .NET Framework only for some reason...

rjmholt · 2018-04-20T23:22:25Z

Looks good to me!

TylerLeonhardt · 2018-04-21T01:58:15Z

verified this on macOS 😁

KeroroLulu · 2018-04-21T04:14:16Z

I'm on MacOS too x)

SeeminglyScience · 2018-04-21T11:57:27Z

Changes look great! Thanks again this is a big help :)

KeroroLulu · 2018-04-21T12:14:02Z

Should I squash my commit ?

TylerLeonhardt · 2018-04-21T15:34:28Z

Merged 😄 Thanks again!!

I tweeted about the PR here:
https://twitter.com/TylerLeonhardt/status/987715764248498176

KirkMunro · 2018-04-21T22:12:58Z

@KeroroLulu Thank you very much for this improvement! This is something that has been needed in VS Code for a long time! 😀

TylerLeonhardt · 2018-04-21T23:46:46Z

It will be available in the next release which I'm going to try to get out next week :)

KeroroLulu force-pushed the master branch from ee228ac to 67b8dd2 Compare April 20, 2018 02:22

Add customize output color enhancement (PowerShell#651)

e2a32aa

Supress Warnings in travis Supress Warning in Travis PowerShell#2 Test for travis Add Comment XML for travis Add Comment for Testing

KeroroLulu force-pushed the master branch from 67b8dd2 to e2a32aa Compare April 20, 2018 02:23

KeroroLulu changed the title ~~Add customize output color enhancement (#651)~~ Add customize output color enhancement Apr 20, 2018

TylerLeonhardt requested review from rkeithhill, TylerLeonhardt, rjmholt and SeeminglyScience April 20, 2018 07:11

SeeminglyScience requested changes Apr 20, 2018

View reviewed changes

GUIGHIL benjamin added 2 commits April 20, 2018 14:52

Suppress Documentation for internal members

d300720

Sorry I didn't notice that was only for public members until I read the error again.

TylerLeonhardt approved these changes Apr 20, 2018

View reviewed changes

Add text for color sumary

ce13e6d

SeeminglyScience approved these changes Apr 21, 2018

View reviewed changes

TylerLeonhardt merged commit e37f1f2 into PowerShell:master Apr 21, 2018

SeeminglyScience mentioned this pull request Sep 8, 2019

Integrated console should allow setting Write-* message colors through PrivateData PowerShell/vscode-powershell#571

Closed

SeeminglyScience mentioned this pull request Nov 19, 2019

Warning stream color PowerShell/vscode-powershell#2317

Closed

rjmholt mentioned this pull request May 10, 2021

Reimplement the way PowerShell is run and hosted in PSES with a dedicated pipeline thread consumer #1459

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add customize output color enhancement #654

Add customize output color enhancement #654

KeroroLulu commented Apr 20, 2018 •

edited by TylerLeonhardt

Loading

msftclas commented Apr 20, 2018 •

edited

Loading

TylerLeonhardt commented Apr 20, 2018 •

edited

Loading

SeeminglyScience left a comment

SeeminglyScience Apr 20, 2018

KeroroLulu Apr 20, 2018 •

edited

Loading

TylerLeonhardt Apr 20, 2018

SeeminglyScience Apr 20, 2018

SeeminglyScience Apr 20, 2018

SeeminglyScience Apr 20, 2018

SeeminglyScience Apr 20, 2018

SeeminglyScience Apr 20, 2018

TylerLeonhardt commented Apr 20, 2018

KeroroLulu commented Apr 20, 2018 •

edited

Loading

TylerLeonhardt left a comment •

edited

Loading

TylerLeonhardt Apr 20, 2018

KeroroLulu Apr 20, 2018

TylerLeonhardt Apr 20, 2018

rjmholt commented Apr 20, 2018 •

edited

Loading

rjmholt commented Apr 20, 2018

TylerLeonhardt commented Apr 21, 2018

KeroroLulu commented Apr 21, 2018

SeeminglyScience commented Apr 21, 2018

KeroroLulu commented Apr 21, 2018

TylerLeonhardt commented Apr 21, 2018

KirkMunro commented Apr 21, 2018

TylerLeonhardt commented Apr 21, 2018

Add customize output color enhancement #654

Add customize output color enhancement #654

Conversation

KeroroLulu commented Apr 20, 2018 • edited by TylerLeonhardt Loading

msftclas commented Apr 20, 2018 • edited Loading

TylerLeonhardt commented Apr 20, 2018 • edited Loading

SeeminglyScience left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KeroroLulu Apr 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TylerLeonhardt commented Apr 20, 2018

KeroroLulu commented Apr 20, 2018 • edited Loading

TylerLeonhardt left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjmholt commented Apr 20, 2018 • edited Loading

rjmholt commented Apr 20, 2018

TylerLeonhardt commented Apr 21, 2018

KeroroLulu commented Apr 21, 2018

SeeminglyScience commented Apr 21, 2018

KeroroLulu commented Apr 21, 2018

TylerLeonhardt commented Apr 21, 2018

KirkMunro commented Apr 21, 2018

TylerLeonhardt commented Apr 21, 2018

KeroroLulu commented Apr 20, 2018 •

edited by TylerLeonhardt

Loading

msftclas commented Apr 20, 2018 •

edited

Loading

TylerLeonhardt commented Apr 20, 2018 •

edited

Loading

KeroroLulu Apr 20, 2018 •

edited

Loading

KeroroLulu commented Apr 20, 2018 •

edited

Loading

TylerLeonhardt left a comment •

edited

Loading

rjmholt commented Apr 20, 2018 •

edited

Loading