cabal-cli icon indicating copy to clipboard operation
cabal-cli copied to clipboard

Unicode render fixes

Open hackergrrl opened this issue 1 year ago • 3 comments

Uses the module wcwidth throughout cabal-cli's render logic.

This modifies several key drawing routines to use the wcwidth module, in order to compute the visual width of Unicode code points on a fixed-width display (i.e. a terminal). A great deal of render bugs apparent on this client are fixed by this.

Depends on:

  • [x] https://github.com/mafintosh/ansi-diff/pull/8

hackergrrl avatar Aug 29 '24 01:08 hackergrrl

Also! For reasons I have not investigated, rendering seems to be significantly faster! 😎

hackergrrl avatar Aug 29 '24 19:08 hackergrrl

nice! will give the patch a spin this weekend :] since you requested my review, here's my feedback & thoughts at a glance:

i was going to ask why markdown-shim.js was deleted since that seemed unrelated to the patch but then i saw:

For now, until it can be fixed to not break terminal rendering.

re

Captalize the "D" in "day changed to" text.

this change does seem unrelated to this patch, so i'll just share that my own personal preference is to keep the lowercase 'd' X)

cblgh avatar Aug 30 '24 12:08 cblgh

i was going to ask why markdown-shim.js was deleted since that seemed unrelated to the patch

Yeah! I found cases in the Starlight Union channel where it breaks rendering on my terminal emulator. supports-hyperlinks claims my terminal supports ANSI links (didn't know that was a thing!), but it in fact does not seem to. I didn't dig deep into the markdown shim piece at all just to be clear, it just seemed like a low-impact thing to remove for now, since the only markdown thing it did was replacing [foo](https://bar.com) patterns.

hackergrrl avatar Aug 30 '24 16:08 hackergrrl

Thanks @cblgh, I appreciate you taking the time!

hackergrrl avatar Sep 01 '24 04:09 hackergrrl

Thank you for looking this over, @cblgh.

hackergrrl avatar Sep 01 '24 23:09 hackergrrl

15.0.1

hackergrrl avatar Sep 01 '24 23:09 hackergrrl

15.0.2. I had package-lock.json files disabled on my npm setup, and so there's another patch release that includes that file being updated, which I believe is what npm install uses.

hackergrrl avatar Sep 02 '24 15:09 hackergrrl