edouardparis / lntop

:zap: LN terminal dashboard :bar_chart:
MIT License
183 stars 24 forks source link

Some node aliases are trimmed #89

Closed rkfg closed 2 years ago

rkfg commented 2 years ago

2022-09-16_19-50-57 Not sure what causes it, needs investigation. should be 電母 and DiamondHands💎 should be DiamondHands💎🙌. Aliases are currently shortened to 24 bytes which is already incorrect since this can break multibyte characters (happens with JoltFun node, for example). I changed it to 24 runes, a trivial change like alias = string([]rune(alias)[:24]) but it still cuts these characters. Plus, emojis are two characters wide so it's still not as straightforward, can shift other columns I think.

rkfg commented 2 years ago

Looks like a problem with the underlying library and variable rune width. The version used in lntop tries to put every rune in the next cell (character position), however it doesn't work like that for hieroglyphs and emojis that occupy two adjacent cells. So every other wide character is lost. I've updated the library to awesome gocui and now all characters are visible but the columns are shifted: 2022-09-17_14-40-39 For the reference, this is what the current version shows: 2022-09-17_14-41-41 I think the proper solution is updating the library and fixing the offset. Probably by dynamically determining the column width for every alias using the length calculation in cells. Only needs to be done for the alias column because the rest is controlled by us, numbers, predefined statuses etc. This also implies that trimming the long aliases should be done in cells and not in runes (and of course not bytes!) because otherwise 24 emojis would occupy 48 cells while 24 regular latin/cyrillic letters would only need 24 cells.

Those node operators love their fancy aliases too much don't they 😒

rkfg commented 2 years ago

I mostly made it work on awesome-gocui, many issues with the new cursor logic there and buffer clearing. Still it's not good because when the alias column is out of screen (when you scroll to the right) the columns are all shifted again. My solution is to calculate the alias column width in Sprintf as 25 - runewidth.StringWidth(alias) + len([]rune(alias)). But when the actual characters aren't printed on screen this calculated width works against the desired goal. A more correct solution would be using separate views for each column. Then all these hacks wouldn't be needed because the lines positions and limitations would be enforced by the library and not us.

rkfg commented 2 years ago

There's another big mess which is being solved, hopefully. For now cutting \ufe0f is a must, otherwise GUI breaks a little even with separate views. I'm tracking my progress in gocui-upd branch. Currently I replaced gocui with awesome-gocui and changed the way the columns are rendered in the channels view. Not sure if the same approach makes sense in other views. Now every column in the channels view is a separate view, this approach is much more flexible as we don't need to care about very precise alignment and spaces. The alias column with variable width characters no longer affects other columns position and outputs all characters that fit in the column. I think some code can be simplified now as well like the channel disabled status.

rkfg commented 2 years ago

Still need to fix the routing view as it also shows aliases and they shift other columns if there are emojis or wide characters. Otherwise looks pretty good to me.

rkfg commented 2 years ago

Routing views are fixed now, also added back current column highlighting with bold. Rebasing onto master is hard because this branch contains the color library changes and channel age colors. Better wait until that PR is merged so gocui-upd becomes a direct continuation.

hieblmi commented 2 years ago

Just a note that I see an issue with this node "bitdenaro 🦡 🇮🇹".

rkfg commented 2 years ago

Yeah, it depends on whether the number of characters is even or odd and also applies to multiple emojis next to each other. My gocui-upd branch fixes it, it's quite a big patch as it replaces the UI library with a modern fork and also refactors two views (main and routing) with multiple separate views for every column. I'll make a proper PR soon, rebasing it is a bit hard because there were so many merges before but all patches this one depends on are already accepted so I guess simply extracting it as a diff with master and making a single commit should do. I was using this branch for a couple of weeks, all looks fine to me.

rkfg commented 2 years ago

Ready for review!