exclude powerline char range from contrast demands#3740
exclude powerline char range from contrast demands#3740meganrogge merged 11 commits intoxtermjs:masterfrom
Conversation
|
still a wip - need to test more |
There was a problem hiding this comment.
Nonetheless a few early remarks from my side:
I'd suggest not to use getAsCharData for several reasons:
- outdated and should be removed (
CharDatawas the cell type of the old bufferline impl, those methods only existed for interim transition, but never got cleaned up afterwards because of reasons 😊) - ah right, too convenient in tests 😸 - the method creates an interim array for every tested cell (creating GC pressure for no good reason)
Maybe use cell.getCode() directly instead?
A note on powerline glyphs - most should reside in the Unicode's PUA (private use area), as you correctly test for. But if I remember right there are powerline-like fonts that also use other areas or even single codepoints all over the place (was it nerdfont? idk). Not sure how you want to deal with that, it is a bit like chasing a rabbit, and if you overdo it, you might end up excluding too much ...
Yes good point and I considered covering additional symbols, but I think that arrows are the main problem case as they look pretty bad ATM. |
Co-authored-by: jerch <[email protected]>

fixes #3739
Based the range on this documentation