fix drawing of wide characters in InfoWindow by Andriamanitra · Pull Request #3919 · micro-editor/micro (original) (raw)

This brings InfoWindow rendering closer to what BufWindow does. Wide characters are no longer rendered as @ (even if they are cut off – if we want to change that then we should do it in BufWindow as well).

closes #2277
closes #3514
closes #3918

I think width = runewidth.RuneWidth(r) is not exactly correct on all terminal emulators in all cases because it's equivalent to the wcwidth approach described in mitchellh's excellent blog post about grapheme clustering in terminals:

What you should not do is assume wcwidth behavior or grapheme clustering behavior. You can see from the table in the previous section that either assumption would be wrong across multiple terminal emulators, even if you only consider the popular or mainstream ones.

However this should still be improvement over what we had before, and I think similar issues already exist elsewhere in micro.

Thank you already for picking it up, since I had no time yet to proceed with #3516. 👍

I've noticed at least two regressions:

When inserting a tab character, the number of inserted spaces is not tabsize but tabsize-1. Even worse, if we insert tabsize-1 letters and then insert a tab character, nothing is inserted. Or actually, an "invisible" tab character (with zero width) is inserted: if after that we press the left arrow key, the cursor doesn't move to the left, it stays where it is.

This certainly has to do with the issue I noticed above.

After typing many characters in the info bar so that they don't fit in the window anymore, something very strange happens: the cursor appears in the buffer window instead of the infobar, as if the buffer window was active.

So... the existing code may be buggy as hell, but do we understand why exactly the existing code does what it does, and where exactly the bug is in it?

After typing many characters in the info bar so that they don't fit in the window anymore, something very strange happens: the cursor appears in the buffer window instead of the infobar, as if the buffer window was active.

I also noticed this but didn't address it since it's unrelated to the rendering of wide characters. I looked into it a bit more just now and I think this is what's happening:

both of the calls to screen.ShowCursor are missing bounds checks so micro tries to place the cursor in a column that is out of bounds
because the new cursor location is invalid the cursor stays in bufwindow instead of being moved to infowindow

edit: It's also worth noting that fake cursors (used for selections & non-primary multicursors) in bufwindow always stay visible when opening infowindow. This is good (probably intended?) because some commands do something with the active selections. The only cursor behaving weirdly is the real one.

I guess the proper fix would be to make infowindow scroll horizontally the same way bufwindow does. I'd prefer for that to be a separate issue and PR though because it's a less trivial change.

Ok, somehow I thought I didn't see the 2nd issue on master, so I thought it was caused by this PR (in some mysterious way). Now I've double-checked that I see it on master as well, so it is unrelated to this PR.

Your updated version of this PR seems correct (at least at first glance), so we can probably merge it. (But the 2nd commit would be better squashed into the 1st one, since it fixes a bug caused by the 1st one).

I guess the proper fix would be to make infowindow scroll horizontally the same way bufwindow does.

Yep, there is a bug #2527 for that.

BTW the 1st issue, I guess, was caused by the fact that, IIRC, RuneWidth() returns 0 for the tab character.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})