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
(GH-872) Progress output never reaches completion #873
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,26 +118,18 @@ public override void WriteDebugLine(string message) | |
} | ||
|
||
private bool hasLoggedStartProgress = false; | ||
private bool hasLoggedFinalProgress = false; | ||
public override void WriteProgress(long sourceId, ProgressRecord record) | ||
{ | ||
if (record.PercentComplete == -1) return; | ||
if (hasLoggedFinalProgress) return; | ||
|
||
if (!hasLoggedStartProgress) | ||
{ | ||
hasLoggedStartProgress = true; | ||
this.Log().Debug(record.Activity.escape_curly_braces()); | ||
} | ||
|
||
// http://stackoverflow.com/a/888569/18475 | ||
Console.Write("\rProgress: {0}% - {1}".format_with(record.PercentComplete.to_string(), record.StatusDescription.PadRight(50, ' '))); | ||
|
||
if (record.PercentComplete == 100 && !hasLoggedFinalProgress) | ||
{ | ||
hasLoggedFinalProgress = true; | ||
//this.Log().Info(""); | ||
//this.Log().Info(record.StatusDescription.Replace("Saving","Finished downloading. Saved")); | ||
} | ||
Console.Write("\rProgress: {0}% - {1}".format_with(record.PercentComplete.to_string(), record.StatusDescription).PadRight(Console.WindowWidth)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where does this pull console.windowwidth from? There are issues with redirects. As long as it is using the Console abstraction, it should work fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's System.Console.WindowWidth. Docs: https://msdn.microsoft.com/en-us/library/system.console.windowwidth(v=vs.110).aspx According to the source code it is possible for it to throw an exception if we can't get a console handle, but I haven't been able to produce that state. I tested redirecting to a file from both powershell and cmd. I tested on Powershell 2.0 as well, and I tested Boxstarter, since it captures the choco output.
Produces this file:
Can you think of other scenarios I should test? I suppose we could throw some exception handling around it just in case, but so far I haven't found a scenario where that would be needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can repro it. Let me find the issue and why you want to use Console adapter. Bonus is you are probably using it and don't realize it (unless you went and clicked on go to implementation aleady). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was able to successfully repro issues with console handling in the beta. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh you're right, I was calling the adapter already without realizing it. Neat! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's kind of the point of the adapters. 👍 |
||
} | ||
|
||
public override void WriteVerboseLine(string message) | ||
|
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.
@bill-long ^^