Skip to content

Implement Number.toFixed#1413

Merged
Perryvw merged 1 commit intoTypeScriptToLua:masterfrom
Dimava:master
Mar 1, 2023
Merged

Implement Number.toFixed#1413
Perryvw merged 1 commit intoTypeScriptToLua:masterfrom
Dimava:master

Conversation

@Dimava
Copy link
Contributor

@Dimava Dimava commented Feb 28, 2023

Needs some throughts on if Number.toFixed has to follow JS rounding behaviour vs having simple implementation and possibly being more fast
Otherwise seems to work fine

Copy link
Member

@Perryvw Perryvw left a comment

Choose a reason for hiding this comment

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

I'm not sure what the 'simple' behavior vs the rounding behavior is, but this looks alright to me

return this.toString();
}
const f = Math.floor(fractionDigits ?? 0);
if (f < 0 || f > 100) {
Copy link
Contributor Author

@Dimava Dimava Feb 28, 2023

Choose a reason for hiding this comment

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

Actually it may make sence to limit the fraction on ~98 to avoid "invalid format (width or precision too long)"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or limit values to <1e19

Copy link
Member

Choose a reason for hiding this comment

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

I don't really care, even leaving out the check would be an option. Do what you think is best.

const f = Math.floor(fractionDigits ?? 0);
// reduced to 99 as string.format only supports 2-digit numbers
if (f < 0 || f > 99) {
throw "toFixed() digits argument must be between 0 and 99";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should errors be exaclty like in JS (between 0 and 100) or this is fine?

@Dimava
Copy link
Contributor Author

Dimava commented Feb 28, 2023

I'm not sure what the 'simple' behavior vs the rounding behavior is, but this looks alright to me

Nevermind, I've missed that __TS__NumberToString uses string.format(simple) on radix === 10 instead of manual rounding

@Perryvw Perryvw merged commit 2dfca0b into TypeScriptToLua:master Mar 1, 2023
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