-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
Change to Read/Write Neutral Progress Messages for Web Cmdlets WriteToStream() #5078
Conversation
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.
Leave a comment
@@ -184,13 +184,13 @@ | |||
<value>Unable to retrieve certificates because the thumbprint is not valid. Verify the thumbprint and retry. </value> | |||
</data> | |||
<data name="WriteRequestComplete" xml:space="preserve"> | |||
<value>Writing web request completed. (Number of bytes remaining: {0})</value> | |||
<value>Web request processing completed. (Number of bytes remaining: {0})</value> |
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:
<value>Web request completed. (Number of bytes processed: {0})</value>
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.
agreed. fixed.
</data> | ||
<data name="WriteRequestProgressStatus" xml:space="preserve"> | ||
<value>Writing request stream... (Number of bytes written: {0})</value> | ||
<value>Processing... (Number of bytes processed: {0})</value> |
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'd remove repeating "Processing" - "processed".
Maybe:
<value>Number of bytes processed: {0}</value>
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.
agreed. fixed.
@@ -184,13 +184,13 @@ | |||
<value>Unable to retrieve certificates because the thumbprint is not valid. Verify the thumbprint and retry. </value> | |||
</data> | |||
<data name="WriteRequestComplete" xml:space="preserve"> | |||
<value>Writing web request completed. (Number of bytes remaining: {0})</value> | |||
<value>Web request completed. (Number of bytes processed: {0})</value> |
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.
Is the original "remaining" correct? Meaning that it should be "Number of bytes to process"?
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 think it was wrong all along. This string is the message that is sent last when the stream has completed.
PowerShell/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/StreamHelper.cs
Lines 362 to 369 in aea561f
if (cmdlet != null) | |
{ | |
ProgressRecord record = new ProgressRecord(ActivityId, | |
WebCmdletStrings.WriteRequestProgressActivity, | |
StringUtil.Format(WebCmdletStrings.WriteRequestComplete, totalWritten)); | |
record.RecordType = ProgressRecordType.Completed; | |
cmdlet.WriteProgress(record); | |
} |
in 5.1:
$Job = Start-Job -ScriptBlock {
Invoke-WebRequest https://dist.nuget.org/win-x86-commandline/latest/nuget.exe -outfile $env:temp\nuget.exe
} | Wait-Job
$progress = $Job.ChildJobs[0].Progress.ReadAll()
$progress[-2].StatusDescription
$progress[-1].StatusDescription
result:
Writing request stream... (Number of bytes written: 5010552)
Writing web request completed. (Number of bytes remaining: 5010552)
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.
Ok, your change looks good then. Thanks!
Closes #3677
The progress message can be a bit confusing depending on the context in which
WriteToStream()
is called. This PR changes the wording to be Read/Write neutral favoring "Processing" instead.