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

Clean up and type number formatting APIs #20334

Merged
merged 7 commits into from
Apr 19, 2024
Merged

Clean up and type number formatting APIs #20334

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7b450ce
base1: drop cockpit.get_byte_units() API
allisonkarlitskaya Apr 18, 2024
0e06f2c
storaged: simplify utils.get_byte_units()
allisonkarlitskaya Apr 18, 2024
7756e36
test: add f`` tagged template string helper
allisonkarlitskaya Apr 18, 2024
1bade22
test: use f-strings in asserts in test-format
allisonkarlitskaya Apr 18, 2024
d254625
storage: add a testcase for format_fsys_usage()
allisonkarlitskaya Apr 18, 2024
85a6912
cockpit.js: add new number formatting API style
allisonkarlitskaya Apr 18, 2024
3ca2dcb
test: port test-format to typescript
allisonkarlitskaya Apr 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 14 additions & 45 deletions doc/guide/cockpit-util.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,75 +44,44 @@ string = cockpit.format_number(number, [precision])
<refsection id="cockpit-format-bytes">
<title>cockpit.format_bytes()</title>
<programlisting>
string = cockpit.format_bytes(number, [factor])
array = cockpit.format_bytes(number, [factor, options])
string = cockpit.format_bytes(number, [options])
</programlisting>
<para>Formats <code>number</code> into a displayable <code>string</code> with a suffix, such as
<emphasis>KB</emphasis> or <emphasis>MB</emphasis>. Returns an <code>array</code> of the
formatted number and the suffix if <code>options.separate</code> is set to <code>true</code>.</para>
<emphasis>KB</emphasis> or <emphasis>MB</emphasis>.</para>

<para>If specifying 1000 or 1024 is specified as a <code>factor</code> then an appropriate suffix
will be chosen. By default the <code>factor</code> is 1000. You can pass a string suffix as a
<code>factor</code> in which case the resulting number will be formatted with the same suffix.</para>

<para>If the <code>number</code> is less than the <code>factor</code> or an unknown factor
was passed in, then the formatted number is returned without a suffix. If <code>options.separate</code>
is true, returns an array of <code>[formatted_number, suffix]</code> or
<code>[formatted_number]</code> if returned without a suffix.</para>
<para>By default, SI units are used. IEC units (1024-based) can be requested by including
<code>base2: true</code> in <code>options</code>.</para>

<para>By default, non-integer numbers will be formatted with 3 digits of precision. This can be changed
with <code>options.precision</code>.</para>

<para>If <code>number</code> is <code>null</code> or <code>undefined</code> an empty string or
an array without a suffix will be returned.</para>
<para>If <code>number</code> is <code>null</code> or <code>undefined</code> an empty string will be
returned.</para>
</refsection>

<refsection id="cockpit-format-bytes-per-sec">
<title>cockpit.format_bytes_per_sec()</title>
<programlisting>
string = cockpit.format_bytes_per_sec(number, [factor])
array = cockpit.format_bytes_per_sec(number, [factor, options])
string = cockpit.format_bytes_per_sec(number, [options])
</programlisting>
<para>Format <code>number</code> of bytes into a displayable speed <code>string</code>.</para>

<para>If specifying 1000 or 1024 is specified as a <code>factor</code> then an appropriate suffix
will be chosen. By default the <code>factor</code> is 1000. You can pass a string suffix as a
<code>factor</code> in which case the resulting number will be formatted with the same suffix.</para>

<para>If the <code>number</code> is less than the <code>factor</code> or an unknown factor
was passed in, then the formatted number is returned without a suffix. If <code>options.separate</code>
is true, returns an array of <code>[formatted_number, suffix]</code> or
<code>[formatted_number]</code> if returned without a suffix.</para>

<para>By default, non-integer numbers will be formatted with 3 digits of precision. This can be changed
with <code>options.precision</code>.</para>

<para>If <code>number</code> is <code>null</code> or <code>undefined</code> an empty string or array
will be returned.</para>
<para>This function is mostly equivalent to <code>cockpit.format_bytes()</code> but the returned
value contains a unit like <emphasis>KB/s</emphasis> or <emphasis>MB/s</emphasis>.</para>
</refsection>

<refsection id="cockpit-format-bits-per-sec">
<title>cockpit.format_bits_per_sec()</title>
<programlisting>
string = cockpit.format_bits_per_sec(number, [factor])
array = cockpit.format_bytes_per_sec(number, [factor, options])
string = cockpit.format_bits_per_sec(number, [options])
</programlisting>
<para>Format <code>number</code> of bits into a displayable speed <code>string</code>.</para>

<para>If specifying 1000 or 1024 is specified as a <code>factor</code> then an appropriate suffix
will be chosen. By default the <code>factor</code> is 1000. You can pass a string suffix as a
<code>factor</code> in which case the resulting number will be formatted with the same suffix.</para>

