Make GenericPopUp() more readable by using more named flags instead of numerals
authorByrial Jensen <byrial@vip.cybercity.dk>
Fri, 16 Dec 2011 01:32:37 +0000 (02:32 +0100)
committerByrial Jensen <byrial@vip.cybercity.dk>
Fri, 16 Dec 2011 01:32:37 +0000 (02:32 +0100)
Defines the flags SAME_ROW (value 1) and NO_OK (value 2) for use with buttons and endmarks.
There should no functional change.
Also add a code comment where a condition depends on an uninitialised value. There is no direct harm, as both
branches do the same (that is nothing) when the value is uninitialised.

backend.h
xoptions.c

index f0d5816..b93f81c 100644 (file)
--- a/backend.h
+++ b/backend.h
@@ -319,13 +319,17 @@ int Explode P((Board board, int fromX, int fromY, int toX, int toY));
 typedef enum { CheckBox, ComboBox, TextBox, Button, Spin, ResetButton, SaveButton,
                 FileName, PathName, Slider, Message, Fractional, Label, Break, EndMark } Control;
 
-/* Flags for Option.min: */             
-#define COMBO_CALLBACK (1 << 0)
-#define NO_GETTEXT     (1 << 1)
-                
+/* Flags Option.min used for ComboBox: */
+#define COMBO_CALLBACK (1 << 0)
+#define NO_GETTEXT     (1 << 1)
+
+/* Flags for Option.min used for Button, SaveButton, EndMark: */
+#define SAME_ROW       (1 << 0)
+#define NO_OK          (1 << 1)
+
 typedef struct _OPT {   // [HGM] options: descriptor of UCI-style option
     int value;          // current setting, starts as default
-    int min;           // Also used for flags with ComboBox
+    int min;           // Also used for flags
     int max;
     void *handle;       // for use by front end
     void *target;       // for use by front end
index 71d9df5..648d452 100644 (file)
@@ -954,7 +954,7 @@ GenericPopUp(Option *option, char *title, int dlgNr)
        if(!n) { DisplayNote(_("Engine has no options")); currentCps = NULL; return 0; }
        if(n > 50) width = 4; else if(n>24) width = 2; else width = 1;
        height = n / width + 1;
-       if(n && (currentOption[n-1].type == Button || currentOption[n-1].type == SaveButton)) currentOption[n].min = 1; // OK on same line
+       if(n && (currentOption[n-1].type == Button || currentOption[n-1].type == SaveButton)) currentOption[n].min = SAME_ROW; // OK on same line
        currentOption[n].type = EndMark; currentOption[n].target = NULL; // delimit list by callback-less end mark
     }
      i = 0;
@@ -1096,10 +1096,14 @@ GenericPopUp(Option *option, char *title, int dlgNr)
          case SaveButton:
          case Button:
            j=0;
-           XtSetArg(args[j], XtNfromVert, option[i].min & 1 ? lastrow : last);  j++;
+           if(option[i].min & SAME_ROW) {
+               XtSetArg(args[j], XtNfromVert, lastrow);  j++;
+               XtSetArg(args[j], XtNfromHoriz, last);  j++;
+           } else {
+               XtSetArg(args[j], XtNfromVert, last);  j++;
+               XtSetArg(args[j], XtNfromHoriz, NULL);  j++; lastrow = forelast;
+           }
            XtSetArg(args[j], XtNlabel, _(option[i].name));  j++;
-           if(option[i].min & 1) { XtSetArg(args[j], XtNfromHoriz, last);  j++; }
-           else  { XtSetArg(args[j], XtNfromHoriz, NULL);  j++; lastrow = forelast; }
            if(option[i].max) { XtSetArg(args[j], XtNwidth, option[i].max);  j++; }
            if(option[i].textValue) { // special for buttons of New Variant dialog
                XtSetArg(args[j], XtNsensitive, appData.noChessProgram || option[i].value < 0
@@ -1182,7 +1186,7 @@ GenericPopUp(Option *option, char *title, int dlgNr)
     for(h=0; h<height; h++) {
        i = h + c*height;
        if(option[i].type == EndMark) break;
-       if(!texts[h]) continue;
+       if(!texts[h]) continue; // Note: texts[h] can be undefined (giving errors in valgrind), but then both if's below will be false.
        j=0;
        if(option[i].type == Spin) {
            XtSetArg(args[j], XtNwidth, maxWidth);  j++;
@@ -1195,10 +1199,10 @@ GenericPopUp(Option *option, char *title, int dlgNr)
     }
   }
 
-  if(!(option[i].min & 2)) {
+  if(!(option[i].min & NO_OK)) {
     j=0;
-    if(option[i].min & 1) {
-       for(j=i-1; option[j+1].min&1 && option[j].type == Button; j--) {
+    if(option[i].min & SAME_ROW) {
+       for(j=i-1; option[j+1].min & SAME_ROW && option[j].type == Button; j--) {
            XtSetArg(args[0], XtNtop, XtChainBottom);
            XtSetArg(args[1], XtNbottom, XtChainBottom);
            XtSetValues(option[j].handle, args, 2);