GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Add minimal progress bar using ANSI rendering by SteveL-MSFT · Pull Request #14414 · PowerShell/PowerShell Conversation
|
|
Noticed an initial rendering issue with the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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" |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Joel Bennett (@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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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-host
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".
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Note that leaving the progress on screen is modeled after various Linux utilities. I can certainly see arguments for both sides. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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? 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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Created #14426, let's have the design discussion there and keep discussion here for code review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
07c2619 to
c98e865
Compare
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Remaining codefactor are existing style or by-design |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
|
||
| internal static bool IsMinimalProgressRenderingEnabled() | ||
| { | ||
| return ExperimentalFeature.IsEnabled("PSAnsiProgress") && PSStyle.Instance.Progress.View == ProgressView.Minimal; |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Why don't we use constants for feature names? "PSAnsiProgress" looks like a puzzle.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Good question, we haven't been doing this.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I'll update it for this feature in this PR, but won't update the other ones. We can follow this pattern going forward.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| /// <summary> | ||
| /// Gets or sets the style for progress bar. | ||
| /// </summary> | ||
| public string Style { get; set; } = "\x1b[33;1m"; |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Maybe add in the description that is the default.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
You mean describe in text what the ANSI escape sequence represents?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| /// <summary> | ||
| /// Gets or sets the style for progress bar. | ||
| /// </summary> | ||
| public ProgressView View { get; set; } = ProgressView.Minimal; |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Maybe ViewStyle if it is a style.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Trying not to overload the style term and use it to mean ANSI decoration. Will change the description to view.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Co-authored-by: Ilya <[email protected]>
Co-authored-by: Ilya <[email protected]>
Co-authored-by: Ilya <[email protected]>
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a few comments.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
The CI tests are being flaky recently. I retried the failed CIs twice but both got test failures related to
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Steve Lee (@SteveL-MSFT) Just noticed a small issue with the new progress bar: When the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ah, another issue, when |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
PSES holds onto a copy of |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
🎉 Handy links: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Improve rendering of Progress
There was an error while loading. Please reload this page.