Skip to content

[node-core-library] Fix Text.truncateWithEllipsis() exceeding maximumLength - #6106

Open
Alexandre Kohler (kwy404) wants to merge 1 commit into
microsoft:mainfrom
kwy404:kwy404/fix-truncate-with-ellipsis
Open

Alexandre Kohler (kwy404) wants to merge 1 commit into
microsoft:mainfrom
kwy404:kwy404/fix-truncate-with-ellipsis

Conversation

@kwy404

Copy link
Copy Markdown

Summary

Text.truncateWithEllipsis() checks s.length <= 3 where it should check maximumLength < 3 before deciding to skip the ellipsis. So for any input longer than 3 characters with maximumLength of 0, 1 or 2, it returns '...', which is longer than maximumLength. For example Text.truncateWithEllipsis('12345', 2) and Text.truncateWithEllipsis('12345', 0) both return '...'.

This PR changes the check to maximumLength < 3.

Details

With the old check, those inputs fall through to s.substring(0, maximumLength - 3) + '...'. The negative end index is treated as 0, so the result is just '...'.

Results do not change for any other input: an input of 3 characters or fewer only reaches this check when maximumLength is already below 3, and maximumLength of 3 or more still gets the ellipsis as before.

rushx can hit this when it truncates script bodies to the console width on a narrow terminal.

How it was tested

Added a unit test in src/test/Text.test.ts that truncates '12345' to lengths 0 and 2. Without the fix it fails (Expected: "", Received: "..."). With the fix all 16 Text tests pass. Also ran the node-core-library build with lint and API Extractor, rush prettier and rush change --verify.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs triage

Development

Successfully merging this pull request may close these issues.

1 participant