[Bug 214933] [patch] bsdinstall: add support for "hidden" Wi-Fi networks

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=214933

--- Comment #8 from Devin Teske <dteske at FreeBSD.org> ---
Getting better.

Now let's change the following in this stanza:

        ENCRYPTION=$(dialog --backtitle "FreeBSD Installer" --title "Network
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=$([space]...[space])
not VAR=$(...) (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 via
API, i18n-friendly, and free of charge)
NB: When you switched from invoking dialog(1) directly in your previous patch
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=Fr_fr.ISO8859-1 and the French
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 into
the core API)
9. You're assigning the direct result of your dialog invocation as the variable
contents; a safer and more portable approach (taking many things into
consideration) is to rely on a technique that involves defining you menu items
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 within
the dialog API already available to you. It's always a good approach to follow
(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 below.
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 (see
"bsdconfig api -dF sanitize" for more info)


So here's what I would replace it with:

        dialog_menu_enctype()
        {
                local prompt="Select encryption type"
                local menu_list="
                        '1 WPA/WPA2 PSK' ''
                        '2 WPA/WPA2 EAP' ''
                        '3 WEP' ''
                        '4 None ''
                " # END-QUOTE
                local hline="$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=$( 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=$?
                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 reorganization
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.
-- 
Cheers,
Devin

-- 
You are receiving this mail because:
You are the assignee for the bug.