2014-05-25 14:23 GMT+04:00 FRIGN <dev_AT_frign.de>:
> Hello,
>
> attached is a small patch to refactor xsetcolorname().
> Note the high similarity to xloadcols(). I wonder how necessary it is
> to realloc if name==NULL, given dc.col[] is only read except in
> xloadcols(), where exactly the same stuff is done to each array-item as
> repeated in xsetcolorname().
>
> If I'm wrong, let me know. If not, we could skip the entire if(!
> name)-part.
>
> Cheers
>
> FRIGN
Hello FRIGN,
this function implements STR command for changing a palette. Empty
name specifically tells to revert to system color, thus, if(!name)
clause is there for restoring a color that may have been changed using
the same procedure.
Here:
> if(BETWEEN(x, 16, 6*6*6+16)) { /* 256 colour */
you forgot -1. Don't forget BETWEEN() is inclusive on both ends.
However, the removal of extra stack variable and some additional
comments are very nice. I wholeheartedly approve of them.
Your concern about repetitiveness is valid and bothered me when I
wrote that function, although back then xloadcols() was in much poorer
state. By now, we may as well reimplement xloadcols() in terms of
xsetcolorname(i, NULL). I'd be glad if you did this, as unfortunately,
I do not have access to development environment right now.
--
Best regards, Alexander Sedov.
Received on Mon May 26 2014 - 06:56:58 CEST