New GUI for protocol selection.

This replaces the pure radio-button setup that we've always had on the
Session config panel.

Since the last release, that set of radio buttons has been getting out
of hand. We've added two new protocols (SUPDUP, and the 'bare
ssh-connection' aka psusan protocol), neither of which is mainstream
enough to be a sensible thing to wave at all users on the front page
of the config GUI, so that they perhaps start wondering if that's the
protocol they want to use, or get sidetracked by going and looking it
up.

The replacement UI still has radio buttons, but only for the most
common protocols, which will typically be SSH and serial. Everything
else is relegated to a drop-down list sitting next to a third radio
button labelled "Other".

In every be_* module providing a backends[] list, there's also a
variable n_ui_backends which indicates how many of the backends ought
to appear as first-level radio buttons.

(Credit where due: this patch is a joint effort between Jacob and me,
and is one of those rare cases where it would be nice to be able to
put both our names into the Author field of the commit. Failing that,
I can at least mention it here.)

be_all.c

@@ -27,3 +27,5 @@ const struct BackendVtable *const backends[] = {
     &sshconn_backend,
     NULL
 };
+
+const size_t n_ui_backends = 1;

be_all_s.c

@@ -20,11 +20,13 @@ const int be_default_protocol = PROT_SSH;
 
 const struct BackendVtable *const backends[] = {
     &ssh_backend,
+    &serial_backend,
     &telnet_backend,
     &rlogin_backend,
     &supdup_backend,
     &raw_backend,
-    &serial_backend,
     &sshconn_backend,
     NULL
 };
+
+const size_t n_ui_backends = 2;

be_none.c

@@ -11,3 +11,5 @@ const int be_default_protocol = -1;
 const struct BackendVtable *const backends[] = {
     NULL
 };
+
+const size_t n_ui_backends = 0;

be_nos_s.c

@@ -12,9 +12,11 @@ const char *const appname = "PuTTYtel";
 
 const struct BackendVtable *const backends[] = {
     &telnet_backend,
+    &serial_backend,
     &rlogin_backend,
     &supdup_backend,
     &raw_backend,
-    &serial_backend,
     NULL
 };
+
+const size_t n_ui_backends = 2;

be_nossh.c

@@ -17,3 +17,5 @@ const struct BackendVtable *const backends[] = {
     &raw_backend,
     NULL
 };
+
+const size_t n_ui_backends = 1;

be_ssh.c

@@ -14,3 +14,5 @@ const struct BackendVtable *const backends[] = {
     &sshconn_backend,
     NULL
 };
+
+const size_t n_ui_backends = 0;  /* not used in programs with a config UI */

config.c

@@ -257,60 +257,167 @@ static void config_port_handler(union control *ctrl, dlgparam *dlg,
 }
 
 struct hostport {
-    union control *host, *port;
+    union control *host, *port, *protradio, *protlist;
+    bool mid_refresh;
 };
 
-static void config_protocolbuttons_handler(union control *ctrl, dlgparam *dlg,
-                                           void *data, int event)
+/*
+ * Shared handler for protocol radio-button and drop-list controls.
+ * Handles the interaction of those two controls, and also changes
+ * the setting of the port box to match the protocol if necessary,
+ * and refreshes both host and port boxes when switching to/from the
+ * serial backend.
+ */
+static void config_protocols_handler(union control *ctrl, dlgparam *dlg,
+                                     void *data, int event)
 {
-    int button;
     Conf *conf = (Conf *)data;
-    struct hostport *hp = (struct hostport *)ctrl->radio.context.p;
+    int curproto = conf_get_int(conf, CONF_protocol);
+    struct hostport *hp = (struct hostport *)ctrl->generic.context.p;
 
-    /*
-     * This function works just like the standard radio-button
-     * handler, except that it also has to change the setting of
-     * the port box, and refresh both host and port boxes when. We
-     * expect the context parameter to point at a hostport
-     * structure giving the `union control's for both.
-     */
     if (event == EVENT_REFRESH) {
-        int protocol = conf_get_int(conf, CONF_protocol);
-        for (button = 0; button < ctrl->radio.nbuttons; button++)
-            if (protocol == ctrl->radio.buttondata[button].i)
-                break;
-        /* We expected that `break' to happen, in all circumstances. */
-        assert(button < ctrl->radio.nbuttons);
-        dlg_radiobutton_set(ctrl, dlg, button);
-    } else if (event == EVENT_VALCHANGE) {
-        int oldproto = conf_get_int(conf, CONF_protocol);
-        int newproto, port;
+        /*
+         * Refresh the states of the controls from Conf.
+         *
+         * When refreshing these controls, we have to watch out for
+         * re-entrancy: because there are two controls involved, the
+         * refresh is not atomic, so the VALCHANGE and/or SELCHANGE
+         * callbacks resulting from our updates here might cause other
+         * settings here to change unwantedly. (E.g. setting the list
+         * selection shouldn't trigger the SELCHANGE side effect of
+         * selecting the Other radio button; setting the radio button
+         * to Other here shouldn't have the side effect of selecting
+         * whatever protocol is _currently_ selected in the list box,
+         * if we haven't selected the right one yet.)
+         */
+        hp->mid_refresh = true;
 
-        button = dlg_radiobutton_get(ctrl, dlg);
-        assert(button >= 0 && button < ctrl->radio.nbuttons);
-        newproto = ctrl->radio.buttondata[button].i;
-        conf_set_int(conf, CONF_protocol, newproto);
+        if (ctrl == hp->protradio) {
+            /* Available buttons were set up when control was created.
+             * Just select one of them, possibly. */
+            for (int button = 0; button < ctrl->radio.nbuttons; button++)
+                /* The final button is "Other:". If we reach that one, the
+                 * current protocol must be in the drop list, so we should
+                 * select the "Other:" button. */
+                if (curproto == ctrl->radio.buttondata[button].i ||
+                    button == ctrl->radio.nbuttons-1) {
+                    dlg_radiobutton_set(ctrl, dlg, button);
+                    break;
+                }
+        } else if (ctrl == hp->protlist) {
+            int curentry = -1;
+            dlg_update_start(ctrl, dlg);
+            dlg_listbox_clear(ctrl, dlg);
+            assert(n_ui_backends > 0 && n_ui_backends < PROTOCOL_LIMIT);
+            for (size_t i = n_ui_backends;
+                 i < PROTOCOL_LIMIT && backends[i]; i++) {
+                dlg_listbox_addwithid(ctrl, dlg,
+                                      backends[i]->displayname,
+                                      backends[i]->protocol);
+                if (backends[i]->protocol == curproto)
+                    curentry = i - n_ui_backends;
+            }
+            if (curentry > 0) {
+                /*
+                 * The currently configured protocol is one of the
+                 * list-box ones, so select it in protlist.
+                 *
+                 * (The corresponding refresh event for protradio
+                 * should have selected the "Other:" radio button, to
+                 * keep things consistent.)
+                 */
+                dlg_listbox_select(ctrl, dlg, curentry);
+            } else {
+                /*
+                 * If the currently configured protocol is one of the
+                 * radio buttons, we must still ensure *something* is
+                 * selected in the list box. The sensible default is
+                 * the first list element, which be_*.c ought to have
+                 * arranged to be the 'runner-up' in protocol
+                 * popularity out of the ones relegated to the list
+                 * box.
+                 *
+                 * We don't make much effort to retain the state of
+                 * the list box when it doesn't correspond to an
+                 * actual protocol. So it's easy for this case to be
+                 * reached as a side effect of other actions, e.g.
+                 * loading a saved session that has a radio-button
+                 * protocol configured.
+                 */
+                dlg_listbox_select(ctrl, dlg, 0);
+            }
+            dlg_update_done(ctrl, dlg);
+        }
 
-        if (oldproto != newproto) {
-            const struct BackendVtable *ovt = backend_vt_from_proto(oldproto);
+        hp->mid_refresh = false;
+    } else if (!hp->mid_refresh) {
+        /*
+         * Potentially update Conf from the states of the controls.
+         */
+        int newproto = curproto;
+
+        if (event == EVENT_VALCHANGE && ctrl == hp->protradio) {
+            int button = dlg_radiobutton_get(ctrl, dlg);
+            assert(button >= 0 && button < ctrl->radio.nbuttons);
+            if (ctrl->radio.buttondata[button].i == -1) {
+                /*
+                 * The 'Other' radio button was selected, which means we
+                 * have to set CONF_protocol based on the currently
+                 * selected list box entry.
+                 *
+                 * (We conditionalise this on there _being_ a selected
+                 * list box entry. I hope the case where nothing is
+                 * selected can't actually come up except during
+                 * initialisation, and I also hope that hp->mid_session
+                 * will prevent that case from getting here. But as a
+                 * last-ditch fallback, this if statement should at least
+                 * guarantee that we don't pass a nonsense value to
+                 * dlg_listbox_getid.)
+                 */
+                int i = dlg_listbox_index(hp->protlist, dlg);
+                if (i >= 0)
+                    newproto = dlg_listbox_getid(hp->protlist, dlg, i);
+            } else {
+                newproto = ctrl->radio.buttondata[button].i;
+            }
+        } else if (event == EVENT_SELCHANGE && ctrl == hp->protlist) {
+            int i = dlg_listbox_index(ctrl, dlg);
+            if (i >= 0) {
+                newproto = dlg_listbox_getid(ctrl, dlg, i);
+                /* Select the "Other" radio button, too */
+                dlg_radiobutton_set(hp->protradio, dlg,
+                                    hp->protradio->radio.nbuttons-1);
+            }
+        }
+
+        if (newproto != curproto) {
+            conf_set_int(conf, CONF_protocol, newproto);
+
+            const struct BackendVtable *cvt = backend_vt_from_proto(curproto);
             const struct BackendVtable *nvt = backend_vt_from_proto(newproto);
-            assert(ovt);
+            assert(cvt);
             assert(nvt);
-            /* Iff the user hasn't changed the port from the old protocol's
-             * default, update it with the new protocol's default.
-             * (This includes a "default" of 0, implying that there is no
-             * sensible default for that protocol; in this case it's
-             * displayed as a blank.)
+            /*
+             * Iff the user hasn't changed the port from the old
+             * protocol's default, update it with the new protocol's
+             * default.
+             *
+             * (This includes a "default" of 0, implying that there is
+             * no sensible default for that protocol; in this case
+             * it's displayed as a blank.)
+             *
              * This helps with the common case of tabbing through the
              * controls in order and setting a non-default port before
              * getting to the protocol; we want that non-default port
-             * to be preserved. */
-            port = conf_get_int(conf, CONF_port);
-            if (port == ovt->default_port)
+             * to be preserved.
+             */
+            int port = conf_get_int(conf, CONF_port);
+            if (port == cvt->default_port)
                 conf_set_int(conf, CONF_port, nvt->default_port);
+
+            dlg_refresh(hp->host, dlg);
+            dlg_refresh(hp->port, dlg);
         }
-        dlg_refresh(hp->host, dlg);
-        dlg_refresh(hp->port, dlg);
     }
 }
 
@@ -1621,6 +1728,7 @@ void setup_config_box(struct controlbox *b, bool midsession,
     if (!midsession) {
         struct hostport *hp = (struct hostport *)
             ctrl_alloc(b, sizeof(struct hostport));
+        memset(hp, 0, sizeof(*hp));
 
         s = ctrl_getset(b, "Session", "hostport",
                         "Specify the destination you want to connect to");
@@ -1635,37 +1743,54 @@ void setup_config_box(struct controlbox *b, bool midsession,
                          config_port_handler, I(0), I(0));
         c->generic.column = 1;
         hp->port = c;
-        ctrl_columns(s, 1, 100);
 
-        c = ctrl_radiobuttons(s, "Connection type:", NO_SHORTCUT, 4,
+        ctrl_columns(s, 1, 100);
+        c = ctrl_text(s, "Connection type:", HELPCTX(session_hostname));
+        ctrl_columns(s, 2, 62, 38);
+        c = ctrl_radiobuttons(s, NULL, NO_SHORTCUT, 3,
                               HELPCTX(session_hostname),
-                              config_protocolbuttons_handler, P(hp), NULL);
+                              config_protocols_handler, P(hp), NULL);
+        c->generic.column = 0;
+        hp->protradio = c;
         c->radio.buttons = sresize(c->radio.buttons, PROTOCOL_LIMIT, char *);
         c->radio.shortcuts = sresize(c->radio.shortcuts, PROTOCOL_LIMIT, char);
         c->radio.buttondata = sresize(c->radio.buttondata, PROTOCOL_LIMIT,
                                       intorptr);
         assert(c->radio.nbuttons == 0);
-        for (int pass = 0; pass < 4; pass++) {
-            for (size_t i = 0; backends[i]; i++) {
-                int pass_needed = (
-                    backends[i]->protocol == be_default_protocol ? 0 :
-                    backends[i]->protocol == PROT_SERIAL ? 1 :
-                    backends[i]->protocol == PROT_RAW ? 2 : 3);
-                if (pass != pass_needed)
-                    continue;
-
-                c->radio.buttons[c->radio.nbuttons] =
-                    dupstr(backends[i]->displayname);
-                c->radio.shortcuts[c->radio.nbuttons] =
-                    (backends[i]->protocol == PROT_SSH ? 's' :
-                     backends[i]->protocol == PROT_SERIAL ? 'r' :
-                     backends[i]->protocol == PROT_RAW ? 'w' :
-                     NO_SHORTCUT);
-                c->radio.buttondata[c->radio.nbuttons] =
-                    I(backends[i]->protocol);
-                c->radio.nbuttons++;
-            }
+        /* UI design assumes there exists at least one 'real' radio button */
+        assert(n_ui_backends > 0 && n_ui_backends < PROTOCOL_LIMIT);
+        for (size_t i = 0; i < n_ui_backends; i++) {
+            assert(backends[i]);
+            c->radio.buttons[c->radio.nbuttons] =
+                dupstr(backends[i]->displayname);
+            c->radio.shortcuts[c->radio.nbuttons] =
+                (backends[i]->protocol == PROT_SSH ? 's' :
+                 backends[i]->protocol == PROT_SERIAL ? 'r' :
+                 backends[i]->protocol == PROT_RAW ? 'w' :  /* FIXME unused */
+                 NO_SHORTCUT);
+            c->radio.buttondata[c->radio.nbuttons] =
+                I(backends[i]->protocol);
+            c->radio.nbuttons++;
         }
+        /* UI design assumes there exists at least one droplist entry */
+        assert(backends[c->radio.nbuttons]);
+
+        c->radio.buttons[c->radio.nbuttons] = dupstr("Other:");
+        c->radio.shortcuts[c->radio.nbuttons] = 't';
+        c->radio.buttondata[c->radio.nbuttons] = I(-1);
+        c->radio.nbuttons++;
+
+        c = ctrl_droplist(s, NULL, NO_SHORTCUT, 100,
+                          HELPCTX(session_hostname),
+                          config_protocols_handler, P(hp));
+        hp->protlist = c;
+        /* droplist is populated in config_protocols_handler */
+        c->generic.column = 1;
+
+        /* Vertically centre the two protocol controls w.r.t. each other */
+        hp->protlist->generic.align_next_to = hp->protradio;
+
+        ctrl_columns(s, 1, 100);
     }
 
     /*

putty.h

@@ -708,6 +708,12 @@ static inline int backend_cfg_info(Backend *be)
 { return be->vt->cfg_info(be); }
 
 extern const struct BackendVtable *const backends[];
+/*
+ * In programs with a config UI, only the first few members of
+ * backends[] will be displayed at the top-level; the others will be
+ * relegated to a drop-down.
+ */
+extern const size_t n_ui_backends;
 
 /*
  * Suggested default protocol provided by the backend link module.