-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deno.consoleSize
+ full-width text/ansi escape codes is bug-prone with no official way of mitigating
#18477
Comments
I don't think this needs to be done in the runtime. Couldn't this be done in some userland javascript module? |
It's true it's not a clear-cut case, but here are the best arguments I can think of for why it should be in the runtime:
If this isn't added to the runtime, adding a note to the JSDoc for Edit: As another possibility, adding a TS port of the |
It's exposing the underlying size that the console reports via the winapi on windows and libc elsewhere. Lines 245 to 287 in e0429e2
This is standard API that's found in many other languages.
Does this mean that someone would have the choice to select which implementation they would want to use? Is there a reason someone would use one implementation over another?
I think this would be better to live in deno_std or somewhere else. The console window size can be retreived via the
I opened #18508 |
Not that I can think of — it's generally a case of one implementation being strictly better or worse than other implementations (modulo font differences etc), which is why IMO having one "canonical" implementation makes sense.
I made a port of the |
Even in "monospace" text, many characters take up double the space, and many take up no space at all. Comparing
Deno.consoleSize().columns
to string length, or even using more accurate Unicode-aware forms of measuring, such as[...str].length
or[...new Intl.Segmenter('en', { granularity: 'grapheme' }).segment(str)].length
, is highly bug-prone:In order for
Deno.consoleSize
to be reliable* for general-purpose use, some method of getting the width of a string also needs to be exposed. Possible candidates:ext/console/02_console.js > getStringWidth
ext/node/polyfills/internal/util/inspect.mjs > getStringWidth
, which is a thin wrapper around 1rustyline
)Of these,
unicode-width
looks to be by far the most comprehensive.* Even once this is implemented there will be edge cases, such as differences in fonts, codepoints that don't have a fixed width, etc. but it should at least be reliable for the majority of real-world use cases.
The text was updated successfully, but these errors were encountered: