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.

Sign up for GitHub

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

Jump to bottom

(GH-872) Progress output never reaches completion #873

Closed
wants to merge 3 commits into from
Closed

(GH-872) Progress output never reaches completion #873

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8236122
(GH-872) Progress output never reaches completion
Jul 25, 2016
dd0d0ce
(GH-872) Progress output logs completion twice
Jul 25, 2016
3b3b1a0
(GH-875) Pad progress output line to console width
Jul 26, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/chocolatey/infrastructure/powershell/PoshHostUserInterface.cs
View file
Open in desktop
Expand Up @@ -129,7 +129,7 @@ public override void WriteProgress(long sourceId, ProgressRecord record)
}

// http://stackoverflow.com/a/888569/18475
Console.Write("\rProgress: {0}% - {1}".format_with(record.PercentComplete.to_string(), record.StatusDescription.PadRight(50, ' ')));
Console.Write("\rProgress: {0}% - {1}".format_with(record.PercentComplete.to_string(), record.StatusDescription).PadRight(Console.WindowWidth));
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Author

Choose a reason for hiding this comment

The 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
Source: http://referencesource.microsoft.com/mscorlib/a.html#19f2a20d8adf969c

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.

choco install -y 7zip --allow-unofficial > C:\users\bill\desktop\output.log

Produces this file:

Downloading 7zip.install 64 bit
  from 'http://www.7-zip.org/a/7z1602-x64.exe'

Progress: 6% - Saving 92.41 KB of 1.31 MB (94632/1378550)
Progress: 19% - Saving 266.36 KB of 1.31 MB (272752/1378550)
Progress: 29% - Saving 397.53 KB of 1.31 MB (407072/1378550)
Progress: 52% - Saving 705.5 KB of 1.31 MB (722432/1378550)
Progress: 72% - Saving 980.68 KB of 1.31 MB (1004212/1378550)
Progress: 85% - Saving 1.13 MB of 1.31 MB (1180872/1378550)
Progress: 100% - Saving 1.31 MB of 1.31 MB (1378550/1378550)
Progress: 100% - Completed download of C:\Users\bill\AppData\Local\Temp\chocolatey\7zip.install\16.02\7z1602-x64.exe (1.31 MB).
Download of 7z1602-x64.exe (1.31 MB) completed.

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.

Copy link
Member

Choose a reason for hiding this comment

The 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).

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

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

Console adapter - https://github.com/chocolatey/choco/pull/873/files#diff-efd76cbe4b6a793d6856110d32039ec7R34

Copy link
Member

Choose a reason for hiding this comment

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

#572

https://github.com/chocolatey/choco/blob/master/src/chocolatey/infrastructure/adapters/Console.cs#L188

Copy link
Author

Choose a reason for hiding this comment

The 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!

Copy link
Member

Choose a reason for hiding this comment

The 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)
Expand Down