<para>If the <code>number</code> is less than the <code>factor</code> or an unknown factor
was passed in, then the formatted number is returned without a suffix. If <code>options.separate</code>
is true, returns an array of <code>[formatted_number, suffix]</code> or
<code>[formatted_number]</code> if returned without a suffix.</para>

<para>By default, non-integer numbers will be formatted with 3 digits of precision. This can be changed
with <code>options.precision</code>.</para>
<para>This function is mostly equivalent to <code>cockpit.format_bytes()</code> but the returned
value contains a unit like <emphasis>kbps</emphasis> or <emphasis>Mbps</emphasis>.</para>

<para>If <code>number</code> is <code>null</code> or <code>undefined</code> an empty string or array
will be returned.</para>
<para>This function does not support IEC units. <code>base2</code> may not be passed as part of
<code>options</code>.</para>
</refsection>

<refsection id="cockpit-info">
Expand Down
31 changes: 26 additions & 5 deletions pkg/base1/test-format.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,18 @@ QUnit.test("format_bytes", function (assert) {
[null, "KB", ""],
];

assert.expect(checks.length * 2 + 2);
for (let i = 0; i < checks.length; i++) {
if (typeof checks[i][1] === 'string') {
// these tests are for backwards compatibility only
continue;
}

const base2 = checks[i][1] == 1024;
assert.strictEqual(cockpit.format_bytes(checks[i][0], { base2 }), checks[i][2],
f`format_bytes(${checks[i][0]}, ${{ base2 }})`);
}

// old API style (deprecated)
for (let i = 0; i < checks.length; i++) {
assert.strictEqual(cockpit.format_bytes(checks[i][0], checks[i][1]), checks[i][2],
f`format_bytes(${checks[i][0]}, ${checks[i][1]})`
Expand Down Expand Up @@ -146,16 +157,26 @@ QUnit.test("format_bytes_per_sec", function (assert) {
[25555678, "kB/s", { precision: 2 }, "25556 kB/s"],
];

assert.expect(checks.length + 2);
for (let i = 0; i < checks.length; i++) {
if (typeof checks[i][1] === 'string') {
// these tests are for backwards compatibility only
continue;
}

const base2 = checks[i][1] == 1024;
assert.strictEqual(cockpit.format_bytes_per_sec(checks[i][0], { base2, ...checks[i][2] }), checks[i][3],
f`format_bytes_per_sec(${checks[i][0]}, ${{ base2, ...checks[i][2] }})`);
}

// old API style (deprecated)
for (let i = 0; i < checks.length; i++) {
assert.strictEqual(cockpit.format_bytes_per_sec(checks[i][0], checks[i][1], checks[i][2]), checks[i][3],
f`format_bytes_per_sec(${checks[i][0]}, ${checks[i][1]}, ${checks[i][2]})`);
}

// separate unit
// separate unit (very deprecated)
assert.deepEqual(cockpit.format_bytes_per_sec(2555, 1024, { separate: true }),
["2.50", "KiB/s"]);
// backwards compatible API for separate flag
// backwards compatible API for separate flag (oh so very deprecated)
assert.deepEqual(cockpit.format_bytes_per_sec(2555, 1024, true),
["2.50", "KiB/s"]);
});
Expand Down
29 changes: 17 additions & 12 deletions pkg/lib/cockpit.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,6 @@ declare module 'cockpit' {

/* === String helpers ======================== */

type FormatOptions = {
precision?: number;
separate?: boolean;
};

function message(problem: string | JsonObject): string;

function gettext(message: string): string;
Expand All @@ -206,11 +201,21 @@ declare module 'cockpit' {
function ngettext(context: string, message1: string, messageN: string, n: number): string;

function format(format_string: string, ...args: unknown[]): string;
function format_number(n: number, precision?: number): string
function format_bytes(n: number, factor?: 1000 | 1024, options?: FormatOptions & { separate?: false }): string;
function format_bytes(n: number, factor: 1000 | 1024, options: FormatOptions & { separate: true }): string[];
function format_bytes_per_sec(n: number, factor?: 1000 | 1024, options?: FormatOptions & { separate?: false }): string;
function format_bytes_per_sec(n: number, factor: 1000 | 1024, options: FormatOptions & { separate: true }): string[];
function format_bits_per_sec(n: number, factor?: 1000 | 1024, options?: FormatOptions & { separate?: false }): string;
function format_bits_per_sec(n: number, factor: 1000 | 1024, options: FormatOptions & { separate: true }): string[];

/* === Number formatting ===================== */

type FormatOptions = {
precision?: number;
base2?: boolean;
};
type MaybeNumber = number | null | undefined;

function format_number(n: MaybeNumber, precision?: number): string
function format_bytes(n: MaybeNumber, options?: FormatOptions): string;
function format_bytes_per_sec(n: MaybeNumber, options?: FormatOptions): string;
function format_bits_per_sec(n: MaybeNumber, options?: FormatOptions & { base2?: false }): string;
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm, so the argument here is. if I wanted to provide an options argument, I'd have to figure out the default factor which would be error prone?

Copy link
Member Author

Choose a reason for hiding this comment

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

The new API basically has two possibilities: SI units (default, old factor=1000) and IEC units (old factor=1024).

Copy link
Member

Choose a reason for hiding this comment

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

@jelly With the "old" API you could specify undefined for the factor, in which case it would be the default "1000". But indeed this is error prone -- in fact, my first version here gave null, which made it fall over completely. That's why I'd like to get rid of that.

Copy link
Member

Choose a reason for hiding this comment

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

Just to make sure I understand this: The base2 restriction is here to make base2: true fall through the "deprecated" declaration, right? Not because enabling base2 would actually change the output type.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm pretty sure that enabling base2 will break this (ie: return just the number) in exactly the same way as passing 1024 as factor would. That particular function lacks base2 units (ie: no Mibps or so)...

Copy link
Member

Choose a reason for hiding this comment

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

Right, and I don't think we ever supported that actually.


/** @deprecated */ function format_bytes(n: MaybeNumber, factor: unknown, options?: object | boolean): string | string[];
/** @deprecated */ function format_bytes_per_sec(n: MaybeNumber, factor: unknown, options?: object | boolean): string | string[];
/** @deprecated */ function format_bits_per_sec(n: MaybeNumber, factor: unknown, options?: object | boolean): string | string[];
}
44 changes: 26 additions & 18 deletions pkg/lib/cockpit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1483,10 +1483,24 @@ function factory() {
});
};

function format_units(number, suffixes, factor, options) {
// backwards compat: "options" argument position used to be a boolean flag "separate"
if (!is_object(options))
options = { separate: options };
let deprecated_format_warned = false;
function format_units(suffixes, number, second_arg, third_arg) {
let options = second_arg;
let factor = options?.base2 ? 1024 : 1000;

// compat API: we used to accept 'factor' as a separate second arg
if (third_arg || (second_arg && !is_object(second_arg))) {
martinpitt marked this conversation as resolved.
Show resolved Hide resolved
if (!deprecated_format_warned) {
console.warn(`cockpit.format_{bytes,bits}[_per_sec](..., ${second_arg}, ${third_arg}) is deprecated.`);
Copy link
Member

Choose a reason for hiding this comment

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

When do we remove it, should we include that? Like in 10 releases this API is gone?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm OK with next release already...

deprecated_format_warned = true;
}

factor = second_arg || 1000;
options = third_arg;
// double backwards compat: "options" argument position used to be a boolean flag "separate"
if (!is_object(options))
options = { separate: options };
}

let suffix = null;

Expand Down Expand Up @@ -1525,15 +1539,15 @@ function factory() {
}
}

const string_representation = cockpit.format_number(number, options.precision);
const string_representation = cockpit.format_number(number, options?.precision);
let ret;

if (string_representation && suffix)
ret = [string_representation, suffix];
else
ret = [string_representation];

if (!options.separate)
if (!options?.separate)
ret = ret.join(" ");

return ret;
Expand All @@ -1544,31 +1558,25 @@ function factory() {
1024: [null, "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB"]
};

cockpit.format_bytes = function format_bytes(number, factor, options) {
if (factor === undefined)
factor = 1000;
return format_units(number, byte_suffixes, factor, options);
cockpit.format_bytes = function format_bytes(number, ...args) {
return format_units(byte_suffixes, number, ...args);
};

const byte_sec_suffixes = {
1000: ["B/s", "kB/s", "MB/s", "GB/s", "TB/s", "PB/s", "EB/s", "ZB/s"],
1024: ["B/s", "KiB/s", "MiB/s", "GiB/s", "TiB/s", "PiB/s", "EiB/s", "ZiB/s"]
};

cockpit.format_bytes_per_sec = function format_bytes_per_sec(number, factor, options) {
if (factor === undefined)
factor = 1000;
return format_units(number, byte_sec_suffixes, factor, options);
cockpit.format_bytes_per_sec = function format_bytes_per_sec(number, ...args) {
return format_units(byte_sec_suffixes, number, ...args);
};

const bit_suffixes = {
1000: ["bps", "Kbps", "Mbps", "Gbps", "Tbps", "Pbps", "Ebps", "Zbps"]
};

cockpit.format_bits_per_sec = function format_bits_per_sec(number, factor, options) {
if (factor === undefined)
factor = 1000;
return format_units(number, bit_suffixes, factor, options);
cockpit.format_bits_per_sec = function format_bits_per_sec(number, ...args) {
return format_units(bit_suffixes, number, ...args);
};

/* ---------------------------------------------------------------------
Expand Down