----- Doug Torrance <dtorra...@monmouthcollege.edu> a écrit : > The RGB panel of the WINGs color panel lists the red, green, and blue values > as > base 10 numbers. However, hexadecimal numbers are very common when dealing > with > RGB colors. This patch adds two radio buttons at the bottom of the RGB panel > to allow users to choose their preferred number system. > --- > WINGs/wcolorpanel.c | 103 > +++++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 85 insertions(+), 18 deletions(-) > > diff --git a/WINGs/wcolorpanel.c b/WINGs/wcolorpanel.c > index 2977702..64ce177 100644 > --- a/WINGs/wcolorpanel.c > +++ b/WINGs/wcolorpanel.c > @@ -161,6 +161,9 @@ typedef struct W_ColorPanel { > WMButton *grayPresetBtn[7]; > > /* RGB Panel */ > + int rgbState; > [...] > +enum { > + RGBdec, > + RGBhex > +};
Hi, May I suggest something different here? enums are much more that a replacement for #defines, so you should use something like this in the structure: + enum { + ... + } rgbState; So the variable will be more than just an int, the compiler can make a number of consistency checks which are useful. > [...] > +void rgbIntToChar(W_ColorPanel *panel, int *value) > +{ > + char tmp[4]; > + char *format; the format being a const string, I would suggest 'const char *format' > + > + if (panel->rgbState == RGBdec) > + format = "%d"; > + if (panel->rgbState == RGBhex) > + format = "%0X"; a switch (panel->rgbState) would probably be better here, amongst other things because by using an enum instead of an int the compiler will report a missing case at compile time (if we suppose someday someone adds RGBoct for example) instead of a strange crash at run time > [...] > +int *rgbCharToInt(W_ColorPanel *panel) > +{ > + static int value[3]; > + > + if (panel->rgbState == RGBdec) { > + value[0] = atoi(WMGetTextFieldText(panel->rgbRedT)); > + value[1] = atoi(WMGetTextFieldText(panel->rgbGreenT)); > + value[2] = atoi(WMGetTextFieldText(panel->rgbBlueT)); > + } > + if (panel->rgbState == RGBhex) { > + value[0] = strtol(WMGetTextFieldText(panel->rgbRedT), NULL, 16); > + value[1] = strtol(WMGetTextFieldText(panel->rgbGreenT), NULL, > 16); > + value[2] = strtol(WMGetTextFieldText(panel->rgbBlueT), NULL, > 16); > + } Same here, a switch would be better. you could probably also use strtol for both case, so actually just switching the value for the base which means less duplicated code. > [...] > @@ -2420,6 +2465,28 @@ static void rgbTextFieldCallback(void *observerData, > WMNotification * notificati > panel->lastChanged = WMRGBModeColorPanel; > } > > +static void rgbDecToHex(WMWidget *w, void *data) > +{ > + W_ColorPanel *panel = (W_ColorPanel *) data; > + (void) w; > + int *value; The "(void) w;" is actually a statement, not a declaration, so it should be after the variable declarations (with a blank line before and after). > [...] Regards, Christophe. -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.