This provides a more consistent user experience to the FreeBSD installer.
Existing functionality is meant to be kept in full.
Details
Details
# BSDINSTALL_CHROOT=/ bsdinstall rootpass
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
I am revisiting this revision for the following reasons:
- It allowed the bsddialog command to be replaced with another through the DIALOG environment variable, which is currently not expected by the other scripts (this was implemented in D44672, which now needs a refresh and review)
- There is a new group interested by the installation experience, and they should be given a new chance to review this
- I could now commit this myself, but I would like the approval from my mentor first (philip@)
Thanks Brad for the initial review!
Comment Actions
Also noteworthy in terms of UX:
- With bsddialog, the <TAB> key will go straight from the first input field to the [ OK ] button; the "arrow down" key should be used instead.
- Entering a mismatching password resets both password fields. (Like the passwd(8) command)
- This does not allow setting an empty password; it then fails with error Incorrect input data. (This is unlike the passwd(8) command)
This could be frustrating to some users.
Comment Actions
I have added everyone on the de-facto installer group that met on Monday; sorry for the noise.
(Should we create a Phabricator group for us? Apologies if I did not find it)
| usr.sbin/bsdinstall/scripts/rootpass | ||
|---|---|---|
| 45 | Nit: deindent these by a block as style(9) does for C switch statements | |
| 92 | This is a weird way to display the error? | |
| 103 | Hm, it would be nice to avoid this magic error return (that in theory could come out of chroot or pw...) | |
| 108 | Probably better as err=$?
if [ $err -eq 0 ]; then
break
fi
errormsg=$(error_get_message $err)or that kind of thing? Bit ugly to turn the error into a string and then treat empty string as success. | |
| usr.sbin/bsdinstall/scripts/rootpass | ||
|---|---|---|
| 45 | Thanks for the heads up; updated. | |
| 92 | Suggestions welcome :) | |
| 103 | I'm also having to add an error message for empty passwords, which do not seem to be accepted. | |
| 108 | Good point; updated. | |
Comment Actions
- Re-indented the case statement
- Added a check for empty passwords
- Reworked error management
Comment Actions
Even though I have the approval of my mentor, it would be great to have approval from the installer group for this change before I proceed with the commit.
Please let me know!
Comment Actions
I've done a little testing and this does what I would expect on my test laptop.
I don't find the interface intuitive, but I think you are hamstrung their by what bsddialog offers. I think getting it into installers sooner is best.
Please add "Tested by: thj" when you commit.
Comment Actions
Ok, I can change the behavior in future versions. TAB will navigate through input fields as well, instead of switching only between [Input fields <-> Buttons]. This could be in the next version or later with Lua integration.
- Entering a mismatching password resets both password fields. (Like the passwd(8) command)
- This does not allow setting an empty password; it then fails with error Incorrect input data. (This is unlike the passwd(8) command)
This could be frustrating to some users.