Hi NRK,
I think you've gotten it backwards, this patch should *faster* if the
> string is too long, since we're incrementing up to w instead of
> decerementing down it it.
Yes you are right of course, I was mixing up the two approaches.
I have attached the changes I was experimenting with.
I still don't like the amount of effort required for drawing an ellipsis
though so I attached another patch file that doesn't try to draw the
ellipsis for comparison.
-Stein
On Sat, Mar 19, 2022 at 5:18 PM Silvan Jegen <s.jegen_AT_gmail.com> wrote:
> Hi
>
> On Sat, Mar 19, 2022 at 12:59 PM Stein Gunnar Bakkeby <bakkeby_AT_gmail.com>
> wrote:
> >
> > Hi NRK,
> >
> > I was tinkering with this for a few hours earlier this week and there
> are several issues with this area of code and not specifically related to
> this modification of the patch.
> >
> > The handling of adding the ellipsis (...) when the text is too long is
> rather hacky and can cause several issues.
> >
> > The general idea is that:
> > - we work out how many consecutive characters we can print for the
> same font
> > - then we check if the length of the string exceeds the remaining
> drawable space
> > - if it is too long then we check the length of the string for every
> character until we find that it no longer fits
> > - then we replace the last three bytes with periods (.)
> >
> > These are the problematic areas I have found:
> >
> > 1. In the original code the text is allowed to bleed into the right
> padding as long as it fits (I presume this is intentional). If you add one
> more character then the text is too long and it will crop of four
> characters to add the ellipsis. This has to do with that the for loop
> reduces len once more than necessary.
> >
> > 2. The adding of the ellipsis naively assumes that the last characters
> in the string are single-byte UTF-8 characters. As such when it replaces
> the last bytes of the string then you can end up with split multi-byte
> UTF-8 characters which can have various effects.
>
> I *think* I somewhat improved this issue in `vis-menu` (which is based
> on `dmenu`, IIRC):
>
>
> https://github.com/Shugyousha/vis/commit/d59b98d934815e54320ad000eebfdaaf8fee344d
>
> Note that this is not well tested and still has issues though.
>
>
> Cheers,
>
> Silvan
>
>
--
Stein Gunnar Bakkeby
OpenBet Developer
bakkeby_AT_gmail.com
Received on Sun Mar 20 2022 - 14:56:33 CET