On Wed, Mar 23, 2022 at 11:13:00AM +0100, Stein Gunnar Bakkeby wrote:
> Regarding the first patch I got the impression that the overflow should be
> triggered if (ew + tmpw > w) {

You're correct. I forgot to mention, but that was another bug I have
fixed locally.

> For the second patch I am not sure if this would be bad form or not, but
> the invert variable is only ever used when text is rendered so I was
> wondering if this might be re-used for the purposes of containing the
> minw value when it is calculating text width.
> 
> w = invert ? invert : ~invert;
> 
> This would avoid the wrapper function and change of signature and simplify
> the patch quite a bit. It would probably be best to rename the parameter in
> that case.

I've thought about that as well, but I don't think it's a good idea to
overload the variable with multiple meaning.

But then again library users should be calling drw_fontset_getwidth(),
so maybe it's fine to overload the variable for internal usage. If
that's deemed acceptable then I can change it.

Anyhow, I've been using the patches on both dwm and dmenu, and so far
everything seems to be working as expected :)

I'll wait until tomorrow and give the patches another look with fresh
eyes to see if there's some edge case unhandled before sending them.

- NRK

Reply via email to