search

visionmedia / bytes.js

node byte string parser
MIT License
461 stars 57 forks source link

Allow custom precision #28

Closed timkendrick closed 8 years ago

timkendrick commented 9 years ago

This PR allows you to specify the maximum number of decimal places to include in the output of bytes.format().

Examples:

bytes(1024 * 1.12345, {precision: 3});
// output: '1.123kB'

bytes(1024 * 1.12345, {precision: 0});
// output: '1kB'

...note that this is a maximum number of decimal places (rather than a fixed number of decimal places) – i.e. trailing zeroes will be hidden:

bytes(1024, {precision: 10});
// output: '1kB'

Let me know if there's any changes I can make to help get this merged!

timkendrick commented 9 years ago

Update: I've added optional support for a fixed number of decimal places, as follows:

bytes(1024, {precision: 3, fixed: true});
// output: '1.000kB'

...this should allow for use-cases such as #25.

LinusU commented 9 years ago

I was just looking for something like this! My use-case is the fixed one.

timkendrick commented 9 years ago

@dougwilson @tj – any chance somebody could take a look at this? Would be great to get it merged!

dougwilson commented 9 years ago

Sorry @timkendrick , I'll take a look :)

dougwilson commented 8 years ago

Sorry it took so long for me to look at this PR! There is an issue with the name of the argument, precision. It seems too easily confused with the existing API in JavaScript Number.prototype.toPrecision, which does the rounding per definition, which does not match what is being done here.

For example, one would expect bytes(1024 * 1.12345, {precision: 3}); to be 1.12kB, not 1.123kB, since the first matches toPrecision and the dictionary definition of precision.

timkendrick commented 8 years ago

No problem – that makes sense. Any suggestions for an alternate name?

dougwilson commented 8 years ago

I couldn't think of any name at the time I wrote that, but maybe decimalPlaces?

timkendrick commented 8 years ago

Fine by me!

dougwilson commented 8 years ago

I just merged up the decimalPlaces portion of the PR. I didn't want to tie up the PR with two changes at the same time, so one didn't block the other. I'm looking over the fixed portion of the changeset now.

dougwilson commented 8 years ago

Right now still debating between the option being fixed or something like fixedDecimals or decimalsFixed or something, IDK.

dougwilson commented 8 years ago

Hi @timkendrick , would you mind re-issuing the fixed part of this PR as another PR?