[p5.js 2.0+ Bug Report]: nfc() truncates instead of rounding and drops the documented trailing zero padding
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 88/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- javascript
- Domain
- frontend
Research direction
Start in src/utilities/utility_functions.js at doNfc, then run test/unit/utilities/utility_functions.js with the reported cases. Confirm that nfc() matches the documented rounding and trailing-zero behavior, preserves string decimal input, handles carry regrouping and negatives, and leaves the existing tests passing.
Written by the indexing model from the issue text.
Description
Most appropriate sub-area of p5.js?
Utilities
p5.js version
2.x main (4b096e2)
Actual vs expected behavior
nfc()'s own reference examples document rounding and zero padding, but doNfc in src/utilities/utility_functions.js compares the requested decimal count against a substring that still contains the leading ., so it truncates where the docs round and skips the padding branch by one:
nfc(12345.67, 1); // actual '12,345.6' documented '12,345.7'
nfc(12345.67, 3); // actual '12,345.67' documented '12,345.670'
nf() already rounds via toFixed, so the two formatters also disagree with each other on the same input. A rounding carry also needs comma re-grouping, e.g. nfc(999.96, 1) should give '1,000.0'.
Steps to reproduce
Outputs above are from executed runs against current main via the unit test harness (npx vitest run test/unit/utilities/utility_functions.js with added cases).
Note
I have a fix ready (route through toFixed like nf, keep the comma grouping, preserve the existing nfc(32000, '3') === '32,000.000' behavior) with unit tests covering rounding, padding, the carry re-group and negatives. Verified the four new tests fail on current main and pass with the change, 23 existing tests unaffected. Filing for approval first per the contributing guide; will open the PR once approved.
- Dominant language
- JavaScript
- Stars
- 24k
- Forks
- 3.8k
- Avg merge
- 3d 9h
- Merged PRs (30d)
- 26
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from processing/p5.js
-
Area:Math p5.js 2.0+
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
processing/p5.js#9197 · 3 comments · 1 assignee ·
-
Area:Core Area:DOM
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
processing/p5.js#9189 ·
-
[Bug]: p5.Image.prototype.copy and blend do not scale destination coordinates for high pixel density Open
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
processing/p5.js#9169 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
processing/p5.js#9129 ·
-
[p5.js 2.0+ Bug Report]: splitTokens() returns [] for a caret delimiter and throws for a backslash Open
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
processing/p5.js#9126 · 1 comment ·
All issues in processing/p5.js
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
HarperFast/skills#96 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Automattic/studio#4908 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
sugarlabs/musicblocks#8847 ·
-
client-controller-update ta-bot-triage team-money-movement
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
MetaMask/metamask-mobile#36594 ·