Re: [hackers] [dwm][PATCH] Status bar magic numbers replaced with configurable variables.

From: Christopher Drelich <cd_AT_cdrakka.com>
Date: Fri, 25 May 2018 19:49:19 -0400

There were two variants of this patch I sent in. The first one allows
for identical behavior with configuration options. The second one does
not have identical behavior as pre-patch (StatusText now has left and
right padding.) I would recommend using the second patch.

For me something like this is just plain arithmetic and not a magic number:
 lrpad / 2

The ones i changed I saw as magic numbers because there is no specific
reason for it to be "2" instead of "20." The value is also used twice
in the code with no specified indication of their connection. I
thought it might be helpful to make this connection clear as well as
add configuration options for horizontal and vertical padding in the
statusbar.

I have submitted the second variant of this patch to the wiki so that
those who are interested can have it available to them. If there is
interest I can add the first patch to the wiki as well.

I apologize if the patches I've been submitting have been coming off
as "bike shedding," as that was not my intention. I've been going
through the dwm code base working hard to understand it and
remembering all my C as well. In the process there were certain things
I would find confusing, missing, or misplaced and fixing these things
were helpful to my understanding of the code base. I thought the
changes would be helpful to the code base as well.

On Fri, May 25, 2018 at 6:59 AM, David Demelier <markand_AT_malikania.fr> wrote:
> On Fri, 2018-05-25 at 06:50 +0200, Hiltjo Posthuma wrote:
>> On Thu, May 24, 2018 at 10:48:27PM -0400, Christopher Drelich wrote:
>> > Currently in dwm there are two magic numbers relating to the
>> > statusbar in dwm.c:
>> >
>> > sw = TEXTW(stext) - lrpad + 2; /* 2px right padding */
>> >
>> > bh = drw->fonts->h + 2;
>> >
>> > I made a patch that replaced these magic numbers with configurable
>> > variables, plus a third configurable variable that made sense with
>> > these:
>> >
>> > vertbarpad is vertical padding for the statusbar.
>> > horizbarpad is horizontal padding for the statusbar.
>> > statusrpad is right hand padding for StatusText in the statusbar.
>> >
>> > It would be possible to add a 'statuslpad' as well, or to just make
>> > the padding for the StatusText the same as that for other elements
>> > of
>> > the statusbar, or to divide it by 2 and just use that for the
>> > rightpadding.
>> >
>> > I think that having StatusText have the same padding as other
>> > elements
>> > in the statusbar would probably end up making things cleanest.
>> >
>> > The main purpose of this patch is to eliminate 'magic numbers,' so
>> > the
>> > hope is to open up discussion on how to do that, be it by this
>> > patch,
>> > a suggested variant of it, something else, or if people think they
>> > should just stay.
>> > Chris
>> >
>>
>> How are they magic? Is any number in arithmetic some magic number for
>> you?
>>
>
> Yes but at least when you want to adjust the padding you only have to
> do it in one place.
>
> I completely second this patch.
>
> Regards,
>
> --
> David
>
Received on Sat May 26 2018 - 01:49:19 CEST