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

Fix regression around OnIdle handler #1936

Merged
merged 2 commits into from
Oct 24, 2022
Merged

Fix regression around OnIdle handler #1936

merged 2 commits into from
Oct 24, 2022

Conversation

andyleejordan
Copy link
Member

Unfortunately the (admittedly hairy) change made in #1918 to fix:

System.Management.Automation.PSInvalidOperationException: The pipeline was not run because a pipeline is already running. Pipelines cannot be run concurrently.

has resulted in PowerShell/vscode-powershell#4219:

System.Management.Automation.PSInvalidOperationException: You should only run a nested pipeline from within a running pipeline.

because we had hacked in that all our pipelines are nested (to allow the concurrency check to pass), but when loading profiles, we do not actually have a running pipeline, so our lie about being nested fails another check.

So I think what happened was that PowerShell/vscode-powershell#4048 was already no longer reproducing by happenstance (as the Az.Tools.Predictor module presumably updated and removed their OnIdle handler). I failed to check that before attempting to fix it following the last result of our investigation. The "fix" inadvertently caused nearly the same PSInvalidOperationException, and by mistake I confused the two scenarios. With this PR, and regression tests, I can confirm that the OnIdle handler works both in a profile and at the regular prompt after reverting the breaking change. Furthermore, I can no longer repro the original issue with Az.Tools.Predictor whatsoever, in any version, hence my conclusion their module updated.

Resolves PowerShell/vscode-powershell#4219

@andyleejordan andyleejordan requested a review from a team October 24, 2022 21:25
Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

LGTM!

@andyleejordan andyleejordan merged commit b0bf31b into main Oct 24, 2022
@andyleejordan andyleejordan deleted the andschwa/onidle branch October 24, 2022 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Engine Issue-Bug A bug to squash.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Starting PowerShell extension from VSCode results in PSReadLine error
2 participants