-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Set the cursor to the place where a user hits tab key #7023
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
Conversation
| // Reset the cursor back to where it started | ||
| if(record.RecordType == ProgressRecordType.Completed) | ||
| { | ||
| _progPane.Hide(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do the hide at a pipeline end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. I observed that the hide() gets called when write-progress completes for cmdlets. However for the cloudshell case, the SHiPS provider calls write-progress when a user hits get tab for example; it then calls a Azure cmdlet that triggers write-progress too. When the Azure cmdlet completes, the hide() gets called. PowerShell handles it properly. But when the WriteProgress for ProgressRecordType.Completed gets called from the SHiPS provider, Show() gets called again. At that time, record.RecordType is ProgressRecordType.Completed. After that, no more hide() gets called.
Call stack:
Microsoft.PowerShell.ConsoleHost.dll!Microsoft.PowerShell.ProgressPane.Show() Line 95
Microsoft.PowerShell.ConsoleHost.dll!Microsoft.PowerShell.ProgressPane.Show(Microsoft.PowerShell.PendingProgress pendingProgress) Line 250
Microsoft.PowerShell.ConsoleHost.dll!Microsoft.PowerShell.ConsoleHostUserInterface.HandleIncomingProgressRecord(long sourceId, System.Management.Automation.ProgressRecord record) Line 99
Microsoft.PowerShell.ConsoleHost.dll!Microsoft.PowerShell.ConsoleHostUserInterface.WriteProgress(long sourceId, System.Management.Automation.ProgressRecord record) Line 1324
System.Management.Automation.dll!System.Management.Automation.Internal.Host.InternalHostUserInterface.WriteProgress(long sourceId, System.Management.Automation.ProgressRecord record) Line 609
System.Management.Automation.dll!System.Management.Automation.MshCommandRuntime.WriteProgress(long sourceId, System.Management.Automation.ProgressRecord progressRecord, bool overrideInquire) Line 413
System.Management.Automation.dll!System.Management.Automation.MshCommandRuntime.WriteProgress(System.Management.Automation.ProgressRecord progressRecord, bool overrideInquire) Line 354
System.Management.Automation.dll!System.Management.Automation.MshCommandRuntime.WriteProgress(System.Management.Automation.ProgressRecord progressRecord) Line 324
System.Management.Automation.dll!System.Management.Automation.Cmdlet.WriteProgress(System.Management.Automation.ProgressRecord progressRecord) Line 560
System.Management.Automation.dll!System.Management.Automation.CmdletProviderContext.WriteProgress(System.Management.Automation.ProgressRecord record) Line 851
System.Management.Automation.dll!System.Management.Automation.Provider.CmdletProvider.WriteProgress(System.Management.Automation.ProgressRecord progressRecord) Line 1794
Microsoft.PowerShell.SHiPS.dll!Microsoft.PowerShell.SHiPS.ProgressTracker.End(CodeOwls.PowerShell.Provider.PathNodeProcessors.IProviderContext context) Line 56
Microsoft.PowerShell.SHiPS.dll!Microsoft.PowerShell.SHiPS.PSScriptRunner.InvokeScriptBlock(CodeOwls.PowerShell.Provider.PathNodeProcessors.IProviderContext context, Microsoft.PowerShell.SHiPS.SHiPSDirectory node, Microsoft.PowerShell.SHiPS.SHiPSDrive drive) Line 87
Microsoft.PowerShell.SHiPS.dll!Microsoft.PowerShell.SHiPS.ContainerNodeService.GetNodeChildrenInternal(CodeOwls.PowerShell.Provider.PathNodeProcessors.IProviderContext context) Line 153
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jianyunt Thanks for great clarify! If I understood correctly SHiPS provider does not create a pipeline. I'm still afraid it's a wrong fix - it might break something - we don't have tests to catch this. Perhaps the fix should be in SHiPS provider.
In general 'ProgressRecordType.Completed' status can be important to show a final message to user. With the fix we remove the message. I guess we shouldn't do that.
/cc @daxian-dbw @lzybkr Could you please look the issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe completion, or even the call to PSConsoleHostReadLine (which is how PSReadLine is called from the console host) should set $ProgressPreference = 'SilentlyContinue'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We thought about turning on & off echo for Write-Progress. @BrucePay could you please remind me the story about it?
Fetching data from Azure takes time, so we need to show the progress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@daxian-dbw I remember you and I had a significant discussion when we were trying to improve the performance in this place - there was a lot of non-obvious things. So I try to be very careful here. :-)
@jianyunt Thanks for great discussion!
Seems we get understanding where right place for the fix (link above) is. I haven't possibility to debug on Unix - if you would grab this it would be great. Otherwise I could do it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iSazonov, please go ahead for the fix as I am tied up with other stuff now. Thank you again for the thorough code review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can not repo this:
When the Azure cmdlet completes, the hide() gets called. PowerShell handles it properly. But when the WriteProgress for ProgressRecordType.Completed gets called from the SHiPS provider, Show() gets called again. At that time, record.RecordType is ProgressRecordType.Completed.
with script:
0..1 | % {Write-Progress -Id 0 -Activity 11 -PercentComplete $_;
Sleep 1;
0..1 | % { Write-Progress -Activity 22 -Id 1 -PercentComplete $_; Sleep 1 };
Write-Progress -Activity 22 -Id 1 -Completed };
Write-Progress -Activity 11 -Id 0 -CompletedCould you please make a test script to repo the problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. won't repro. As @BrucePay also mentioned above, this code path doesn't get hit in the cmdlet scenario. It will repro with APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may write an app in C#, call WriteProgress(), then call the above PowerShell script from C#.
Or you may try the following:
On your Linux, install-module azurerm.netcore
install-module azurepsdrive
Login-AzureRmAccount
new-psdrive -name az -psprovider SHiPS -root 'Azurepsdrive#Azure'
cd az:
Get- then tab
you will see the problem
cd az:\AutomationTeam\WebApps
Get- tab, you will see the nested progress issue.
This reverts commit 8805ca2.

PR Summary
Addressed the issue #7022
For the PSReadLine part of issue, see PR PowerShell/PSReadLine#708
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests