From: PatR <rankin@nethack.org>
Date: Thu, 21 Apr 2022 05:22:36 +0000 (-0700)
Subject: redo status_hilite_menu_fld() [pull request #740]
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0f1674a262c57cb109b948b92b93f162083cb185;p=nethack

redo status_hilite_menu_fld() [pull request #740]

Pull request #740 by argrath removes some redundant code.  Instead
of adopting that, this rewrites the section of code in question.

The menu involved allows the user to select both "delete hilites"
and "add new hilite" but if both are selected it would only do
deletion.  It also falsely claimed to have done something if delete
was chosen but no highlights were selected to be deleted.  And it
only added one new highlight for the specified field when adding.
This supports both add and delete on the same menu invocation, for
addition it keeps adding until no new highlight is specified, and
has caller do the highlight updating if any changes are made.

Supersedes #740
---

diff --git a/src/botl.c b/src/botl.c
index fb96cb688..5dfb8fc51 100644
--- a/src/botl.c
+++ b/src/botl.c
@@ -3886,7 +3886,7 @@ status_hilite_menu_fld(int fld)
     int count = status_hilite_linestr_countfield(fld);
     struct _status_hilite_line_str *hlstr;
     char buf[BUFSZ];
-    boolean acted = FALSE;
+    boolean acted;
 
     if (!count) {
         if (status_hilite_menu_add(fld)) {
@@ -3944,55 +3944,37 @@ status_hilite_menu_fld(int fld)
         any = cg.zeroany;
         any.a_int = -2;
         add_menu(tmpwin, &nul_glyphinfo, &any, 'Z', 0, ATR_NONE,
-                 "Add a new hilite", MENU_ITEMFLAGS_NONE);
+                 "Add new hilites", MENU_ITEMFLAGS_NONE);
     }
 
     Sprintf(buf, "Current %s hilites:", initblstats[fld].fldname);
     end_menu(tmpwin, buf);
 
+    acted = FALSE;
     if ((res = select_menu(tmpwin, PICK_ANY, &picks)) > 0) {
-        int mode = 0;
+        int idx;
+        unsigned mode = 0;
 
         for (i = 0; i < res; i++) {
-            int idx = picks[i].item.a_int;
-
-            if (idx == -1) {
-                /* delete selected hilites */
-                if (mode)
-                    goto shlmenu_free;
-                mode = -1;
-                break;
-            } else if (idx == -2) {
-                /* create a new hilite */
-                if (mode)
-                    goto shlmenu_free;
-                mode = -2;
-                break;
-            }
+            idx = picks[i].item.a_int;
+            if (idx == -1)
+                mode |= 1; /* delete selected hilites */
+            else if (idx == -2)
+                mode |= 2; /* create new hilites */
         }
-
-        if (mode == -1) {
-            /* delete selected hilites */
+        if (mode & 1) { /* delete selected hilites */
             for (i = 0; i < res; i++) {
-                int idx = picks[i].item.a_int;
-
-                if (idx > 0)
-                    (void) status_hilite_remove(idx);
+                idx = picks[i].item.a_int;
+                if (idx > 0 && status_hilite_remove(idx))
+                    acted = TRUE;
             }
-            reset_status_hilites();
-            acted = TRUE;
-        } else if (mode == -2) {
-            /* create a new hilite */
-            if (status_hilite_menu_add(fld))
+        }
+        if (mode & 2) { /* create new hilites */
+            while (status_hilite_menu_add(fld))
                 acted = TRUE;
         }
-
-        free((genericptr_t) picks);
+        free((genericptr_t) picks), picks = 0;
     }
-
- shlmenu_free:
-
-    picks = (menu_item *) 0;
     destroy_nhwindow(tmpwin);
     return acted;
 }
@@ -4072,10 +4054,12 @@ status_hilite_menu(void)
     end_menu(tmpwin, "Status hilites:");
     if ((res = select_menu(tmpwin, PICK_ONE, &picks)) > 0) {
         i = picks->item.a_int - 1;
-        if (i < 0)
+        if (i < 0) {
             status_hilites_viewall();
-        else
-            (void) status_hilite_menu_fld(i);
+        } else {
+            if (status_hilite_menu_fld(i))
+                reset_status_hilites();
+        }
         free((genericptr_t) picks), picks = (menu_item *) 0;
         redo = TRUE;
     }