Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 03 Dec 2016 05:45:24 +0000
From:      bugzilla-noreply@freebsd.org
To:        freebsd-sysinstall@FreeBSD.org
Subject:   [Bug 214933] [patch] bsdinstall: add support for "hidden" Wi-Fi networks
Message-ID:  <bug-214933-2920-64GiqcbJ5m@https.bugs.freebsd.org/bugzilla/>
In-Reply-To: <bug-214933-2920@https.bugs.freebsd.org/bugzilla/>
References:  <bug-214933-2920@https.bugs.freebsd.org/bugzilla/>

next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D214933

--- Comment #8 from Devin Teske <dteske@FreeBSD.org> ---
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.=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-214933-2920-64GiqcbJ5m>