From owner-freebsd-sysinstall@freebsd.org Sat Dec 3 05:45:25 2016 Return-Path: Delivered-To: freebsd-sysinstall@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id D0EB9C64B10 for ; Sat, 3 Dec 2016 05:45:25 +0000 (UTC) (envelope-from bugzilla-noreply@freebsd.org) Received: from kenobi.freebsd.org (kenobi.freebsd.org [IPv6:2001:1900:2254:206a::16:76]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C03C0F2C for ; Sat, 3 Dec 2016 05:45:25 +0000 (UTC) (envelope-from bugzilla-noreply@freebsd.org) Received: from bugs.freebsd.org ([127.0.1.118]) by kenobi.freebsd.org (8.15.2/8.15.2) with ESMTP id uB35jODY051114 for ; Sat, 3 Dec 2016 05:45:25 GMT (envelope-from bugzilla-noreply@freebsd.org) From: bugzilla-noreply@freebsd.org To: freebsd-sysinstall@FreeBSD.org Subject: [Bug 214933] [patch] bsdinstall: add support for "hidden" Wi-Fi networks Date: Sat, 03 Dec 2016 05:45:24 +0000 X-Bugzilla-Reason: AssignedTo X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: Base System X-Bugzilla-Component: bin X-Bugzilla-Version: CURRENT X-Bugzilla-Keywords: patch X-Bugzilla-Severity: Affects Only Me X-Bugzilla-Who: dteske@FreeBSD.org X-Bugzilla-Status: New X-Bugzilla-Resolution: X-Bugzilla-Priority: --- X-Bugzilla-Assigned-To: freebsd-sysinstall@FreeBSD.org X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: https://bugs.freebsd.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: freebsd-sysinstall@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Sysinstall Work List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 03 Dec 2016 05:45:25 -0000 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D214933 --- Comment #8 from Devin Teske --- Getting better. Now let's change the following in this stanza: ENCRYPTION=3D$(dialog --backtitle "FreeBSD Installer" --title "Netw= ork Selection" --menu "Select encryption type" 0 0 0 "WPA/WPA2 PSK" "" "WPA/WPA2 EAP" "" "WEP" "" "None" "" 2>&1 1>&3) 1. [FYI] FreeBSD's style in bsdinstall/bsdconfig is VAR=3D$([space]...[spac= e]) not VAR=3D$(...) (just add space after the left-paren and before the right-= paren) 2. You should defer the f_dialog_title_restore() call (do it later) so you = can use $DIALOG_TITLE here to reduce duplication (e.g., --title "$DIALOG_TITLE") 3. With f_dialog_title_restore() done later, you can use $DIALOG_BACKTITLE = as well (e.g., --backtitle "$DIALOG_BACKTITLE") 4. You've not used the internationalized attributes (e.g., --ok-label "$msg= _ok" --cancel-label "$msg_cancel" (NB: $msg_ok and $msg_cancel are already set v= ia API, i18n-friendly, and free of charge) NB: When you switched from invoking dialog(1) directly in your previous pat= ch to using f_dialog_yesno() from the dialog API, you got i18n capabilities in that dialog for free (the Yes/No dialog will pick up alternate languages if loaded; e.g., it might say "Oui/Non" if LC_ALL=3DFr_fr.ISO8859-1 and the Fr= ench language files are loaded, which my friend Guillaume has, over @mozilla). 5. You've not calculated the optimum width/height for the list (relying on dialog to do this is faulty) 6. You're not supporting alternate dialog back-ends 7. Multiple menu items start with the letter W (preventing the user from utilizing the type-ahead ability in dialog(1) to highlight the item starting with unique letter, making it easier to select said item with space/enter) 8. You don't provide an "hline" to give the user a hint as to what keys are appropriate for interacting with the dialog (an example is given below where hline is in-use and adheres to i18n standards to allow alternate languages loaded) NB: I used $hline_arrows_tab_enter which is defined in the core i18n layer (/usr/libexec/bsdconfig/include/messages.subr[.$LC_ALL] is included by dialog.subr at the top of bsdinstall's scripts/wlanconfig so it's available= for free; if you define your own value that is not already available, it should= be added to the top of scripts/wlanconfig as a candidate for later inclusion i= nto the core API) 9. You're assigning the direct result of your dialog invocation as the vari= able contents; a safer and more portable approach (taking many things into consideration) is to rely on a technique that involves defining you menu it= ems separately and using the dialog API elements for mapping back the results (= see below example). NB: Assigning your menu list to a variable allows you to do menu things wit= hin the dialog API already available to you. It's always a good approach to fol= low (again, see below; you can calculate the appropriate size of a dialog given said info). 10. You're redirecting stdout to a static file descriptor (3) when this information has been abstracted to a variable for you, should it ever be customized and/or changed. Use $DIALOG_TERMINAL_PASSTHRU_FD as is shown bel= ow. 11. You haven't loaded your menu into a function for convenience. There's a reason why dialog's --menu doesn't have an entry in the dialog API and that= 's because, well, I haven't sat down to write it yet but really, because it's pretty heady when dealing with. You'll see below that it works well in a function (for which the standard naming convention is "dialog_menu_*()". 12. You're not sanitizing the dialog response for spurious libc messages (s= ee "bsdconfig api -dF sanitize" for more info) So here's what I would replace it with: dialog_menu_enctype() { local prompt=3D"Select encryption type" local menu_list=3D" '1 WPA/WPA2 PSK' '' '2 WPA/WPA2 EAP' '' '3 WEP' '' '4 None '' " # END-QUOTE local hline=3D"$hline_arrows_tab_enter" local height width rows eval f_dialog_menu_size height width rows \ \"\$DIALOG_TITLE\" \ \"\$DIALOG_BACKTITLE\" \ \"\$prompt\" \ \"\$hline\" \ $menu_list local menu_choice menu_choice=3D$( eval $DIALOG \ --title \"\$DIALOG_TITLE\" \ --backtitle \"\$DIALOG_BACKTITLE\" \ --hline \"\$hline\" \ --ok-label \"\$msg_ok\" \ --cancel-label \"\$msg_cancel\" \ --menu \"\$prompt\" \ $height $width $rows \ $menu_list \ 2>&1 >&$DIALOG_TERMINAL_PASSTHRU_FD ) local retval=3D$? f_dialog_menutag_sanitize menu_choice f_dialog_menutag_store -s "$menu_choice" return $retval } This also introduces a new API to use (a pair of functions, actually): f_dialog_menutag_store() and f_dialog_menutag_fetch() (see "bsdconfig api -= dF _menutag_" for more info). Here's how the dialog_menu_enctype() function should be used: dialog_menu_enctype || exit f_dialog_menutag_fetch mtag case "$mtag" in *" WPA/WPA2 PSK") ... ;; *" WPA/WPA2 EAP") ... ;; *" WEP") ... ;; *" None") ... ;; esac Note how the case/esac uses * for the numerical prefix, allowing reorganiza= tion at a future date, if necessary. Continuing... It looks like the following was at the wrong indentation level: 1) # No exit 1 ;; And the "esac" on the very next line appears to be extra and undesired. --=20 Cheers, Devin --=20 You are receiving this mail because: You are the assignee for the bug.=