build(deps): bump unicode-width from =0.1.12 to 0.2.2#15231
build(deps): bump unicode-width from =0.1.12 to 0.2.2#15231RoloEdits wants to merge 1 commit intohelix-editor:masterfrom
unicode-width from =0.1.12 to 0.2.2#15231Conversation
aecc55f to
ff6aa3c
Compare
|
ref: ratatui/ratatui#2188 |
|
So the rabbit-hole was even deeper. Came across some odd behavior while working on #12369, and integrating these changes together, I found that Nerd Fonts seemed to only be reporting a width of 1, even if it was a wide glyph. Turns out (at least as far as I understand), This is a problem, as most Nerd Fonts we would use are wide (2 width). So some extra handling was added to make sure this is covered, but its not full-proof; not all Nerf Fonts are wide. The current design is to assume that the vast majority of Nerd Fonts are wide, and then we just exclude narrow cases as they are found. I'm not sure how robust this will end up being, but the more I learn about unicode + terminals the more I see how crazy there aren't more issues. I tried to document the reasoning, but let me know if more is needed or clarified. Happy to take any direction from anyone who knows better. |
|
Hmm, these changes are leading to artifacting:
I saw this before, and I think its related to the diff implementation in You can find the cheat sheet in text form here: https://github.com/8bitmcu/NerdFont-Cheat-Sheet |
|
Another issue is if users have a monospace font version. This would force the icons to fit within 1 cell at all times. So we cannot just go off of a visual check of the nerd fonts to see which should be which. I think it might be fine to just let the width be 1, as the current handling should be fine in most situations. It can also be accounted for better if needed under the assumption they are all 1 width. The other issue I found was that even things like |
cd75a61 to
dedb590
Compare
|
Some more context: ryanoasis/nerd-fonts#1103 |

This is (hopefully) a corrected version of #14643, which was reverted in #15150.
Different this time is that we upgrade to
0.2.2, which includes the newline width = 1 change. In0.1.14, despite being free from this change, there was actually a larger change that made all control characters have a width of 1. The issue that lead to the reverting of the previous PR was due to this. Its seems that\twas the culprit here. As this needed special handling anyways, I decided to handle both this and the newline change, as the "fix" is the same for both.I started by consolidating the raw
unicode-widthcalls and wrapped it in a function that would hold our fixes. With the abstracting in place, I then propagated the newwidthfunction where needed.I say "fix" because its not really that the new widths are wrong, terminals just expect them to be 0, and that there could be other areas where this is fixed, like in the rendering, handling the control characters there instead. But these changes piggyback off of what existed in the most straightforward way, and so was chosen.
The main change is to count the new 1 width control characters and then subtract the width that
unicode-widthgives us. There is some special handling that\r\nhas inunicode-width, where the sequence itself has a width of 1, so this required some extra site documentation explaining how just\ncan work here.There is a
char::is_control, but I opted for the manualmatches!as this could be faster if the full set of options is realistically only a handle full of known characters. Time will tell if this is enough,Beyond the implementation, it would be really nice if this could be tested out for a bit before merging. This is a tricky area that could have some edge cases. For now I have confirmed that the previous
goplsissue is resolved and that it seems to be fine as far as rendering the rest of the UI.Closes: #14642