Skip to content

Conversation

@rami3l
Copy link
Member

@rami3l rami3l commented Nov 29, 2025

In #4604, the total_bytes fields seem to be inconsistently left-aligned in some places, and right-aligned in others. As such, scenarios like the following will occur from time to time (note the cargo line):

t-y

Thus, it is required to make them right-aligned with :>, however it seems (from my testing) that specifying the alignment without a width is a no-op, so a width of 10 11 is deduced from the longest string from my observation 123.45 MiB 1023.00 KiB.

However, since the rest of the codebase assumes a width of 9, I have manually increased every appearance of explicit width by a value of 1 2 for the sake of coherence.

This gives something like:

image

@rami3l rami3l requested review from ChrisDenton and djc November 29, 2025 12:14
@rami3l
Copy link
Member Author

rami3l commented Nov 29, 2025

@djc Thanks for the quick response! Do you happen to have any info proving my point that the field is no longer than 123.45 MiB? That's just from my observation so I'm not 100% sure about that :)

@djc
Copy link
Contributor

djc commented Nov 29, 2025

Seems unlikely that we'll have 1GB components, so probably fine for now?

@rami3l
Copy link
Member Author

rami3l commented Nov 29, 2025

Oh no, just found out myself and that's gross... indicatif::HumanBytes(1023 * 1024).to_string() gives 1023.00 KiB so the maximum width is 11. However this is going to make the correct gap look even bigger than in the above image. I'll see what this will actually look like in the current setting though.

Update: Looking at https://github.com/console-rs/indicatif/blob/95088ffd980a6f0cdd6571418fd9c6d6fd1603d1/src/style.rs#L740 it does look like I'll need to bump the width to 11. Sorry!

A maximal width of 11 is deduced from `1023.00 KiB` which should be the
longest possible string for the `total_bytes` field given the current
component sizes.
@rami3l rami3l force-pushed the fix/progress-align branch from 6ea935d to d75bbf4 Compare November 29, 2025 13:15
@rami3l rami3l enabled auto-merge November 29, 2025 13:20
@rami3l rami3l added this pull request to the merge queue Nov 29, 2025
Merged via the queue into rust-lang:main with commit 526cb43 Nov 29, 2025
29 checks passed
@rami3l rami3l deleted the fix/progress-align branch November 29, 2025 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants