Add minimal progress bar using ANSI rendering #14414

SteveL-MSFT · 2020-12-14T16:10:56Z

PR Summary

The current progress bar is large and complex and provides a different experience on Windows vs Linux/macOS. Redo the progress bar to be a single line leveraging ANSI escape sequence to render progress. $PSStyle.Progress now controls configuration:

$PSStyle.Progress.Style is an ANSI string setting the rendering style
$PSStyle.Progress.MaxWidth sets the max width of the view, defaults to 120. Set to 0 for console width.
$PSStyle.Progress.View is an enum with values Minimal and Classic where Classic is existing rendering with no changes. Current default is Minimal.

Example:

Since this visualization is built into the console host, the new settings would not apply to other hosts like EditorServices.

Fix #14426

PR Checklist

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressNode.cs

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressPane.cs

SteveL-MSFT · 2020-12-14T17:39:56Z

Noticed an initial rendering issue with the Expand-Archive example I need to fix

jhoneill · 2020-12-14T18:16:05Z

One of the attractions of the existing progress reporting is it can list things with a fair amount of detail and when they are done, the output goes away. This seems to end up with clutter left behind and the formatting makes it very "attention grabbing"
I would actually want to turn off progress indications if they did that, and would stop using write-progress in my code.

doctordns · 2020-12-14T18:21:23Z

One of the attractions of the existing progress reporting is it can list things with a fair amount of detail and when they are done, the output goes away. This seems to end up with clutter left behind and the formatting makes it very "attention grabbing"
I would actually want to turn off progress indications if they did that, and would stop using write-progress in my code.

This progress bar and pop up stuff is also an issue for me in VS Code. Using VS Code and doing server management pops up all manner of stuff that obscures what is going on in the screen and does not go away. I'd like to see this addressed in general.

Jaykul · 2020-12-14T23:23:59Z

One of the attractions of the existing progress reporting is it can list things with a fair amount of detail and when they are done, the output goes away. This seems to end up with clutter left behind and the formatting makes it very "attention grabbing"

The current implementation is far bigger and more attention grabbing -- I wrote a script just last week using Invoke-RestMethod repeatedly which ended up completely obscuring the more important actual output of the script behind a wall of RestMethod progress reports that weren't cleaned up until the script ended.

Obviously one can always turn off progress output (we already do this on PS5.1 because of performance problems), but I think leaving behind a trace of the progress that was is better than the current situation where the progress is totally gone after the fact --regardless of outcome. Currently scripts cannot rely heavily on progress to report state because if they crash, the information is completely lost.

Having said that, I have two other questions:

Have you tested this with long status strings? There's currently nothing preventing people from writing multiple lines worth of information into a Status or an Activity, right?
Is there an option to still have multi-line output? I'm concerned that using the same space for the text and the progress is going to make this hard to deal with for some users...

SteveL-MSFT · 2020-12-14T23:53:33Z

@Jaykul regarding long status messages, I did test that and the current PR explicitly truncates it adding an ellipsis to the end. This is similar to how content is truncated in other parts of PowerShell formatting. Anyone writing progress needs to be thoughtful on the information being presented (like long file paths). I'd rather than support multi-line output.

Do you have a scenario that requires multi-line progress output? Keep in mind that nested and multi-progress bars is supported, but each bar is limited to one line currently.

jhoneill · 2020-12-15T00:10:04Z

One of the attractions of the existing progress reporting is it can list things with a fair amount of detail and when they are done, the output goes away. This seems to end up with clutter left behind and the formatting makes it very "attention grabbing"

The current implementation is far bigger and more attention grabbing -- I wrote a script just last week using Invoke-RestMethod repeatedly which ended up completely obscuring the more important actual output of the script behind a wall of RestMethod progress reports that weren't cleaned up until the script ended.

The important thing is they are cleaned up when the script ends. The job of Write-Progress is to give the user some indication that the script hasn't hung. That's it. If you want information about what happened that's for Write-Debug, or Write-Verbose, or in the case of people who feel the strong urge to generate lots of display on the console Write-hostwith-foreground`

Obviously one can always turn off progress output (we already do this on PS5.1 because of performance problems), but I think leaving behind a trace of the progress that was is better than the current situation where the progress is totally gone after the fact --regardless of outcome.

