progress-bar: use dynamic size units#14423
Conversation
I like this in general. If the units are the same we could skip intermediate units though and keep the last one. That would make things less verbose and reduce duplication. |
8ba0a5a to
7075567
Compare
|
@xokdvium I've just implemented your suggestion: 
I kind of dislike that it is still kind too quiet. How about adding some kind of spinner like with npm or pnpm that indicates that nix is not frozen and still working? |
7075567 to
09e521e
Compare
Sounds great! That could be a separate PR if you'd like. |
| throw UsageError("'%s' is not an integer", s); | ||
| } | ||
|
|
||
| size_t getSizeUnit(int64_t value); |
There was a problem hiding this comment.
Could we maybe add an enum for the unit? Something like enum class SizeUnit
There was a problem hiding this comment.
like its implemented now?
35e1301 to
ba9fd9b
Compare
5b07a5d to
db9a922
Compare
958bcb5 to
3a081b6
Compare
|
@xokdvium good to go? |
3a081b6 to
0c53c88
Compare
xokdvium
left a comment
There was a problem hiding this comment.
Sounds good. Not a huge fan of the amount of duplication, but that can get cleaned up another time.
|
I've tried to implement it in the first function, but I've failed and just hardcoded the unit in a copy of the function. |
|
Apparently this broke download size rendering. Will be looking into that. |
| return s; | ||
| }; | ||
|
|
||
| auto renderSizeActivity = [&](ActivityType type, const std::string & itemFmt = "%s") { |
There was a problem hiding this comment.
It should have been this:
| auto renderSizeActivity = [&](ActivityType type, const std::string & itemFmt = "%s") { | |
| auto renderSizeActivity = [&] [[nodiscard]] (ActivityType type, const std::string & itemFmt = "%s") { |
So that the bug below couldn't have been introduced
| } | ||
|
|
||
| showActivity(actFileTransfer, "%s MiB DL", "%.1f", MiB); | ||
| renderSizeActivity(actFileTransfer, "%s DL"); |
There was a problem hiding this comment.
This does nothing, since the return value is ignored
This was broken in #14423 accidentally. Add [[nodiscard]] to prevent such mistakes in the future.
|
Fix in #14647 |
Motivation
I've also implemented the changes from #14364 for the progress bar now.
Although it works, I am not quite happy with it.
If I copy something big from one host to another, the total size will probably be >1GiB. The problem now is that the current progress stays as 0 for way too long because the already transferred bytes are still below 0.1GiB.
and after like 10 seconds it looks like that:
This problem could be fixed by not displaying the unit only once for a
X/X/X GiBblock, but instead for every block:X GiB/ X GiB/X GiB.In the reality, this could look like this:
100KiB/25MiB/1GiB. But I am afraid this could be too verbose.Maybe the community or maintainers could express some kind of opinion in the direction this should be going.
Also, this could just be a me-problem, because I am expecting that the number is always changing because it was like that till now.
Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.