fix: guard against numeric noteArg in getNote()#7452
Conversation
|
🧪 Jest Test Results ❌ Some Jest tests failed. Please check the logs and fix the issues before merging. Coverage: Statements: 46.79% | Branches: 38.27% | Functions: 51.22% | Lines: 47.22% Note: These failures may be introduced by this PR or may already exist in the master branch. Failed Tests: |
|
@Gungunverma1227, can you add the PR category correctly, other than that your changes looks good to me |
github-actions
Bot
added
bug fix
|
@021nirav-blip Thank you! I have added the PR category correctly. Could you please approve the review? |
021nirav-blip
left a comment
021nirav-blip
left a comment
There was a problem hiding this comment.
@walterbender lgtm
|
Please address the linting errors |
|
@Gungunverma1227 |
Gungunverma1227
force-pushed
the
fix-getnote-typeguard-clean
branch
from
0b5b0cc to
d47b4eb
Compare
|
@walterbender @021nirav-blip I have rebased to the latest master branch and fixed the linting errors. Please review! |
|
🧪 Jest Test Results ✅ All Jest tests passed! This PR is ready to merge. Coverage: Statements: 48.3% | Branches: 39.91% | Functions: 53.01% | Lines: 48.7% |
| ) { | ||
| if (typeof noteArg === "number") { | ||
| noteArg = noteArg.toString(); | ||
| } |
There was a problem hiding this comment.
This indentation doesn't look correct
There was a problem hiding this comment.
@walterbender I have fixed the indentation. Please review!
|
🧪 Jest Test Results ✅ All Jest tests passed! This PR is ready to merge. Coverage: Statements: 48.3% | Branches: 39.91% | Functions: 53.01% | Lines: 48.7% |
|
@walterbender @021nirav-blip please review , i have updated the code. |
3 participants
Problem
When a custom temperament is active, pitch blocks pass integer step indices to getNote(). The function internally calls .substr() assuming a string argument, causing a crash:
TypeError: noteArg.substr is not a function
Fix
Added a type guard at the entry of getNote() in musicutils.js to convert numeric noteArg to string before processing.
Category