There are two different requirements, printing "chatty" output which a programmer expects a user to read, and providing something which says "still working, haven't crashed".

Currently scripts cannot rely heavily on progress to report state because if they crash, the information is completely lost.

Nor should they . Imagine if (say) the copy dialog in explorer remained open to tell you how many bytes / files had been copied at what speed afterwards. You don't care, the tasks was completed without errors, you can get on with the next task without stopping to read about what happened which is past your ability to change.

SteveL-MSFT · 2020-12-15T00:16:01Z

Note that leaving the progress on screen is modeled after various Linux utilities. I can certainly see arguments for both sides.

jhoneill · 2020-12-15T01:51:41Z

Note that leaving the progress on screen is modeled after various Linux utilities. I can certainly see arguments for both sides.

Well yes, the unix world doesn't have verbose, debug, information and progress to write to, so all manner of garbage goes to standard output (and of course there isn't the distinction between "For human eyeballs" and "Output" that PowerShell has)

Being cleaner and tidier is one of the attractions of PowerShell. Getting the error output to be shorter was held to be a Good Thing, yes?
Developers have figured out what should be transient (something for eyeballs while work is happening) generally don't force users to read stuff they don't need to, and make most of the "hey this is what the program has done" stuff verbose / debug. It works well , but nothing is stopping developers writing commands in the style of Linux, just replace write-verbose with write-host . So this proposal doesn't allow anything extra but would take away something good and distinctive.

Anything which has a loop of display progress message, output a row, and remove the progress message will - I suspect - get the messages interleaved with the output.

SteveL-MSFT · 2020-12-15T03:44:59Z

Native commands actually use stderr for the purpose of progress/verbose/warning/error messages and stdout is only for output. In this case, it's no different in that pipelining/redirection (unless stream(s) other than output is specified) only happens for output/stdout. I'm thinking of making whether it stays or clears user settable at least during the experimental phase to get real world usage feedback.

iSazonov · 2020-12-15T04:00:33Z

I'd prefer to keep current (release version) behavior in the PR. I wonder we discuss this in the PR. I think we should discuss this in an issue for all kinds of progress bar. There are some issues with feedback for progress bar. Current design/implementation has fundamental difficulties and we could improve this.

SteveL-MSFT · 2020-12-15T05:41:35Z

Created #14426, let's have the design discussion there and keep discussion here for code review

ghost · 2021-01-02T08:00:09Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment.

…cape sequence

SteveL-MSFT · 2021-01-20T17:31:27Z

Found an issue with how progress is rendered when objects are emitted to pipeline. Looks like I'll have to support clearing in all cases to make this work.

SteveL-MSFT · 2021-01-29T03:25:34Z

Remaining codefactor are existing style or by-design

src/Microsoft.PowerShell.ConsoleHost/host/msh/PendingProgress.cs

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressNode.cs

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressPane.cs

src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs

src/Microsoft.PowerShell.ConsoleHost/host/msh/PendingProgress.cs

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressNode.cs

iSazonov · 2021-02-04T06:11:51Z

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressNode.cs

@@ -336,6 +356,88 @@ private static void RenderFullDescription(string description, string indent, int
                    maxWidth));
        }

