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

Appearance settings

Delay progress bar in Copy-Item and Remove-Item cmdlets#24013

Merged
iSazonov merged 10 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
TheSpyGod:masterTheSpyGod/PowerShell:masterCopy head branch name to clipboard
Aug 20, 2024
Merged

Delay progress bar in Copy-Item and Remove-Item cmdlets#24013
iSazonov merged 10 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
TheSpyGod:masterTheSpyGod/PowerShell:masterCopy head branch name to clipboard

Conversation

@TheSpyGod

Copy link
Copy Markdown
Contributor

PR Summary

I've optimized the Progress Bar of the Copy and Remove commands to display only after the operation lasts more then 200ms, otherwise do the operation without a Progress Bar, I've done this by adding to the IF conditional of IF _copyStopwatch or _removeStopwatch Elapsed time is higher then 0.2 seconds(200ms) add the Progress Bar to the process.

PR Context

Fix #23831

PR Checklist

TheSpyGod and others added 4 commits July 3, 2024 22:30
I intend to remove the progressbar for less then or equal to 5 file, to make it less anoying TODO: Check for errors when running Remove-Item
Made it so that the progress bar for removing only appears after the _removeStopwatch reaches 600ms
Made the copy and remove command display the Progress bar only after the process is lasting more then 200ms
@TheSpyGod TheSpyGod requested a review from anmenaga as a code owner July 4, 2024 19:15
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs
TheSpyGod and others added 2 commits July 5, 2024 14:29
…econds

Made it so that instead of skipping it all together, only the instance will not be shown until 3 seconds elapse, the calculations will still continue
Changed the Creating of the Progress Bar's instance to be after 3 seconds
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated
TheSpyGod and others added 2 commits July 5, 2024 19:41
Fixed Indentation for IF as Guidelines Say and Switched the Duration const to PascalCase
Fixed Indentation, Removed Empty Spaces, and Switched Const to PascalCase
@microsoft-github-policy-service microsoft-github-policy-service Bot added the Review - Needed The PR is being reviewed label Jul 13, 2024
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs
@microsoft-github-policy-service microsoft-github-policy-service Bot added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Review - Needed The PR is being reviewed labels Aug 5, 2024
@SteveL-MSFT

Copy link
Copy Markdown
Member

@TheSpyGod if you don't want to agree to the CLA, I'll submit a new PR to address this

@SteveL-MSFT SteveL-MSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fixed the indentation issue

@TheSpyGod

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@microsoft-github-policy-service microsoft-github-policy-service Bot removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 13, 2024
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs
@SteveL-MSFT

Copy link
Copy Markdown
Member

@iSazonov can you take another look? I've pushed changes you've requested as I'd like to get this into 7.5 as it annoys me when doing small copies/removes. I've also reduced the threshold to 2 secs intead of 3. After building the branch privately, 3 secs seems way too long.

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Aug 20, 2024

@iSazonov iSazonov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@SteveL-MSFT

ProgressBarDurationThreshold = 2.0;

Classic definition for interactive scenarios is 5 - 15 seconds. My personal preference is gold middle 10s for the constant. (Something like Exchange Server PowerShell console does.) Otherwise I expect users will see flapped lines. Nevertheless I merge the PR so we get early feedback and adjust the constant before release.

@iSazonov iSazonov changed the title Optimized Progress Bar, Displays based on Operation Duration Delay progress bar in Copy-Item and Remove-Item cmdlets Aug 20, 2024
@iSazonov iSazonov merged commit be59e5e into PowerShell:master Aug 20, 2024
@microsoft-github-policy-service

microsoft-github-policy-service Bot commented Aug 20, 2024

Copy link
Copy Markdown
Contributor

📣 Hey @TheSpyGod, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

@iSazonov

Copy link
Copy Markdown
Collaborator

@TheSpyGod Thanks for your contribution.

@kilasuit

Copy link
Copy Markdown
Collaborator

This PR seems to have fixed #25679 and like @SteveL-MSFT mentions needs backporting to 7.5.x releases
Though can we be sure the issue in #25679 wouldn't just reoccur with a longer duration at all?

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

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove-Item progress should only be displayed if > 5 items

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.