From: David Shea dshea@redhat.com
Add a new signal handler that can be blocked during username guessing instead of cleaning up variables after the fact and hoping nothing was asynchronous.
Also quit using a dict as a boolean, remove the references to the hostname guessing that never existed that was probably the reason for the dict, and remove gender-specific language. --- pyanaconda/ui/gui/spokes/user.glade | 1 + pyanaconda/ui/gui/spokes/user.py | 51 ++++++++++++++++++++----------------- 2 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/user.glade b/pyanaconda/ui/gui/spokes/user.glade index e15c966..8025849 100644 --- a/pyanaconda/ui/gui/spokes/user.glade +++ b/pyanaconda/ui/gui/spokes/user.glade @@ -111,6 +111,7 @@ <property name="visible">True</property> <property name="can_focus">True</property> <signal name="changed" handler="username_changed" swapped="no"/> + <signal name="changed" handler="on_username_set_by_user" swapped="no"/> <child internal-child="accessible"> <object class="AtkObject" id="t_username-atkobject"> <property name="AtkObject::accessible-name" translatable="yes">User Name</property> diff --git a/pyanaconda/ui/gui/spokes/user.py b/pyanaconda/ui/gui/spokes/user.py index f627e0f..317aeba 100644 --- a/pyanaconda/ui/gui/spokes/user.py +++ b/pyanaconda/ui/gui/spokes/user.py @@ -32,6 +32,7 @@ from pyanaconda.ui.common import FirstbootSpokeMixIn from pyanaconda.ui.helpers import InputCheck from pyanaconda.ui.gui.helpers import GUISpokeInputCheckHandler, GUIDialogInputCheckHandler +from pyanaconda.ui.gui.utils import blockedHandler
from pyanaconda.constants import ANACONDA_ENVIRON, FIRSTBOOT_ENVIRON,\ PASSWORD_EMPTY_ERROR, PASSWORD_CONFIRM_ERROR_GUI, PASSWORD_STRENGTH_DESC,\ @@ -248,9 +249,7 @@ def initialize(self): self._waiveStrengthClicks = 0 self._waiveASCIIClicks = 0
- self.guesser = { - self.username: True - } + self.guesser = True
# Updated during the password changed event and used by the password # field validity checker @@ -442,35 +441,41 @@ def password_changed(self, editable=None, data=None): # Update the password/confirm match check on changes to the main password field self._confirm_check.update_check_status()
- def username_changed(self, editable=None, data=None): - """Called by Gtk callback when the username or hostname - entry changes. It disables the guess algorithm if the - user added his own text there and reenable it when the - user deletes the whole text.""" + def on_username_set_by_user(self, editable, data=None): + """Called by Gtk on user-driven changes to the username field.
- if editable.get_text() == "": - self.guesser[editable] = True - self.b_advanced.set_sensitive(False) + This handler is blocked during changes from the username guesser. + """ + + # If the user set a user name, turn off the username guesser. + # If the user cleared the username, turn it back on. + if editable.get_text(): + self.guesser = False else: - self.guesser[editable] = False + self.guesser = True + + def username_changed(self, editable, data=None): + """Called by Gtk on all username changes.""" + + # Disable the advanced user dialog button when no username is set + if editable.get_text(): self.b_advanced.set_sensitive(True) + else: + self.b_advanced.set_sensitive(False)
- # Re-run the password checks against the new username - self.pw.emit("changed") - self.confirm.emit("changed") + # Re-run the password checks against the new username + self.pw.emit("changed") + self.confirm.emit("changed")
def full_name_changed(self, editable=None, data=None): - """Called by Gtk callback when the full name field changes. - It guesses the username and hostname, strips diacritics - and make those lowercase. - """ + """Called by Gtk callback when the full name field changes."""
- # after the text is updated in guesser, the guess has to be reenabled - if self.guesser[self.username]: + if self.guesser: fullname = self.fullname.get_text() username = guess_username(fullname) - self.username.set_text(username) - self.guesser[self.username] = True + + with blockedHandler(self.username, self.on_username_set_by_user): + self.username.set_text(username)
def on_admin_toggled(self, togglebutton, data=None): # Add or remove "wheel" from the grouplist on changes to the admin checkbox
From: David Shea dshea@redhat.com
Use different signal handlers for different widgets in the advanced user dialog so that unrelated portions of the UI do not need to be checked for every change. Remove an unnecessary call to the advanced user button signal handler that is handled by the initial state of the dialog. Use the arguments passed in to the signal handlers. --- pyanaconda/ui/gui/spokes/advanced_user.glade | 4 ++-- pyanaconda/ui/gui/spokes/user.py | 28 ++++++++++++---------------- 2 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/advanced_user.glade b/pyanaconda/ui/gui/spokes/advanced_user.glade index 8bec496..c2252d6 100644 --- a/pyanaconda/ui/gui/spokes/advanced_user.glade +++ b/pyanaconda/ui/gui/spokes/advanced_user.glade @@ -165,7 +165,7 @@ <property name="xalign">0</property> <property name="draw_indicator">True</property> <signal name="mnemonic-activate" handler="on_uid_mnemonic_activate" swapped="no"/> - <signal name="toggled" handler="_apply_checkboxes" swapped="no"/> + <signal name="toggled" handler="on_uid_checkbox_toggled" swapped="no"/> </object> <packing> <property name="left_attach">0</property> @@ -182,7 +182,7 @@ <property name="xalign">0</property> <property name="draw_indicator">True</property> <signal name="mnemonic-activate" handler="on_gid_mnemonic_activate" swapped="no"/> - <signal name="toggled" handler="_apply_checkboxes" swapped="no"/> + <signal name="toggled" handler="on_gid_checkbox_toggled" swapped="no"/> </object> <packing> <property name="left_attach">0</property> diff --git a/pyanaconda/ui/gui/spokes/user.py b/pyanaconda/ui/gui/spokes/user.py index 317aeba..5c20e3e 100644 --- a/pyanaconda/ui/gui/spokes/user.py +++ b/pyanaconda/ui/gui/spokes/user.py @@ -99,17 +99,6 @@ def initialize(self): # Validate the group input box self.add_check(self._tGroups, self._validateGroups)
- def _apply_checkboxes(self, _editable=None, data=None): - """Update the state of this screen according to the - checkbox states on the screen. It is called from - the toggled Gtk event. - """ - c_uid = self._cUid.get_active() - c_gid = self._cGid.get_active() - - self._spinUid.set_sensitive(c_uid) - self._spinGid.set_sensitive(c_gid) - def refresh(self): if self._user.homedir: homedir = self._user.homedir @@ -121,7 +110,6 @@ def refresh(self):
self._cUid.set_active(bool(self._user.uid)) self._cGid.set_active(bool(self._user.gid)) - self._apply_checkboxes()
self._spinUid.update() self._spinGid.update() @@ -170,6 +158,14 @@ def run(self): self.window.hide() return rc
+ def on_uid_checkbox_toggled(self, togglebutton, data=None): + # Set the UID spinner sensitivity based on the UID checkbox + self._spinUid.set_sensitive(togglebutton.get_active()) + + def on_gid_checkbox_toggled(self, togglebutton, data=None): + # Same as above, for GID + self._spinGid.set_sensitive(togglebutton.get_active()) + def on_uid_mnemonic_activate(self, widget, group_cycling, user_data=None): # If this is the only widget with the mnemonic (group_cycling is False), # and the checkbox is not currently toggled, toggle the checkbox and @@ -427,8 +423,8 @@ def usepassword_toggled(self, togglebutton=None, data=None): """Called by Gtk callback when the "Use password" check button is toggled. It will make password entries in/sensitive."""
- self.pw.set_sensitive(self.usepassword.get_active()) - self.confirm.set_sensitive(self.usepassword.get_active()) + self.pw.set_sensitive(togglebutton.get_active()) + self.confirm.set_sensitive(togglebutton.get_active())
# Re-check the password self.pw.emit("changed") @@ -467,11 +463,11 @@ def username_changed(self, editable, data=None): self.pw.emit("changed") self.confirm.emit("changed")
- def full_name_changed(self, editable=None, data=None): + def full_name_changed(self, editable, data=None): """Called by Gtk callback when the full name field changes."""
if self.guesser: - fullname = self.fullname.get_text() + fullname = editable.get_text() username = guess_username(fullname)
with blockedHandler(self.username, self.on_username_set_by_user):
From: David Shea dshea@redhat.com
Two of the conditionals around the grab_focus calls (password checkbox set but no password, full name set but username not set) are only possible via kickstart in which case the UI should be left alone anyway, and the default case is already handled by focusWidgetName. The sensitivity of the advanced button is handled by the input signal handlers and does not need to be doubled in refresh(). --- pyanaconda/ui/gui/spokes/user.py | 10 ---------- 1 file changed, 10 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/user.py b/pyanaconda/ui/gui/spokes/user.py index 5c20e3e..b8bc4e0 100644 --- a/pyanaconda/ui/gui/spokes/user.py +++ b/pyanaconda/ui/gui/spokes/user.py @@ -324,16 +324,6 @@ def refresh(self): self.pw.emit("changed") self.confirm.emit("changed")
- if self.username.get_text() and self.usepassword.get_active() and \ - self._user.password == "": - self.pw.grab_focus() - elif self.fullname.get_text(): - self.username.grab_focus() - else: - self.fullname.grab_focus() - - self.b_advanced.set_sensitive(bool(self._user.name)) - @property def status(self): if len(self.data.user.userList) == 0:
Do we support things like defining user in a kickstart file and adding password later in the UI? If so, this would break such use case.
We don't differentiate between a user command with a missing password and a user command with an empty password. So a kickstart line such as `user --name=dshea` would be treated as a user with a kickstart-provided, empty password, and when combined with `pwpolicy --changesok` the UI would show that the password is set from kickstart and would allow it to be changed.
So, I guess I don't understand what would be being broken. With the change and a kickstart like I described (username set via kickstart, password not set), the user spoke will focus on the Full Name field. Without the change, it will focus on the password field, which may or may not be correct, and more troubling it would obscure the "The password was set by kickstart" placeholder text.
As far as the second conditional, it is not possible (both in GUI and kickstart) to set the full name without setting the username, so that statement would only be true if both the username and the fullname are set. And in that case I don't understand why it would be helpful to focus one field over the other. Why not just have the spoke behave as it always would?
And the advanced button sensitivity is handled in the username_changed signal handler so the set_sensitive call there is redundant.
Sounds good to me, thanks for the extra explanation!
From: David Shea dshea@redhat.com
There's no need to validate the password twice, and it would make more sense anyway for the strength reported by the strength bar to match the strength we're using to enforce policy.
Also, one of the checks was bypassing the input delay that I went through all the trouble of adding. Checking the password on every keystroke can case UI freezes and we should avoid doing it.
Initialize the strength bar to "0" in glade so that entering the spoke does not display a medium strength before the initial check runs. --- pyanaconda/ui/gui/spokes/password.glade | 6 +--- pyanaconda/ui/gui/spokes/password.py | 52 ++++++++++++++--------------- pyanaconda/ui/gui/spokes/user.glade | 1 - pyanaconda/ui/gui/spokes/user.py | 59 +++++++++++++++------------------ 4 files changed, 53 insertions(+), 65 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/password.glade b/pyanaconda/ui/gui/spokes/password.glade index c0b38f6..d632049 100644 --- a/pyanaconda/ui/gui/spokes/password.glade +++ b/pyanaconda/ui/gui/spokes/password.glade @@ -1,5 +1,5 @@ <?xml version="1.0" encoding="UTF-8"?> -<!-- Generated with glade 3.18.3 --> +<!-- Generated with glade 3.19.0 --> <interface> <requires lib="gtk+" version="3.6"/> <requires lib="AnacondaWidgets" version="1.0"/> @@ -120,7 +120,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="valign">center</property> - <property name="value">2</property> <property name="max_value">4</property> <property name="mode">discrete</property> </object> @@ -169,9 +168,6 @@ <child> <placeholder/> </child> - <child> - <placeholder/> - </child> </object> <packing> <property name="expand">False</property> diff --git a/pyanaconda/ui/gui/spokes/password.py b/pyanaconda/ui/gui/spokes/password.py index 895a0d3..fab502e 100644 --- a/pyanaconda/ui/gui/spokes/password.py +++ b/pyanaconda/ui/gui/spokes/password.py @@ -195,22 +195,12 @@ def _checkPasswordConfirm(self, inputcheck):
return result
- def _updatePwQuality(self): + def _updatePwQuality(self, empty, strength): """Update the password quality information. - - This function is called by the ::changed signal handler on the - password field. """
- pwtext = self.pw.get_text() - - # Reset the counters used for the "press Done twice" logic - self._waiveStrengthClicks = 0 - self._waiveASCIIClicks = 0 - - self._pwq_valid, strength, self._pwq_error = validatePassword(pwtext, "root", minlen=self.policy.minlen) - - if not pwtext: + # If the password is empty, clear the strength bar + if empty: val = 0 elif strength < 50: val = 1 @@ -226,38 +216,48 @@ def _updatePwQuality(self): self.pw_label.set_text(text)
def on_password_changed(self, editable, data=None): + # Reset the counters used for the "press Done twice" logic + self._waiveStrengthClicks = 0 + self._waiveASCIIClicks = 0 + # Update the password/confirm match check on changes to the main password field self._confirm_check.update_check_status()
- self._updatePwQuality() - def _checkPasswordStrength(self, inputcheck): """Update the error message based on password strength.
- Convert the strength set by _updatePwQuality into an error message. + Update the password strength bar and set an error message. """
pw = self.pw.get_text() confirm = self.confirm.get_text()
# Skip the check if no password is required - if (not pw and not confirm) and self._kickstarted: + if self._kickstarted: + return InputCheck.CHECK_OK + + # If the password is empty, clear the strength bar and skip this check + if not pw and not confirm: + self._updatePwQuality(True, 0) return InputCheck.CHECK_OK
- # Check for validity errors - if (not self._pwq_valid) and (self._pwq_error): - return self._pwq_error + # determine the password strength + valid, pwstrength, error = validatePassword(pw, "root", minlen=self.policy.minlen) + + # set the strength bar + self._updatePwQuality(False, pwstrength)
- # use strength from policy, not bars - _valid, pwstrength, _error = validatePassword(pw, "root", minlen=self.policy.minlen) + # If the password failed the validity check, fail this check + if not valid and error: + return error
if pwstrength < self.policy.minquality: # If Done has been clicked twice, waive the check if self._waiveStrengthClicks > 1: return InputCheck.CHECK_OK elif self._waiveStrengthClicks == 1: - if self._pwq_error: - return _(PASSWORD_WEAK_CONFIRM_WITH_ERROR) % self._pwq_error + if error: + return _(PASSWORD_WEAK_CONFIRM_WITH_ERROR) % error else: return _(PASSWORD_WEAK_CONFIRM) else: @@ -267,8 +267,8 @@ def _checkPasswordStrength(self, inputcheck): else: done_msg = _(PASSWORD_DONE_TWICE)
- if self._pwq_error: - return _(PASSWORD_WEAK_WITH_ERROR) % self._pwq_error + " " + done_msg + if error: + return _(PASSWORD_WEAK_WITH_ERROR) % error + " " + done_msg else: return _(PASSWORD_WEAK) % done_msg else: diff --git a/pyanaconda/ui/gui/spokes/user.glade b/pyanaconda/ui/gui/spokes/user.glade index 8025849..80cfdc2 100644 --- a/pyanaconda/ui/gui/spokes/user.glade +++ b/pyanaconda/ui/gui/spokes/user.glade @@ -233,7 +233,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="valign">center</property> - <property name="value">2</property> <property name="max_value">4</property> <property name="mode">discrete</property> </object> diff --git a/pyanaconda/ui/gui/spokes/user.py b/pyanaconda/ui/gui/spokes/user.py index b8bc4e0..cc47c0a 100644 --- a/pyanaconda/ui/gui/spokes/user.py +++ b/pyanaconda/ui/gui/spokes/user.py @@ -247,11 +247,6 @@ def initialize(self):
self.guesser = True
- # Updated during the password changed event and used by the password - # field validity checker - self._pwq_error = None - self._pwq_valid = True - self.pw_bar = self.builder.get_object("password_bar") self.pw_label = self.builder.get_object("password_label")
@@ -381,20 +376,12 @@ def sensitive(self): def completed(self): return len(self.data.user.userList) > 0
- def _updatePwQuality(self): + def _updatePwQuality(self, empty, strength): """This method updates the password indicators according to the password entered by the user. """ - pwtext = self.pw.get_text() - username = self.username.get_text() - - # Reset the counters used for the "press Done twice" logic - self._waiveStrengthClicks = 0 - self._waiveASCIIClicks = 0 - - self._pwq_valid, strength, self._pwq_error = validatePassword(pwtext, username) - - if not pwtext: + # If the password is empty, clear the strength bar + if empty: val = 0 elif strength < 50: val = 1 @@ -404,6 +391,7 @@ def _updatePwQuality(self): val = 3 else: val = 4 + text = _(PASSWORD_STRENGTH_DESC[val])
self.pw_bar.set_value(val) @@ -422,7 +410,9 @@ def usepassword_toggled(self, togglebutton=None, data=None):
def password_changed(self, editable=None, data=None): """Update the password strength level bar""" - self._updatePwQuality() + # Reset the counters used for the "press Done twice" logic + self._waiveStrengthClicks = 0 + self._waiveASCIIClicks = 0
# Update the password/confirm match check on changes to the main password field self._confirm_check.update_check_status() @@ -508,36 +498,39 @@ def _checkPasswordConfirm(self, inputcheck): def _checkPasswordStrength(self, inputcheck): """Update the error message based on password strength.
- The password strength has already been checked in _updatePwQuality, called - previously in the signal chain. This method converts the data set from there - into an error message. - The password strength check can be waived by pressing "Done" twice. This is controlled through the self._waiveStrengthClicks counter. The counter is set in on_back_clicked, which also re-runs this check manually. """
# Skip the check if no password is required - if (not self.usepassword.get_active()) or \ - ((not self.pw.get_text()) and (self._user.password_kickstarted)): + if not self.usepassword.get_active or self._user.password_kickstarted: return InputCheck.CHECK_OK
- # If the password failed the validity check, fail this check - if (not self._pwq_valid) and (self._pwq_error): - return self._pwq_error - - # use strength from policy, not bars + # If the password is empty, clear the strength bar and skip this check pw = self.pw.get_text() + if not pw: + self._updatePwQuality(True, 0) + return InputCheck.CHECK_OK + + # determine the password strength username = self.username.get_text() - _valid, pwstrength, _error = validatePassword(pw, username, minlen=self.policy.minlen) + valid, pwstrength, error = validatePassword(pw, username, minlen=self.policy.minlen) + + # set the strength bar + self._updatePwQuality(False, pwstrength) + + # If the password failed the validity check, fail this check + if not valid and error: + return error
if pwstrength < self.policy.minquality: # If Done has been clicked twice, waive the check if self._waiveStrengthClicks > 1: return InputCheck.CHECK_OK elif self._waiveStrengthClicks == 1: - if self._pwq_error: - return _(PASSWORD_WEAK_CONFIRM_WITH_ERROR) % self._pwq_error + if error: + return _(PASSWORD_WEAK_CONFIRM_WITH_ERROR) % error else: return _(PASSWORD_WEAK_CONFIRM) else: @@ -547,8 +540,8 @@ def _checkPasswordStrength(self, inputcheck): else: done_msg = _(PASSWORD_DONE_TWICE)
- if self._pwq_error: - return _(PASSWORD_WEAK_WITH_ERROR) % self._pwq_error + " " + done_msg + if error: + return _(PASSWORD_WEAK_WITH_ERROR) % error + " " + done_msg else: return _(PASSWORD_WEAK) % done_msg else:
From: David Shea dshea@redhat.com
You know what else can hold state for the spoke? The spoke. --- pyanaconda/ui/gui/spokes/user.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/user.py b/pyanaconda/ui/gui/spokes/user.py index cc47c0a..2d09385 100644 --- a/pyanaconda/ui/gui/spokes/user.py +++ b/pyanaconda/ui/gui/spokes/user.py @@ -261,7 +261,7 @@ def initialize(self): self.policy = self.data.anaconda.PwPolicyData()
# indicate when the password was set by kickstart - self._user.password_kickstarted = self.data.user.seen + self._password_kickstarted = self.data.user.seen
# Password checks, in order of importance: # - if a password is required, is one specified? @@ -292,7 +292,7 @@ def initialize(self): # Modify the GUI based on the kickstart and policy information # This needs to happen after the input checks have been created, since # the Gtk signal handlers use the input check variables. - if self._user.password_kickstarted: + if self._password_kickstarted: self.usepassword.set_active(True) self.pw.set_placeholder_text(_("The password was set by kickstart.")) self.confirm.set_placeholder_text(_("The password was set by kickstart.")) @@ -340,7 +340,7 @@ def apply(self): # this should preserve the kickstart based password if self.usepassword.get_active(): if self.pw.get_text(): - self._user.password_kickstarted = False + self._password_kickstarted = False self._user.password = cryptPassword(self.pw.get_text()) self._user.isCrypted = True self.pw.set_placeholder_text("") @@ -352,7 +352,7 @@ def apply(self): self.confirm.set_placeholder_text("") self._user.password = "" self._user.isCrypted = False - self._user.password_kickstarted = False + self._password_kickstarted = False
self._user.name = self.username.get_text() self._user.gecos = self.fullname.get_text() @@ -468,11 +468,11 @@ def _checkPasswordEmpty(self, inputcheck): """
# If the password was set by kickstart, skip the strength check - if self._user.password_kickstarted and not self.policy.changesok: + if self._password_kickstarted and not self.policy.changesok: return InputCheck.CHECK_OK
# Skip the check if no password is required - if (not self.usepassword.get_active()) or self._user.password_kickstarted: + if (not self.usepassword.get_active()) or self._password_kickstarted: return InputCheck.CHECK_OK elif not self.get_input(inputcheck.input_obj): if inputcheck.input_obj == self.pw: @@ -486,7 +486,7 @@ def _checkPasswordConfirm(self, inputcheck): """If the user has entered confirmation data, check whether it matches the password."""
# Skip the check if no password is required - if (not self.usepassword.get_active()) or self._user.password_kickstarted: + if (not self.usepassword.get_active()) or self._password_kickstarted: result = InputCheck.CHECK_OK elif self.confirm.get_text() and (self.pw.get_text() != self.confirm.get_text()): result = _(PASSWORD_CONFIRM_ERROR_GUI) @@ -504,7 +504,7 @@ def _checkPasswordStrength(self, inputcheck): """
# Skip the check if no password is required - if not self.usepassword.get_active or self._user.password_kickstarted: + if not self.usepassword.get_active or self._password_kickstarted: return InputCheck.CHECK_OK
# If the password is empty, clear the strength bar and skip this check
From: David Shea dshea@redhat.com
Do not overwrite home directory values from kickstart unless the user actually changed the value. Revert to the default home directory if the home directory input is cleared instead of setting the home directory to "/". --- pyanaconda/ui/gui/spokes/user.py | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/user.py b/pyanaconda/ui/gui/spokes/user.py index 2d09385..c68c7b4 100644 --- a/pyanaconda/ui/gui/spokes/user.py +++ b/pyanaconda/ui/gui/spokes/user.py @@ -76,9 +76,15 @@ def __init__(self, user, data): self._user = user
# Track whether the user has requested a home directory other - # than the default. + # than the default. This way, if the home directory is left as + # the default, the default will change if the username changes. + # Otherwise, once the directory is set it stays that way. self._origHome = None - self._homeSet = False + + if self._user.homedir: + self._homeSet = True + else: + self._homeSet = False
def _grabObjects(self): self._cUid = self.builder.get_object("c_uid") @@ -125,16 +131,27 @@ def run(self): if rc == 1: # Input checks pass if self.on_ok_clicked(): - # If the user changed the home directory input, either this time or - # during any earlier run of the dialog, set homedir to the value - # in the input box. homedir = self._tHome.get_text() - if not os.path.isabs(homedir): - homedir = "/" + homedir - if self._homeSet or self._origHome != homedir: + + # If the user cleared the home directory, revert back to the + # default + if not homedir: + self._homeSet = False + self._user.homedir = None + # If the user modified the home directory input, save that the + # home directory has been modified and use the value. + elif self._origHome != homedir: self._homeSet = True + + if not os.path.isabs(homedir): + homedir = "/" + homedir self._user.homedir = homedir
+ # Otherwise leave the home directory alone. If the home + # directory is currently the default value, the next call + # to refresh() will update the input text to reflect + # changes in the username. + if self._cUid.get_active(): self._user.uid = int(self._uid.get_value()) else:
From: David Shea dshea@redhat.com
Move all of the data copying into an apply method instead of doing it three indents deep in run(). --- pyanaconda/ui/gui/spokes/user.py | 68 +++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 32 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/user.py b/pyanaconda/ui/gui/spokes/user.py index c68c7b4..b169e85 100644 --- a/pyanaconda/ui/gui/spokes/user.py +++ b/pyanaconda/ui/gui/spokes/user.py @@ -122,6 +122,41 @@ def refresh(self):
self._tGroups.set_text(", ".join(self._user.groups))
+ def apply(self): + # Copy data from the UI back to the kickstart object + homedir = self._tHome.get_text() + + # If the user cleared the home directory, revert back to the + # default + if not homedir: + self._homeSet = False + self._user.homedir = None + # If the user modified the home directory input, save that the + # home directory has been modified and use the value. + elif self._origHome != homedir: + self._homeSet = True + + if not os.path.isabs(homedir): + homedir = "/" + homedir + self._user.homedir = homedir + + # Otherwise leave the home directory alone. If the home + # directory is currently the default value, the next call + # to refresh() will update the input text to reflect + # changes in the username. + + if self._cUid.get_active(): + self._user.uid = int(self._uid.get_value()) + else: + self._user.uid = None + + if self._cGid.get_active(): + self._user.gid = int(self._gid.get_value()) + else: + self._user.gid = None + + self._user.groups = [g.strip() for g in self._tGroups.get_text().split(",")] + def run(self): self.window.show() while True: @@ -131,38 +166,7 @@ def run(self): if rc == 1: # Input checks pass if self.on_ok_clicked(): - homedir = self._tHome.get_text() - - # If the user cleared the home directory, revert back to the - # default - if not homedir: - self._homeSet = False - self._user.homedir = None - # If the user modified the home directory input, save that the - # home directory has been modified and use the value. - elif self._origHome != homedir: - self._homeSet = True - - if not os.path.isabs(homedir): - homedir = "/" + homedir - self._user.homedir = homedir - - # Otherwise leave the home directory alone. If the home - # directory is currently the default value, the next call - # to refresh() will update the input text to reflect - # changes in the username. - - if self._cUid.get_active(): - self._user.uid = int(self._uid.get_value()) - else: - self._user.uid = None - - if self._cGid.get_active(): - self._user.gid = int(self._gid.get_value()) - else: - self._user.gid = None - - self._user.groups = [g.strip() for g in self._tGroups.get_text().split(",")] + self.apply() break # Input checks fail, try again else:
From: David Shea dshea@redhat.com
The UserData object is handy for sharing data across the two windows used by the user spoke, but ksdata should not be modified until Done is clicked. --- pyanaconda/ui/gui/spokes/user.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/user.py b/pyanaconda/ui/gui/spokes/user.py index b169e85..3b8bf89 100644 --- a/pyanaconda/ui/gui/spokes/user.py +++ b/pyanaconda/ui/gui/spokes/user.py @@ -22,6 +22,7 @@
import re import os +import copy from pyanaconda.flags import flags from pyanaconda.i18n import _, CN_ from pyanaconda.users import cryptPassword, validatePassword, guess_username @@ -248,8 +249,10 @@ def __init__(self, *args): def initialize(self): NormalSpoke.initialize(self)
+ # Create a new UserData object to store this spoke's state + # as well as the state of the advanced user dialog. if self.data.user.userList: - self._user = self.data.user.userList[0] + self._user = copy.copy(self.data.user.userList[0]) else: self._user = self.data.UserData()
@@ -378,13 +381,17 @@ def apply(self): self._user.name = self.username.get_text() self._user.gecos = self.fullname.get_text()
- # the user will be created only if the username is set + # Copy the spoke data back to kickstart + # If the user name is not set, no user will be created. if self._user.name: - if self._user not in self.data.user.userList: - self.data.user.userList.append(self._user) + ksuser = copy.copy(self._user)
- elif self._user in self.data.user.userList: - self.data.user.userList.remove(self._user) + if not self.data.user.userList: + self.data.user.userList.append(ksuser) + else: + self.data.user.userList[0] = ksuser + elif self.data.user.userList: + self.data.user.userList.pop(0)
@property def sensitive(self):
From: David Shea dshea@redhat.com
--- pyanaconda/ui/gui/spokes/user.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/user.py b/pyanaconda/ui/gui/spokes/user.py index 3b8bf89..735293f 100644 --- a/pyanaconda/ui/gui/spokes/user.py +++ b/pyanaconda/ui/gui/spokes/user.py @@ -71,8 +71,8 @@ def _validateGroups(self, inputcheck): def __init__(self, user, data): GUIObject.__init__(self, data)
- self._saveButton = self.builder.get_object("save_button") - GUIDialogInputCheckHandler.__init__(self, self._saveButton) + _saveButton = self.builder.get_object("save_button") + GUIDialogInputCheckHandler.__init__(self, _saveButton)
self._user = user
@@ -244,7 +244,6 @@ def should_run(cls, environment, data): def __init__(self, *args): NormalSpoke.__init__(self, *args) GUISpokeInputCheckHandler.__init__(self) - self._oldweak = None
def initialize(self): NormalSpoke.initialize(self)
In reply to line 75 of pyanaconda/ui/gui/spokes/user.py:
I think one more character should be removed here. There's no reason for the name to start with an underscore.
In reply to line 75 of pyanaconda/ui/gui/spokes/user.py:
Will do.
Other than the question plus a nitpick above these all look good to me. Good job, @dashea!
Closed.
Removed that underscore and pushed.
anaconda-patches@lists.fedorahosted.org