diff --git a/be_all.c b/be_all.c index c049f020..668541b8 100644 --- a/be_all.c +++ b/be_all.c @@ -27,3 +27,5 @@ const struct BackendVtable *const backends[] = { &sshconn_backend, NULL }; + +const size_t n_ui_backends = 1; diff --git a/be_all_s.c b/be_all_s.c index eb977558..4e502930 100644 --- a/be_all_s.c +++ b/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; diff --git a/be_none.c b/be_none.c index f520e0bb..588bb1d4 100644 --- a/be_none.c +++ b/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; diff --git a/be_nos_s.c b/be_nos_s.c index 566a2ff8..097433aa 100644 --- a/be_nos_s.c +++ b/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; diff --git a/be_nossh.c b/be_nossh.c index a537874f..1c94f3ae 100644 --- a/be_nossh.c +++ b/be_nossh.c @@ -17,3 +17,5 @@ const struct BackendVtable *const backends[] = { &raw_backend, NULL }; + +const size_t n_ui_backends = 1; diff --git a/be_ssh.c b/be_ssh.c index 69e8b8ab..81be62d8 100644 --- a/be_ssh.c +++ b/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 */ diff --git a/config.c b/config.c index 334d4528..b70bfe30 100644 --- a/config.c +++ b/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); } /* diff --git a/putty.h b/putty.h index 206ee905..a73817af 100644 --- a/putty.h +++ b/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.