Skip to content
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.

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

Integrated Console - Write-Progress messes things up #3423

Closed
Tiberriver256 opened this issue Jun 17, 2021 · 11 comments
Closed

Integrated Console - Write-Progress messes things up #3423

Tiberriver256 opened this issue Jun 17, 2021 · 11 comments
Assignees
Labels
Area-Extension Terminal Issue-Bug A bug to squash. Resolution-Fixed Will close automatically.

Comments

@Tiberriver256
Copy link

Issue Description

Running a script with Write-Progress screws up the console.

Example Script

function MyAwesomeScript {

	[CmdletBinding()]
	param (
		[Parameter()]
		[string]
		$Name
	)

	$WelcomeMessage = @();

	for ($i = 0; $i -lt 50; $i++) {
		Write-Progress -Activity "Getting ready to greet you!" -PercentComplete ($i/50*100)
		$WelcomeMessage += "Hello $Name"
	}

	$WelcomeMessage
}

MyAwesomeScript -Name Micah

Expected Behaviour

Should look like this (example taken from pwsh running in VSCode):

image

Actual Behaviour

Looks like this:

image

The prompt is getting rendered towards the top instead of at the end of the content

@ghost ghost added the Needs: Triage Maintainer attention needed! label Jun 17, 2021
@andyleejordan andyleejordan added Area-Extension Terminal Issue-Bug A bug to squash. and removed Needs: Triage Maintainer attention needed! labels Jun 29, 2021
@andyleejordan
Copy link
Member

That is indeed a bug, thanks for reporting it. It's unfortunately a bit difficult to fix as its a custom PowerShell host. We will look into this.

@andyleejordan
Copy link
Member

@rjmholt thinks this a race condition between Write-Progress and the printing of the prompt. This may be fixed by PowerShell/PowerShellEditorServices#1295

@andyleejordan andyleejordan added this to the Consider-vNext milestone Jun 29, 2021
@Tiberriver256
Copy link
Author

🤞

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label Jun 29, 2021
@andyleejordan andyleejordan removed the Needs: Maintainer Attention Maintainer attention needed! label Jul 13, 2021
@andyleejordan
Copy link
Member

Hello, and thank you for your patience! The latest PowerShell Preview for VS Code is now out, and includes the reworking of our pipeline and threading architecture in PowerShell/PowerShellEditorServices#1295. Could you verify if this issue still reproduces using v2021.10.3-preview? Please note that this preview is likely to include other bugs, and you should feel free to file new issues for those so we can work through them. Thanks again!

@andyleejordan andyleejordan added the Needs: Author Feedback Please give us the requested feedback! label Oct 29, 2021
@Tiberriver256
Copy link
Author

Thanks for reaching out. Looks like it's still an issue in the preview version.

image

@ghost ghost added Needs: Maintainer Attention Maintainer attention needed! and removed Needs: Author Feedback Please give us the requested feedback! labels Oct 29, 2021
@StevenBucher98 StevenBucher98 added Up for Grabs Will shepherd PRs. and removed Needs: Maintainer Attention Maintainer attention needed! labels Nov 9, 2021
@StevenBucher98
Copy link
Contributor

Thanks for checking @Tiberriver256! We will leave this issue open and continue to track it.

@SeeminglyScience
Copy link
Collaborator

A couple of things missing from the old host implementation:

https://github.com/PowerShell/PowerShellEditorServices/blob/6b1e77d6223ff61c4af3cbeedf687e1f65f3ac18/src/PowerShellEditorServices/Services/PowerShellContext/Session/Host/EditorServicesPSHostUserInterface.cs#L609-L622

Track current progress messages so they can be cleared up when a the top level pipeline completes. Also a little bit under that it's hooking into an execution status event to call ClearProgress after pipeline completion.


Since it's not very obvious why you'd need to do that, I'll copy and paste an explanation from when I was suggesting it as the fix:

I still think the best bet is to reuse the existing ConsoleHost. I tested it, it does work. The problem is that since ConsoleHost isn't the one creating pipelines, it isn't cleaning up progress messages after each pipeline invocation. There's a few options

  1. Keep track of the progress messages we send to ConsoleHost and send clean up progress messages for any that the user did not. This would be my suggestion (assuming it works)

(it does)

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label Nov 10, 2021
@andyleejordan andyleejordan self-assigned this Nov 10, 2021
@andyleejordan andyleejordan removed the Needs: Maintainer Attention Maintainer attention needed! label Nov 10, 2021
@andyleejordan
Copy link
Member

Hey so the good news is that this appears fixed in the latest Preview: the prompt now shows up where it should (the end). There is a remaining bug, but it's #3807 (Write-Progress doesn't quite finish).

@andyleejordan
Copy link
Member

Screen Shot 2022-02-18 at 10 47 41 AM

@ghost ghost closed this as completed Feb 18, 2022
@ghost
Copy link

ghost commented Feb 18, 2022

This issue has been marked as fixed. It has been automatically closed for housekeeping purposes.

@andyleejordan
Copy link
Member

The remaining fix for this is out in PowerShell Preview v2022.2.2!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Extension Terminal Issue-Bug A bug to squash. Resolution-Fixed Will close automatically.
Projects
None yet
Development

No branches or pull requests

4 participants