Shorthand HumanFloatCount and HumanCount

[ReagentX]

• Use # to specify new alternative shorthand formatter for HumanFloatCount and HumanCount
  • Use width to truncate HumanCount values
  • Resolve Feature Req.: Human readable per_sec rate #637
• Add human_pos_formatted, human_len_formatted, and per_sec_short

Examples:

"{spinner:.green} [{elapsed}] [{bar:.blue}] {human_pos}/{human_len} ({per_sec_formatted}, ETA: {eta})"

"{spinner:.green} [{elapsed}] [{bar:.blue}] {human_pos:3}/{human_len:3} ({per_sec_formatted}, ETA: {eta})"

"{spinner:.green} [{elapsed}] [{bar:.blue}] {human_pos:2}/{human_len:2} ({per_sec}, ETA: {eta})"

[ReagentX]

This includes changes from #696 but can be implemented without them if required.

[djc]

@chris-laplante I'm inclined to think that the human_ prefix gives us a certain amount of latitude to change things, what do you think? I feel like adding this as yet another separate axis is not so compelling and we should instead change the human_* fields to do something like this by default.

Personally I think we should try to keep the number of significant digits constant, so that we get 123, 1.23 or 12.3 but not 12k (should be 12.0k) or 12.34M (should also be 12.3M).

Also please rebase and squash into clean commits (potentially just one in this case).

The assertion style with outcome, input seems unidiomatic but maybe that's pre-existing?

[ReagentX]

I'm inclined to think that the human_ prefix gives us a certain amount of latitude to change things

That would break existing templates; this PR retains existing functionality and only adds new features. Old templates will behave the same as how they did before.

If you have a specification in mind I am more than happy to implement it.

The assertion style with outcome, input seems unidiomatic

I agree; I just copied the design of what was already in the repo for both tests and code to maintain consistency.

[djc]

I'm inclined to think that the human_ prefix gives us a certain amount of latitude to change things

That would break existing templates; this PR retains existing functionality and only adds new features. Old templates will behave the same as how they did before.

If you have a specification in mind I am more than happy to implement it.

How would it break existing templates? Or do you mean it would change the output for existing templates? I'm saying that old templates behaving exactly the same as before isn't a very important goal to me.

The assertion style with outcome, input seems unidiomatic

I agree; I just copied the design of what was already in the repo for both tests and code to maintain consistency.

Okay, let's leave it for now.

[ReagentX]

I didn't want to change any existing behavior unless that was specifically requested.

[djc]

IMO this behavior should become the default for human_ values, with an easy way to opt out.

[ReagentX]

I can just flip the logic for when the alternative formatter is enabled, are there any other design details I should include? It sounds like human_ as a prefix doesn't really describe any of the behavior, it's more like formatted_ and shorthand_.

[djc]

I can just flip the logic for when the alternative formatter is enabled, are there any other design details I should include? It sounds like human_ as a prefix doesn't really describe any of the behavior, it's more like formatted_ and shorthand_.

Flipping the behavior sounds good to me. I don't see much of an issue with the human_ prefix, and we're kind of stuck with it now, anyway.

[chris-laplante]

I can just flip the logic for when the alternative formatter is enabled, are there any other design details I should include? It sounds like human_ as a prefix doesn't really describe any of the behavior, it's more like formatted_ and shorthand_.

Flipping the behavior sounds good to me. I don't see much of an issue with the human_ prefix, and we're kind of stuck with it now, anyway.

(sorry for the radio silence - I was on vacation)

I am also fine with flipping the behavior.

[ReagentX]

I pushed the relevant changes.

[djc]

Please rebase on top of main. Here's what I'm looking for:

• No new template keys
• Default formats for (some) human_ keys now always show 3 significant digits (if 100+)
• Potentially custom ways to format human_ keys, which are explained in the documentation

I don't see new formatting options ($ or #.??) explained in the docs.

[ReagentX]

I added logic to maintain 3 digits by default; I am not sure what the rest of your comment is referring to.

[djc]

I added logic to maintain 3 digits by default; I am not sure what the rest of your comment is referring to.

There is quite a bit of documentation at https://github.com/console-rs/indicatif/blob/main/src/lib.rs#L136 about the template syntax. Please update this to describe the changes you've made.

In my mind, this PR should not be adding new template keys (like the _formatted ones). My intention is that we make human_* and related keys' values better/more human-friendly by default, with an opt-in fallback to the old behavior. Most of the added tests cover templates that include the # sign which I understood to activate the opt-in fallback, meaning that there aren't many tests that cover the changed default behavior?

[djc]

I don't think we need both this and HUMAN_COUNT_TRESHOLDS, presumably we can drive the float value from the integer one?

[ReagentX]

The types are different, so they have to be defined separately unless I want to cast them every time the formatter runs. I could do something like either of these:

macro_rules! define_thresholds {
    ($(($num:expr, $suffix:expr)),*) => {
        const HUMAN_COUNT_THRESHOLDS: [(u64, &str); 5] = [
            $(($num, $suffix),)*
        ];

        const HUMAN_FLOAT_COUNT_THRESHOLDS: [(f64, &str); 5] = [
            $(($num as f64, $suffix),)*
        ];
    };
}

// Explicit type for 1T because Rust infers numeric literals to be i32 by default
define_thresholds!(
    (1_000_000_000_000i64, "T"),
    (1_000_000_000, "B"),
    (1_000_000, "M"),
    (1_000, "k"),
    (0, "")
);

or

const TRILLION: u64 = 1_000_000_000_000;
const BILLION: u64 = 1_000_000_000;
const MILLION: u64 = 1_000_000;
const THOUSAND: u64 = 1_000;
const ZERO: u64 = 0;

const HUMAN_COUNT_THRESHOLDS: [(u64, &str); 5] = [
    (TRILLION, "T"),
    (BILLION, "B"),
    (MILLION, "M"),
    (THOUSAND, "k"),
    (ZERO, ""),
];

const HUMAN_FLOAT_COUNT_THRESHOLDS: [(f64, &str); 5] = [
    (TRILLION as f64, "T"),
    (BILLION as f64, "B"),
    (MILLION as f64, "M"),
    (THOUSAND as f64, "k"),
    (ZERO as f64, ""),
];

but they both feel a lot less maintainable than just having two arrays.

[ReagentX]

I updated the documentation in my PR: https://github.com/console-rs/indicatif/pull/697/files#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759

I didn't add the #, that is a Rust stdlib feature that indicatif is already using:

indicatif/src/style.rs

Lines 326 to 331 in 458e4dc

	"elapsed_precise" => buf
	.write_fmt(format_args!("{}", FormattedDuration(state.elapsed())))
	.unwrap(),
	"elapsed" => buf
	.write_fmt(format_args!("{:#}", HumanDuration(state.elapsed())))
	.unwrap(),

The indicatif template string is not the same as fmt; what happens in indicatif is it builds a format string based on the provided TemplatePart. In cases where we want to enable the alternate formatter, we have to add a new key so that format_state() knows what format string to build.

The tests call format!() on the structs that implement fmt directly, but at runtime we have to use a TemplatePart to tell indicatif what to pass to format_args!().

[ReagentX]

Do you need anything else from me?

[djc]

I don't remember. This is still on my list to review but it's not very high priority unless it can be funded, sorry.