Repository navigation
fix: account for escape sequences in prefixes - #631
Conversation
This is a known bug we've had for quite some time. Basically, we were checking `prefix.length` to calculate how much space the prefix (e.g. the guide) takes up. This is incorrect since the prefix usually contains escape sequences, so it can be off by several characters. Using fast-string-width instead means we get the actual width. Note that fast-string-width is already a deep dependency, so this just declares it at the root.
🦋 Changeset detectedLatest commit: 3cdb798 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
dreyfus92
left a comment
dreyfus92
left a comment
There was a problem hiding this comment.
hey james, awesome job. one thing i think it's lacking, could be a follow up, is checking limitOptions. the same overcount is still in columnPadding: prefix.length in select.ts, multi-select.ts and group-multi-select.ts. afaik also limitOptions subtracts it from the width, so options will wrap 10 columns early. switching those to stringWidth(prefix) should fix it.
|
let's do it in a follow up (or multiple) since there's probably a few places that can gain from a similar fix |
This is a known bug we've had for quite some time. Basically, we were
checking
prefix.lengthto calculate how much space the prefix (e.g.the guide) takes up.
This is incorrect since the prefix usually contains escape sequences, so
it can be off by several characters. Using fast-string-width instead
means we get the actual width.
Note that fast-string-width is already a deep dependency, so this just
declares it at the root.
As part of this, I also wrapped the footer of select-like prompts FYI.
Type of change
Checklist
pnpm testpasses (or targeted tests for my change)pnpm formathas been runAI-generated code disclosure