Fix format_number function

[MaxVerevkin]

When I fist read the code I thought that format_number should format numbers to engineering notation, but in fact it produces outputs like "0.43MB" instead of "430KB", even if the minimal suffix is set to "K". For me, it doesn't make much sense. Please correct me if the current behavior is intentional.

[ammgws]

For the sake of comparison, what are each of these tests currently returning with the current code?

[MaxVerevkin]

@ammgws

Before

[src/util.rs:562] format_number(1.0, 3, "", "s") = "1.00s"
[src/util.rs:563] format_number(1.007, 3, "", "s") = "1.01s"
[src/util.rs:564] format_number(1.007, 4, "K", "s") = "0.001Ks"
[src/util.rs:565] format_number(1007., 3, "K", "s") = "1.01Ks"
[src/util.rs:566] format_number(107_000., 3, "", "s") = "0.11Ms"
[src/util.rs:567] format_number(107., 3, "", "s") = "0.11Ks"
[src/util.rs:568] format_number(0.000_123_123, 3, "", "N") = "0.12mN"

After

[src/util.rs:562] format_number(1.0, 3, "", "s") = "1.00s"
[src/util.rs:563] format_number(1.007, 3, "", "s") = "1.01s"
[src/util.rs:564] format_number(1.007, 4, "K", "s") = "0.001Ks"
[src/util.rs:565] format_number(1007., 3, "K", "s") = "1.01Ks"
[src/util.rs:566] format_number(107_000., 3, "", "s") = "107Ks"
[src/util.rs:567] format_number(107., 3, "", "s") = "107s"
[src/util.rs:568] format_number(0.000_123_123, 3, "", "N") = "123uN"

[ammgws]

@GladOSkar What do you think?

[GladOSkar]

When i implemented this i based it off the proposed format from #240, which suggests 0.42M instead of 420K. I believe the idea was to always have a decimal point in there such that the entire string always has the same length (given a monospaced font) so the bar doesn't jump around when the value changes.

That said, i don't mind either way. But this could be considered a regression by some.

[MaxVerevkin]

One simple solution I see is to count characters, not digits. For example, if I have total_digits=3 (which should be renamed then), I would get "420K" and "5.3M".

Edit: no, that's bad idea. (but I'm not sure)

[MaxVerevkin]

And maybe make this configurable, because switching between these two behaviors (from this PR and current) is quite easy:

---    let exp_level = (raw_value.log10().div_euclid(3.) as i32)
+++    let exp_level = ((raw_value.log10() + 1.).div_euclid(3.) as i32)

[GladOSkar]

Sounds reasonable, although it could be a bit awkward for non-monospaced fonts and also doesn't read as nicely. But that decision is not up to me @ammgws. I'm fine with either.

[MaxVerevkin]

Well, with non-monospace fonts the bar will change its size either way. @ammgws, @YodaEmbedding what do you think?

[MaxVerevkin]

I will leave it as it is, maybe just refactor it later.

[ammgws]

it produces outputs like "0.43MB" instead of "430KB", even if the minimal suffix is set to "K"

This does seem like it should be fixed though

[MaxVerevkin]

If "430KB" is the expected behavior, then if user wants the bar not to change it's size, he may set speed_min_unit = "M".

[ammgws]

Yeah that's my thought too.

[MaxVerevkin]

By the way, clamp() is stabilized in rust 1.50.0 which releases soon.

[ammgws]

Cheers!