Skip to content

Format decimal#50

Closed
simbig wants to merge 2 commits intomasterfrom
format-decimal
Closed

Format decimal#50
simbig wants to merge 2 commits intomasterfrom
format-decimal

Conversation

@simbig
Copy link
Contributor

@simbig simbig commented Sep 11, 2025

No description provided.

@simbig simbig requested a review from Copilot September 11, 2025 13:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new formatDecimal function to handle decimal number formatting with configurable precision and trailing zero trimming options.

  • Adds formatDecimal function with options for min/max decimals and trailing zero control
  • Includes comprehensive test coverage for various formatting scenarios
  • Updates imports to include the new function

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/number.ts Implements the new formatDecimal function with rounding and formatting logic
src/number.test.ts Adds test cases covering different decimal formatting scenarios

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +39 to +42
let str = rounded.toFixed(Math.max(minDecimals, maxDecimals));

if (trimTrailingZeros) {
str = str.replace(/(\.[0-9]*?)0+$/, '$1').replace(/\.$/, '');
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

The logic for toFixed should use maxDecimals to ensure proper rounding, then handle minDecimals separately. Currently, when minDecimals > maxDecimals, this will display more decimals than the rounded value supports, potentially showing incorrect precision.

Suggested change
let str = rounded.toFixed(Math.max(minDecimals, maxDecimals));
if (trimTrailingZeros) {
str = str.replace(/(\.[0-9]*?)0+$/, '$1').replace(/\.$/, '');
// Always round to maxDecimals
let str = rounded.toFixed(maxDecimals);
if (trimTrailingZeros) {
// Remove trailing zeros, but ensure at least minDecimals remain
const [intPart, decPart = ''] = str.split('.');
let trimmedDec = decPart.replace(/0+$/, '');
// Pad with zeros if needed to reach minDecimals, but never more than maxDecimals
if (trimmedDec.length < minDecimals) {
trimmedDec = trimmedDec.padEnd(minDecimals, '0');
}
// If there are decimals, join; else, just intPart
str = trimmedDec.length > 0 ? `${intPart}.${trimmedDec}` : intPart;
} else if (minDecimals < maxDecimals) {
// If not trimming, pad with zeros to minDecimals if needed
const [intPart, decPart = ''] = str.split('.');
let paddedDec = decPart;
if (paddedDec.length < minDecimals) {
paddedDec = paddedDec.padEnd(minDecimals, '0');
}
str = paddedDec.length > 0 ? `${intPart}.${paddedDec}` : intPart;

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +37
const factor = 10 ** maxDecimals;
const rounded = Math.round(value * factor) / factor;
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

This rounding logic duplicates the existing round function. Consider using round(value, maxDecimals) instead to avoid code duplication and maintain consistency.

Suggested change
const factor = 10 ** maxDecimals;
const rounded = Math.round(value * factor) / factor;
const rounded = round(value, maxDecimals);

Copilot uses AI. Check for mistakes.
@simbig simbig closed this Sep 11, 2025
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