+        internal static bool IsMinimalProgressRenderingEnabled()
+        {
+            return ExperimentalFeature.IsEnabled("PSAnsiProgress") && PSStyle.Instance.Progress.View == ProgressView.Minimal;


Why don't we use constants for feature names? "PSAnsiProgress" looks like a puzzle.

Good question, we haven't been doing this.

I'll update it for this feature in this PR, but won't update the other ones. We can follow this pattern going forward.

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressPane.cs

src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs

iSazonov · 2021-02-04T06:40:43Z

src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs

+            /// <summary>
+            /// Gets or sets the style for progress bar.
+            /// </summary>
+            public string Style { get; set; } = "\x1b[33;1m";


Maybe add in the description that is the default.

You mean describe in text what the ANSI escape sequence represents?

iSazonov · 2021-02-04T06:42:00Z

src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs

+            /// <summary>
+            /// Gets or sets the style for progress bar.
+            /// </summary>
+            public ProgressView View { get; set; } = ProgressView.Minimal;


Maybe ViewStyle if it is a style.

Trying not to overload the style term and use it to mean ANSI decoration. Will change the description to view.

Co-authored-by: Ilya <darpa@yandex.ru>

daxian-dbw

LGTM with a few comments.

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressNode.cs

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressPane.cs

src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs

...stem.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs

daxian-dbw · 2021-02-05T18:50:01Z

The CI tests are being flaky recently. I retried the failed CIs twice but both got test failures related to WinCompat:

Additional tests for Import-Module with WinCompat.Tests that ErrorAction/WarningAction have effect when Import-Module with WinCompat is used. Verify that Warning is generated with default WarningAction

Given that the second to last commit had successful CI runs (only CodeFactor failed which can be ignored) and the last commit only made trivial changes to code that is unrelated to the failed WinCompat tests, I will merge this PR.

daxian-dbw · 2021-02-05T18:56:23Z

@SteveL-MSFT Just noticed a small issue with the new progress bar:

When the MaxWidth is not big enough and the message cannot be fully displayed, I expect the message will be truncated with ellipsis ..., but it displays only 1 dot.

daxian-dbw · 2021-02-05T18:59:43Z

Ah, another issue, when MaxWidth is too small to have enough space for the [...] part, progress rendering will fail with exceptions:

PS:12> $PSStyle.Progress.MaxWidth = 10
PS:13> Expand-Archive -Path .\source.zip -DestinationPath C:\arena\tmp\deleteme
Write-Progress: C:\arena\repos\powershell\src\powershell-win-core\bin\debug\net5.0\win7-x64\publish\Modules\Microsoft.PowerShell.Archive\Microsoft.PowerShell.Archive.psm1:1179
Line |
1179 |          Write-Progress -Activity $cmdletName -Status $status -Percent …
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Length cannot be less than zero. (Parameter 'length')

Remove-Item: C:\arena\repos\powershell\src\powershell-win-core\bin\debug\net5.0\win7-x64\publish\Modules\Microsoft.PowerShell.Archive\Microsoft.PowerShell.Archive.psm1:418
Line |
 418 |  …               $expandedItems | % { Remove-Item "$_" -Force -Recurse }
     |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Cannot find path 'C:\arena\tmp\deleteme\PowerShell-7.2.0-preview.1\.devcontainer\' because it
     | does not exist.

Remove-Item: C:\arena\repos\powershell\src\powershell-win-core\bin\debug\net5.0\win7-x64\publish\Modules\Microsoft.PowerShell.Archive\Microsoft.PowerShell.Archive.psm1:418
Line |
 418 |  …               $expandedItems | % { Remove-Item "$_" -Force -Recurse }
     |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Cannot find path 'C:\arena\tmp\deleteme\PowerShell-7.2.0-preview.1\.devcontainer\Dockerfile'
     | because it does not exist.

Remove-Item: C:\arena\repos\powershell\src\powershell-win-core\bin\debug\net5.0\win7-x64\publish\Modules\Microsoft.PowerShell.Archive\Microsoft.PowerShell.Archive.psm1:418
Line |
 418 |  …               $expandedItems | % { Remove-Item "$_" -Force -Recurse }
     |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Cannot find path
     | 'C:\arena\tmp\deleteme\PowerShell-7.2.0-preview.1\.devcontainer\devcontainer.json' because it
     | does not exist.

Remove-Item: C:\arena\repos\powershell\src\powershell-win-core\bin\debug\net5.0\win7-x64\publish\Modules\Microsoft.PowerShell.Archive\Microsoft.PowerShell.Archive.psm1:418
Line |
 418 |  …               $expandedItems | % { Remove-Item "$_" -Force -Recurse }
     |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Cannot find path 'C:\arena\tmp\deleteme\PowerShell-7.2.0-preview.1\.devcontainer\fedora30\'
     | because it does not exist.

Remove-Item: C:\arena\repos\powershell\src\powershell-win-core\bin\debug\net5.0\win7-x64\publish\Modules\Microsoft.PowerShell.Archive\Microsoft.PowerShell.Archive.psm1:418
Line |
 418 |  …               $expandedItems | % { Remove-Item "$_" -Force -Recurse }
     |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Cannot find path
     | 'C:\arena\tmp\deleteme\PowerShell-7.2.0-preview.1\.devcontainer\fedora30\Dockerfile' because it
     | does not exist.

Remove-Item: C:\arena\repos\powershell\src\powershell-win-core\bin\debug\net5.0\win7-x64\publish\Modules\Microsoft.PowerShell.Archive\Microsoft.PowerShell.Archive.psm1:418
Line |
 418 |  …               $expandedItems | % { Remove-Item "$_" -Force -Recurse }
     |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Cannot find path
     | 'C:\arena\tmp\deleteme\PowerShell-7.2.0-preview.1\.devcontainer\fedora30\devcontainer.json'
     | because it does not exist.

Remove-Item: C:\arena\repos\powershell\src\powershell-win-core\bin\debug\net5.0\win7-x64\publish\Modules\Microsoft.PowerShell.Archive\Microsoft.PowerShell.Archive.psm1:418
Line |
 418 |  …               $expandedItems | % { Remove-Item "$_" -Force -Recurse }
     |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Cannot find path 'C:\arena\tmp\deleteme\PowerShell-7.2.0-preview.1\.editorconfig' because it does
     | not exist.

Write-Progress: C:\arena\repos\powershell\src\powershell-win-core\bin\debug\net5.0\win7-x64\publish\Modules\Microsoft.PowerShell.Archive\Microsoft.PowerShell.Archive.psm1:1179
Line |
1179 |          Write-Progress -Activity $cmdletName -Status $status -Percent …
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Length cannot be less than zero. (Parameter 'length')

SeeminglyScience · 2021-02-05T19:00:15Z

Since this visualization is built into the console host, the new settings would not apply to other hosts like EditorServices.

PSES holds onto a copy of ConsoleHost and uses it's implementation of WriteProgress. I don't think it's likely to cause issues, but it will apply there as well.

ghost · 2021-02-12T02:07:22Z

🎉v7.2.0-preview.3 has been released which incorporates this pull request.:tada:

Handy links:

Release Notes

ghost assigned daxian-dbw Dec 14, 2020

iSazonov reviewed Dec 14, 2020

View reviewed changes

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressNode.cs Outdated Show resolved Hide resolved

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressNode.cs Show resolved Hide resolved

src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressPane.cs Outdated Show resolved Hide resolved

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Dec 18, 2020

ghost added the Stale label Jan 2, 2021

SteveL-MSFT added 6 commits January 8, 2021 15:24

Add simpler progress bar using ANSI rendering

37f0719

fix rendering of progress bar

bd050c6

remove unused code, limit progress to 120 chars

e4fec68

fix codefactor issues

b91b75e

restore cursor visibility

3c992bc

Fix initial rendering issue to take into account length of Reverse es…

c98e865

…cape sequence

SteveL-MSFT force-pushed the progress-bar branch from 07c2619 to c98e865 Compare January 8, 2021 23:24

ghost removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept Stale labels Jan 8, 2021

Support configuration for progress

7f18605

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Jan 12, 2021

SteveL-MSFT requested a review from daxian-dbw January 19, 2021 02:52

SteveL-MSFT added 2 commits January 28, 2021 18:48

correctly handle when Progress.MaxWidth > Console.WindowWidth

a09dc88

fix cleanup when done rendering

f693855

SteveL-MSFT mentioned this pull request Jan 29, 2021

Document new ProgressView MicrosoftDocs/PowerShell-Docs#7195

Closed

SteveL-MSFT marked this pull request as ready for review January 29, 2021 03:20

SteveL-MSFT requested a review from anmenaga as a code owner January 29, 2021 03:20

address Codefactor

c0033bc

Remove unnecessary check

60c50ba

rjmholt reviewed Feb 2, 2021

View reviewed changes

address Rob's feedback

5b34d26

rjmholt approved these changes Feb 3, 2021

View reviewed changes

iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Feb 4, 2021

iSazonov reviewed Feb 4, 2021

View reviewed changes

SteveL-MSFT and others added 4 commits February 4, 2021 12:55

Update src/Microsoft.PowerShell.ConsoleHost/host/msh/PendingProgress.cs

e485762

Co-authored-by: Ilya <darpa@yandex.ru>

Update src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressNode.cs

635476f

Co-authored-by: Ilya <darpa@yandex.ru>

Update src/Microsoft.PowerShell.ConsoleHost/host/msh/ProgressPane.cs

69c36af

Co-authored-by: Ilya <darpa@yandex.ru>

address Ilya's feedback

13e5c29

daxian-dbw reviewed Feb 5, 2021

View reviewed changes

address Dongbo's feedback

b05f249

daxian-dbw merged commit 887467e into PowerShell:master Feb 5, 2021

SteveL-MSFT deleted the progress-bar branch February 5, 2021 18:52

daxian-dbw mentioned this pull request Feb 5, 2021

A couple issues in the new progress bar #14717

Closed

iSazonov added this to the 7.2.0-preview.3 milestone Feb 6, 2021

ghost mentioned this pull request Feb 12, 2021

Improve rendering of Progress #14426

Closed

iSazonov mentioned this pull request Apr 20, 2021

Fix Minimal ProgressView to handle Activity that is longer than console width #15264

Merged

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add minimal progress bar using ANSI rendering #14414

Add minimal progress bar using ANSI rendering #14414

SteveL-MSFT commented Dec 14, 2020 •

edited

SteveL-MSFT commented Dec 14, 2020

jhoneill commented Dec 14, 2020

doctordns commented Dec 14, 2020

Jaykul commented Dec 14, 2020

SteveL-MSFT commented Dec 14, 2020

jhoneill commented Dec 15, 2020

SteveL-MSFT commented Dec 15, 2020

jhoneill commented Dec 15, 2020

SteveL-MSFT commented Dec 15, 2020

iSazonov commented Dec 15, 2020

SteveL-MSFT commented Dec 15, 2020

ghost commented Jan 2, 2021

SteveL-MSFT commented Jan 20, 2021

SteveL-MSFT commented Jan 29, 2021

iSazonov Feb 4, 2021

SteveL-MSFT Feb 4, 2021

SteveL-MSFT Feb 4, 2021

iSazonov Feb 4, 2021

SteveL-MSFT Feb 4, 2021

iSazonov Feb 4, 2021

SteveL-MSFT Feb 4, 2021

daxian-dbw left a comment

daxian-dbw commented Feb 5, 2021

daxian-dbw commented Feb 5, 2021

daxian-dbw commented Feb 5, 2021

SeeminglyScience commented Feb 5, 2021

ghost commented Feb 12, 2021

Add minimal progress bar using ANSI rendering #14414

Add minimal progress bar using ANSI rendering #14414

Conversation

SteveL-MSFT commented Dec 14, 2020 • edited

PR Summary

PR Checklist

SteveL-MSFT commented Dec 14, 2020

jhoneill commented Dec 14, 2020

doctordns commented Dec 14, 2020

Jaykul commented Dec 14, 2020

SteveL-MSFT commented Dec 14, 2020

jhoneill commented Dec 15, 2020

SteveL-MSFT commented Dec 15, 2020

jhoneill commented Dec 15, 2020

SteveL-MSFT commented Dec 15, 2020

iSazonov commented Dec 15, 2020

SteveL-MSFT commented Dec 15, 2020

ghost commented Jan 2, 2021

SteveL-MSFT commented Jan 20, 2021

SteveL-MSFT commented Jan 29, 2021

iSazonov Feb 4, 2021

Choose a reason for hiding this comment

SteveL-MSFT Feb 4, 2021

Choose a reason for hiding this comment

SteveL-MSFT Feb 4, 2021

Choose a reason for hiding this comment

iSazonov Feb 4, 2021

Choose a reason for hiding this comment

SteveL-MSFT Feb 4, 2021

Choose a reason for hiding this comment

iSazonov Feb 4, 2021

Choose a reason for hiding this comment

SteveL-MSFT Feb 4, 2021

Choose a reason for hiding this comment

daxian-dbw left a comment

Choose a reason for hiding this comment

daxian-dbw commented Feb 5, 2021

daxian-dbw commented Feb 5, 2021

daxian-dbw commented Feb 5, 2021

SeeminglyScience commented Feb 5, 2021

ghost commented Feb 12, 2021

SteveL-MSFT commented Dec 14, 2020 •

edited