Just some clean-up of the text UI that needed to get done: First patch cleans up input handling, which had some inconsistencies and other sloppiness. Most of this is just moving code around, renaming things, and deleting unneeded bits. Second patch just gets rid of an unneeded property in the storage spoke.
This is just some clean-up that needed to get done: -- removes some unnecessary error catching -- fixes up some typos -- fixes some inconsistency with variable naming --- pyanaconda/ui/tui/spokes/askvnc.py | 12 +++-- pyanaconda/ui/tui/spokes/software.py | 19 ++++---- pyanaconda/ui/tui/spokes/source.py | 77 +++++++++++++++--------------- pyanaconda/ui/tui/spokes/storage.py | 87 +++++++++++++++++----------------- pyanaconda/ui/tui/spokes/time_spoke.py | 52 ++++++++++---------- 5 files changed, 125 insertions(+), 122 deletions(-)
diff --git a/pyanaconda/ui/tui/spokes/askvnc.py b/pyanaconda/ui/tui/spokes/askvnc.py index 9847874..8d04f61 100644 --- a/pyanaconda/ui/tui/spokes/askvnc.py +++ b/pyanaconda/ui/tui/spokes/askvnc.py @@ -68,11 +68,15 @@ class AskVNCSpoke(NormalTUISpoke): """Override input so that we can launch the VNC password spoke"""
try: - number = int(key) - choice = self._choices[number -1] - except (ValueError, KeyError, IndexError): + keyid = int(key) - 1 + except ValueError: return key
+ if keyid in range(0, len(self._choices)): + choice = self._choices[keyid] + else: + return None + if choice == USETEXT: self._usevnc = False else: @@ -83,7 +87,7 @@ class AskVNCSpoke(NormalTUISpoke):
self.apply() self.close() - return None + return True
def apply(self): self.data.vnc.enabled = self._usevnc diff --git a/pyanaconda/ui/tui/spokes/software.py b/pyanaconda/ui/tui/spokes/software.py index fc0298e..94c680b 100644 --- a/pyanaconda/ui/tui/spokes/software.py +++ b/pyanaconda/ui/tui/spokes/software.py @@ -140,16 +140,19 @@ class SoftwareSpoke(NormalTUISpoke):
def input(self, args, key): """ Handle the input; this chooses the desktop environment. """ - if key.lower() == "c" and self._selection in range(len(self.payload.environments)): - self.apply() - try: keyid = int(key) - 1 - if keyid in range(len(self.payload.environments)): - self._selection = keyid - return None - except (ValueError, IndexError): - return key + except ValueError: + if key.lower() == "c" and self._selection in range(len(self.payload.environments)): + self.apply() + self.close() + return True + else: + return key + + if keyid in range(len(self.payload.environments)): + self._selection = keyid + return None
@property def ready(self): diff --git a/pyanaconda/ui/tui/spokes/source.py b/pyanaconda/ui/tui/spokes/source.py index 776d179..48c32bd 100644 --- a/pyanaconda/ui/tui/spokes/source.py +++ b/pyanaconda/ui/tui/spokes/source.py @@ -143,46 +143,47 @@ class SourceSpoke(EditTUISpoke): """ Handle the input; this decides the repo protocol. """ try: num = int(key) - if args == 2: - # network install - self._selection = num - if self._selection == 1: - # closest mirror - self.data.method.method = None - self.apply() - self.close() - return True - elif self._selection in range(2, 5): - self.data.method.method = "url" - newspoke = SpecifyRepoSpoke(self.app, self.data, self.storage, - self.payload, self.instclass, self._selection) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return True - elif self._selection == 5: - # nfs - self.data.method.method = "nfs" - newspoke = SpecifyNFSRepoSpoke(self.app, self.data, self.storage, - self.payload, self.instclass, self._selection, self.errors) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return True - else: - if num == 1: - # iso selected, just set some vars and return to main hub - self.data.method.method = "cdrom" - self.payload.install_device = self._cdrom - self.apply() - self.close() - return True - else: - self.app.switch_screen(self, num) - return None - except (ValueError, IndexError): + except ValueError: return key
+ if args == 2: + # network install + self._selection = num + if self._selection == 1: + # closest mirror + self.data.method.method = None + self.apply() + self.close() + return True + elif self._selection in range(2, 5): + self.data.method.method = "url" + newspoke = SpecifyRepoSpoke(self.app, self.data, self.storage, + self.payload, self.instclass, self._selection) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return True + elif self._selection == 5: + # nfs + self.data.method.method = "nfs" + newspoke = SpecifyNFSRepoSpoke(self.app, self.data, self.storage, + self.payload, self.instclass, self._selection, self.errors) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return True + else: + if num == 1: + # iso selected, just set some vars and return to main hub + self.data.method.method = "cdrom" + self.payload.install_device = self._cdrom + self.apply() + self.close() + return True + else: + self.app.switch_screen(self, num) + return None + def getRepoMetadata(self): """ Pull down yum repo metadata """ try: diff --git a/pyanaconda/ui/tui/spokes/storage.py b/pyanaconda/ui/tui/spokes/storage.py index 51ce0ab..92b5bf2 100644 --- a/pyanaconda/ui/tui/spokes/storage.py +++ b/pyanaconda/ui/tui/spokes/storage.py @@ -197,23 +197,22 @@ class StorageSpoke(NormalTUISpoke): def input(self, args, key): """Grab the disk choice and update things"""
- if key == "c": - if self.selected_disks: - newspoke = AutoPartSpoke(self.app, self.data, self.storage, - self.payload, self.instclass) - self.app.switch_screen_modal(newspoke) - self.apply() - self.execute() - self.close() - return None - try: - number = int(key) - self._update_disk_list(self.disks[number -1]) + keyid = int(key) - 1 + self._update_disk_list(self.disks[keyid]) return None - - except (ValueError, KeyError, IndexError): - return key + except (ValueError, IndexError): + if key.lower() == "c": + if self.selected_disks: + newspoke = AutoPartSpoke(self.app, self.data, self.storage, + self.payload, self.instclass) + self.app.switch_screen_modal(newspoke) + self.apply() + self.execute() + self.close() + return None + else: + return key
def apply(self): if not flags.automatedInstall: @@ -316,8 +315,8 @@ class AutoPartSpoke(NormalTUISpoke): return True
@property - def completed(self): - return True # We're always complete + def completed(self): + return True # We're always complete
def refresh(self, args = None): NormalTUISpoke.refresh(self, args) @@ -347,26 +346,25 @@ class AutoPartSpoke(NormalTUISpoke): def input(self, args, key): """Grab the choice and update things"""
- if key == "c": - newspoke = PartitionSchemeSpoke(self.app, self.data, self.storage, - self.payload, self.instclass) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return False - try: - number = int(key) - self.clearPartType = PARTTYPES[self.parttypelist[number -1]] - self.apply() - self.close() - return False + keyid = int(key) - 1 + except ValueError: + if key.lower() == "c": + newspoke = PartitionSchemeSpoke(self.app, self.data, self.storage, + self.payload, self.instclass) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return True + else: + return key
- except (ValueError, KeyError, IndexError): - return key + if keyid in range(1, len(self.parttypelist)): + self.clearPartType = PARTTYPES[self.parttypelist[keyid]] + return None
class PartitionSchemeSpoke(NormalTUISpoke): - """ SPoke to select what partitioning scheme to use on disk(s). """ + """ Spoke to select what partitioning scheme to use on disk(s). """ title = _("Partition Scheme Options") category = "destination"
@@ -399,18 +397,19 @@ class PartitionSchemeSpoke(NormalTUISpoke): def input(self, args, key): """ Grab the choice and update things. """
- if key == "c" and self._selection: - self.apply() - self.close() - return None - try: - keyid = int(key) - if keyid in range(0, 4): - self._selection = keyid - return None - except (ValueError, IndexError, KeyError): - return key + keyid = int(key) - 1 + except ValueError: + if key.lower() == "c" and self._selection: + self.apply() + self.close() + return True + else: + return key + + if keyid in range(0, len(self.partschemes)): + self._selection = keyid + return None
def apply(self): """ Apply our selections. """ diff --git a/pyanaconda/ui/tui/spokes/time_spoke.py b/pyanaconda/ui/tui/spokes/time_spoke.py index 7e3467b..24d5863 100644 --- a/pyanaconda/ui/tui/spokes/time_spoke.py +++ b/pyanaconda/ui/tui/spokes/time_spoke.py @@ -87,47 +87,43 @@ class TimeZoneSpoke(FirstbootSpokeMixIn, NormalTUISpoke): def input(self, args, key): try: keyid = int(key) - 1 - if args: - self._selection = "%s/%s" % (args, self._timezones[args][keyid]) + except ValueError: + if key.lower().replace("_", " ") in self._lower_zones: + index = self._lower_zones.index(key.lower().replace("_", " ")) + self._selection = self._zones[index] self.apply() self.close() - else: - if len(self._timezones[self._regions[keyid]]) == 1: - self._selection = "%s/%s" % (self._regions[keyid], - self._timezones[self._regions[keyid]][0]) + return True + elif key.lower() in self._lower_regions: + index = self._lower_regions.index(key.lower()) + if len(self._timezones[self._regions[index]]) == 1: + self._selection = "%s/%s" % (self._regions[index], + self._timezones[self._regions[index]][0]) self.apply() self.close() else: - self.app.switch_screen(self, self._regions[keyid]) - return True - except (ValueError, IndexError): - pass + self.app.switch_screen(self, self._regions[id]) + return True + elif key.lower() == "b": + self.app.switch_screen(self, None) + return True + else: + return key
- if key.lower().replace("_", " ") in self._lower_zones: - index = self._lower_zones.index(key.lower().replace("_", " ")) - self._selection = self._zones[index] + if args: + self._selection = "%s/%s" % (args, self._timezones[args][keyid]) self.apply() self.close() - return True - - elif key.lower() in self._lower_regions: - index = self._lower_regions.index(key.lower()) - if len(self._timezones[self._regions[index]]) == 1: - self._selection = "%s/%s" % (self._regions[index], - self._timezones[self._regions[index]][0]) + else: + if len(self._timezones[self._regions[keyid]]) == 1: + self._selection = "%s/%s" % (self._regions[keyid], + self._timezones[self._regions[keyid]][0]) self.apply() self.close() else: - self.app.switch_screen(self, self._regions[id]) - return True - - elif key.lower() == "b": - self.app.switch_screen(self, None) + self.app.switch_screen(self, self._regions[keyid]) return True
- else: - return key - def prompt(self, args = None): return _("Please select the timezone.\nUse numbers or type names directly [b to region list, q to quit]: ")
On Tue, Sep 03, 2013 at 08:31:58AM -0400, Samantha N. Bueno wrote:
if keyid in range(0, len(self._choices)):choice = self._choices[keyid]
Why use range here instead of if keyid < len(...
The same question applies to a number of places in this patch.
if keyid in range(1, len(self.parttypelist)):self.clearPartType = PARTTYPES[self.parttypelist[keyid]]
In this case you could use
if 0 < keyid < len(...):
I don't see any reason to use range for all of these comparisons, it seems wasteful to me, creating a list each time you want to compare.
Based on bcl/vpodzime's suggestions.
This is just some clean-up that needed to get done: -- removes some unnecessary error catching -- fixes up some typos -- fixes some inconsistency with variable naming --- pyanaconda/ui/tui/spokes/askvnc.py | 12 +++-- pyanaconda/ui/tui/spokes/software.py | 19 ++++---- pyanaconda/ui/tui/spokes/source.py | 77 ++++++++++++++--------------- pyanaconda/ui/tui/spokes/storage.py | 88 +++++++++++++++++----------------- pyanaconda/ui/tui/spokes/time_spoke.py | 52 ++++++++++---------- 5 files changed, 126 insertions(+), 122 deletions(-)
diff --git a/pyanaconda/ui/tui/spokes/askvnc.py b/pyanaconda/ui/tui/spokes/askvnc.py index 9847874..36637bf 100644 --- a/pyanaconda/ui/tui/spokes/askvnc.py +++ b/pyanaconda/ui/tui/spokes/askvnc.py @@ -68,11 +68,15 @@ class AskVNCSpoke(NormalTUISpoke): """Override input so that we can launch the VNC password spoke"""
try: - number = int(key) - choice = self._choices[number -1] - except (ValueError, KeyError, IndexError): + keyid = int(key) - 1 + except ValueError: return key
+ if 0 <= keyid < len(self._choices): + choice = self._choices[keyid] + else: + return None + if choice == USETEXT: self._usevnc = False else: @@ -83,7 +87,7 @@ class AskVNCSpoke(NormalTUISpoke):
self.apply() self.close() - return None + return True
def apply(self): self.data.vnc.enabled = self._usevnc diff --git a/pyanaconda/ui/tui/spokes/software.py b/pyanaconda/ui/tui/spokes/software.py index fc0298e..2462c4d 100644 --- a/pyanaconda/ui/tui/spokes/software.py +++ b/pyanaconda/ui/tui/spokes/software.py @@ -140,16 +140,19 @@ class SoftwareSpoke(NormalTUISpoke):
def input(self, args, key): """ Handle the input; this chooses the desktop environment. """ - if key.lower() == "c" and self._selection in range(len(self.payload.environments)): - self.apply() - try: keyid = int(key) - 1 - if keyid in range(len(self.payload.environments)): - self._selection = keyid - return None - except (ValueError, IndexError): - return key + except ValueError: + if key.lower() == "c" and 0 <= self._selection < len(self.payload.environments): + self.apply() + self.close() + return True + else: + return key + + if 0 <= keyid < len(self.payload.environments): + self._selection = keyid + return None
@property def ready(self): diff --git a/pyanaconda/ui/tui/spokes/source.py b/pyanaconda/ui/tui/spokes/source.py index 776d179..48c32bd 100644 --- a/pyanaconda/ui/tui/spokes/source.py +++ b/pyanaconda/ui/tui/spokes/source.py @@ -143,46 +143,47 @@ class SourceSpoke(EditTUISpoke): """ Handle the input; this decides the repo protocol. """ try: num = int(key) - if args == 2: - # network install - self._selection = num - if self._selection == 1: - # closest mirror - self.data.method.method = None - self.apply() - self.close() - return True - elif self._selection in range(2, 5): - self.data.method.method = "url" - newspoke = SpecifyRepoSpoke(self.app, self.data, self.storage, - self.payload, self.instclass, self._selection) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return True - elif self._selection == 5: - # nfs - self.data.method.method = "nfs" - newspoke = SpecifyNFSRepoSpoke(self.app, self.data, self.storage, - self.payload, self.instclass, self._selection, self.errors) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return True - else: - if num == 1: - # iso selected, just set some vars and return to main hub - self.data.method.method = "cdrom" - self.payload.install_device = self._cdrom - self.apply() - self.close() - return True - else: - self.app.switch_screen(self, num) - return None - except (ValueError, IndexError): + except ValueError: return key
+ if args == 2: + # network install + self._selection = num + if self._selection == 1: + # closest mirror + self.data.method.method = None + self.apply() + self.close() + return True + elif self._selection in range(2, 5): + self.data.method.method = "url" + newspoke = SpecifyRepoSpoke(self.app, self.data, self.storage, + self.payload, self.instclass, self._selection) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return True + elif self._selection == 5: + # nfs + self.data.method.method = "nfs" + newspoke = SpecifyNFSRepoSpoke(self.app, self.data, self.storage, + self.payload, self.instclass, self._selection, self.errors) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return True + else: + if num == 1: + # iso selected, just set some vars and return to main hub + self.data.method.method = "cdrom" + self.payload.install_device = self._cdrom + self.apply() + self.close() + return True + else: + self.app.switch_screen(self, num) + return None + def getRepoMetadata(self): """ Pull down yum repo metadata """ try: diff --git a/pyanaconda/ui/tui/spokes/storage.py b/pyanaconda/ui/tui/spokes/storage.py index 51ce0ab..c12cfff 100644 --- a/pyanaconda/ui/tui/spokes/storage.py +++ b/pyanaconda/ui/tui/spokes/storage.py @@ -197,23 +197,22 @@ class StorageSpoke(NormalTUISpoke): def input(self, args, key): """Grab the disk choice and update things"""
- if key == "c": - if self.selected_disks: - newspoke = AutoPartSpoke(self.app, self.data, self.storage, - self.payload, self.instclass) - self.app.switch_screen_modal(newspoke) - self.apply() - self.execute() - self.close() - return None - try: - number = int(key) - self._update_disk_list(self.disks[number -1]) + keyid = int(key) - 1 + self._update_disk_list(self.disks[keyid]) return None - - except (ValueError, KeyError, IndexError): - return key + except (ValueError, IndexError): + if key.lower() == "c": + if self.selected_disks: + newspoke = AutoPartSpoke(self.app, self.data, self.storage, + self.payload, self.instclass) + self.app.switch_screen_modal(newspoke) + self.apply() + self.execute() + self.close() + return None + else: + return key
def apply(self): if not flags.automatedInstall: @@ -347,31 +346,31 @@ class AutoPartSpoke(NormalTUISpoke): def input(self, args, key): """Grab the choice and update things"""
- if key == "c": - newspoke = PartitionSchemeSpoke(self.app, self.data, self.storage, - self.payload, self.instclass) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return False - try: - number = int(key) - self.clearPartType = PARTTYPES[self.parttypelist[number -1]] - self.apply() - self.close() - return False + keyid = int(key) - 1 + except ValueError: + if key.lower() == "c": + newspoke = PartitionSchemeSpoke(self.app, self.data, self.storage, + self.payload, self.instclass) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return True + else: + return key
- except (ValueError, KeyError, IndexError): - return key + if 0 <= keyid < len(self.parttypelist): + self.clearPartType = PARTTYPES[self.parttypelist[keyid]] + self.apply() + return None
class PartitionSchemeSpoke(NormalTUISpoke): - """ SPoke to select what partitioning scheme to use on disk(s). """ + """ Spoke to select what partitioning scheme to use on disk(s). """ title = _("Partition Scheme Options") category = "destination"
# set default FS to LVM, for consistency with graphical behavior - _selection = 2 + _selection = 1
def __init__(self, app, data, storage, payload, instclass): NormalTUISpoke.__init__(self, app, data, storage, payload, instclass) @@ -389,7 +388,7 @@ class PartitionSchemeSpoke(NormalTUISpoke): for sch in schemelist: box = CheckboxWidget(title="%i) %s" %(schemelist.index(sch) \ + 1, sch), completed=(schemelist.index(sch) \ - + 1 == self._selection)) + == self._selection)) self._window += [box, ""]
message = _("Select a partition scheme configuration.") @@ -399,18 +398,19 @@ class PartitionSchemeSpoke(NormalTUISpoke): def input(self, args, key): """ Grab the choice and update things. """
- if key == "c" and self._selection: - self.apply() - self.close() - return None - try: - keyid = int(key) - if keyid in range(0, 4): - self._selection = keyid - return None - except (ValueError, IndexError, KeyError): - return key + keyid = int(key) - 1 + except ValueError: + if key.lower() == "c" and self._selection: + self.apply() + self.close() + return True + else: + return key + + if 0 <= keyid < len(self.partschemes): + self._selection = keyid + return None
def apply(self): """ Apply our selections. """ diff --git a/pyanaconda/ui/tui/spokes/time_spoke.py b/pyanaconda/ui/tui/spokes/time_spoke.py index 7e3467b..24d5863 100644 --- a/pyanaconda/ui/tui/spokes/time_spoke.py +++ b/pyanaconda/ui/tui/spokes/time_spoke.py @@ -87,47 +87,43 @@ class TimeZoneSpoke(FirstbootSpokeMixIn, NormalTUISpoke): def input(self, args, key): try: keyid = int(key) - 1 - if args: - self._selection = "%s/%s" % (args, self._timezones[args][keyid]) + except ValueError: + if key.lower().replace("_", " ") in self._lower_zones: + index = self._lower_zones.index(key.lower().replace("_", " ")) + self._selection = self._zones[index] self.apply() self.close() - else: - if len(self._timezones[self._regions[keyid]]) == 1: - self._selection = "%s/%s" % (self._regions[keyid], - self._timezones[self._regions[keyid]][0]) + return True + elif key.lower() in self._lower_regions: + index = self._lower_regions.index(key.lower()) + if len(self._timezones[self._regions[index]]) == 1: + self._selection = "%s/%s" % (self._regions[index], + self._timezones[self._regions[index]][0]) self.apply() self.close() else: - self.app.switch_screen(self, self._regions[keyid]) - return True - except (ValueError, IndexError): - pass + self.app.switch_screen(self, self._regions[id]) + return True + elif key.lower() == "b": + self.app.switch_screen(self, None) + return True + else: + return key
- if key.lower().replace("_", " ") in self._lower_zones: - index = self._lower_zones.index(key.lower().replace("_", " ")) - self._selection = self._zones[index] + if args: + self._selection = "%s/%s" % (args, self._timezones[args][keyid]) self.apply() self.close() - return True - - elif key.lower() in self._lower_regions: - index = self._lower_regions.index(key.lower()) - if len(self._timezones[self._regions[index]]) == 1: - self._selection = "%s/%s" % (self._regions[index], - self._timezones[self._regions[index]][0]) + else: + if len(self._timezones[self._regions[keyid]]) == 1: + self._selection = "%s/%s" % (self._regions[keyid], + self._timezones[self._regions[keyid]][0]) self.apply() self.close() else: - self.app.switch_screen(self, self._regions[id]) - return True - - elif key.lower() == "b": - self.app.switch_screen(self, None) + self.app.switch_screen(self, self._regions[keyid]) return True
- else: - return key - def prompt(self, args = None): return _("Please select the timezone.\nUse numbers or type names directly [b to region list, q to quit]: ")
On Wed, Sep 04, 2013 at 12:41:29PM -0400, Samantha N. Bueno wrote:
except ValueError:if key.lower() == "c" and 0 <= self._selection < len(self.payload.environments):self.apply()self.close()return Trueelse:return key
Should probably add a docstring with info about the possible return values. True, key, None? T/F, key?
if 0 <= keyid < len(self.payload.environments):self._selection = keyidreturn None
Should this return False instead of None?
else:if num == 1:# iso selected, just set some vars and return to main hubself.data.method.method = "cdrom"self.payload.install_device = self._cdromself.apply()self.close()return Trueelse:self.app.switch_screen(self, num)return None
Same question here, and in the rest of the patch.
On Wed, 2013-09-04 at 14:41 -0700, Brian C. Lane wrote:
On Wed, Sep 04, 2013 at 12:41:29PM -0400, Samantha N. Bueno wrote:
except ValueError:if key.lower() == "c" and 0 <= self._selection < len(self.payload.environments):self.apply()self.close()return Trueelse:return keyShould probably add a docstring with info about the possible return values. True, key, None? T/F, key?
This is described in the pyanaconda/ui/tui/simpleline/base.py:UIObject.input method's docstring, but I agree it's so complicated that it should be either commented everywhere or replaced with something clearer. Maybe we could use some constants for True/None and False like INPUT_PROCESSED and INPUT_DISCARDED. Returning key is quite clear, I think.
On Thu, Sep 05, 2013 at 08:38:58AM +0200, Vratislav Podzimek wrote:
On Wed, 2013-09-04 at 14:41 -0700, Brian C. Lane wrote:
On Wed, Sep 04, 2013 at 12:41:29PM -0400, Samantha N. Bueno wrote:
except ValueError:if key.lower() == "c" and 0 <= self._selection < len(self.payload.environments):self.apply()self.close()return Trueelse:return keyShould probably add a docstring with info about the possible return values. True, key, None? T/F, key?
This is described in the pyanaconda/ui/tui/simpleline/base.py:UIObject.input method's docstring, but I agree it's so complicated that it should be either commented everywhere or replaced with something clearer. Maybe we could use some constants for True/None and False like INPUT_PROCESSED and INPUT_DISCARDED. Returning key is quite clear, I think.
I like that idea; and using some clearly identifiable constants rather than T/F and adding docstrings everywhere this occurs is a bit cleaner (since this occurs in nearly every spoke). I'll post a new patch in just a bit with those edits.
Samantha
This is just some clean-up that needed to get done: -- removes some unnecessary error catching -- fixes up some typos -- fixes some inconsistency with variable naming -- also adds a couple constants to make the end result of input processing in the spokes less confusing --- pyanaconda/constants_text.py | 4 ++ pyanaconda/ui/tui/spokes/askvnc.py | 13 +++-- pyanaconda/ui/tui/spokes/network.py | 7 +-- pyanaconda/ui/tui/spokes/software.py | 20 +++++--- pyanaconda/ui/tui/spokes/source.py | 78 +++++++++++++++-------------- pyanaconda/ui/tui/spokes/storage.py | 91 +++++++++++++++++----------------- pyanaconda/ui/tui/spokes/time_spoke.py | 57 ++++++++++----------- 7 files changed, 142 insertions(+), 128 deletions(-)
diff --git a/pyanaconda/constants_text.py b/pyanaconda/constants_text.py index b557229..08a8046 100644 --- a/pyanaconda/constants_text.py +++ b/pyanaconda/constants_text.py @@ -52,3 +52,7 @@ TEXT_YES_BUTTON = Translator(TEXT_YES_STR, TEXT_YES_CHECK) TEXT_NO_STR = N_("No") TEXT_NO_CHECK = "no" TEXT_NO_BUTTON = Translator(TEXT_NO_STR, TEXT_NO_CHECK) + +# Make the return calls from the UIScreen input() function more clear +INPUT_PROCESSED = None +INPUT_DISCARDED = False diff --git a/pyanaconda/ui/tui/spokes/askvnc.py b/pyanaconda/ui/tui/spokes/askvnc.py index 9847874..f6e9528 100644 --- a/pyanaconda/ui/tui/spokes/askvnc.py +++ b/pyanaconda/ui/tui/spokes/askvnc.py @@ -22,6 +22,7 @@ from pyanaconda.ui.tui.spokes import NormalTUISpoke from pyanaconda.ui.tui.simpleline import TextWidget, ColumnWidget from pyanaconda.constants import USEVNC, USETEXT +from pyanaconda.constants_text import INPUT_PROCESSED from pyanaconda.i18n import _ import getpass
@@ -68,11 +69,15 @@ class AskVNCSpoke(NormalTUISpoke): """Override input so that we can launch the VNC password spoke"""
try: - number = int(key) - choice = self._choices[number -1] - except (ValueError, KeyError, IndexError): + keyid = int(key) - 1 + except ValueError: return key
+ if 0 <= keyid < len(self._choices): + choice = self._choices[keyid] + else: + return INPUT_PROCESSED + if choice == USETEXT: self._usevnc = False else: @@ -83,7 +88,7 @@ class AskVNCSpoke(NormalTUISpoke):
self.apply() self.close() - return None + return INPUT_PROCESSED
def apply(self): self.data.vnc.enabled = self._usevnc diff --git a/pyanaconda/ui/tui/spokes/network.py b/pyanaconda/ui/tui/spokes/network.py index cdd9209..2b56389 100644 --- a/pyanaconda/ui/tui/spokes/network.py +++ b/pyanaconda/ui/tui/spokes/network.py @@ -29,6 +29,7 @@ from pyanaconda.i18n import _ from pyanaconda import network from pyanaconda.nm import nm_activated_devices, nm_state, nm_devices, nm_device_type_is_ethernet, nm_device_ip_config, nm_activate_device_connection, nm_device_setting_value from pyanaconda.regexes import IPV4_PATTERN_WITHOUT_ANCHORS +from pyanaconda.constants_text import INPUT_PROCESSED
# pylint: disable-msg=E0611 from gi.repository import NetworkManager @@ -182,7 +183,7 @@ class NetworkSpoke(EditTUISpoke): self.app.switch_screen_modal(self.hostname_dialog, Entry(_("Hostname"), "hostname", re.compile(".*$"), True)) self.apply() - return True + return INPUT_PROCESSED elif 2 <= num <= len(self.supported_devices) + 1: # configure device devname = self.supported_devices[num-2] @@ -198,7 +199,7 @@ class NetworkSpoke(EditTUISpoke): ndata.bootProto = "static" if not ndata.gateway or not ndata.netmask: self.errors.append(_("Configuration not saved: gateway or netmask missing in static configuration")) - return True + return INPUT_PROCESSED
if ndata.ipv6 == "ignore": ndata.noipv6 = True @@ -213,7 +214,7 @@ class NetworkSpoke(EditTUISpoke): nm_activate_device_connection(devname, uuid)
self.apply() - return True + return INPUT_PROCESSED else: return key
diff --git a/pyanaconda/ui/tui/spokes/software.py b/pyanaconda/ui/tui/spokes/software.py index fc0298e..9b37f55 100644 --- a/pyanaconda/ui/tui/spokes/software.py +++ b/pyanaconda/ui/tui/spokes/software.py @@ -28,6 +28,7 @@ from pyanaconda.i18n import _
from pyanaconda.constants import THREAD_PAYLOAD, THREAD_PAYLOAD_MD from pyanaconda.constants import THREAD_CHECK_SOFTWARE, THREAD_SOFTWARE_WATCHER +from pyanaconda.constants_text import INPUT_PROCESSED
__all__ = ["SoftwareSpoke"]
@@ -140,16 +141,19 @@ class SoftwareSpoke(NormalTUISpoke):
def input(self, args, key): """ Handle the input; this chooses the desktop environment. """ - if key.lower() == "c" and self._selection in range(len(self.payload.environments)): - self.apply() - try: keyid = int(key) - 1 - if keyid in range(len(self.payload.environments)): - self._selection = keyid - return None - except (ValueError, IndexError): - return key + except ValueError: + if key.lower() == "c" and 0 <= self._selection < len(self.payload.environments): + self.apply() + self.close() + return INPUT_PROCESSED + else: + return key + + if 0 <= keyid < len(self.payload.environments): + self._selection = keyid + return INPUT_PROCESSED
@property def ready(self): diff --git a/pyanaconda/ui/tui/spokes/source.py b/pyanaconda/ui/tui/spokes/source.py index 16f5de4..de8eac4 100644 --- a/pyanaconda/ui/tui/spokes/source.py +++ b/pyanaconda/ui/tui/spokes/source.py @@ -30,6 +30,7 @@ from pyanaconda.image import opticalInstallMedia
from pyanaconda.constants import THREAD_SOURCE_WATCHER, THREAD_SOFTWARE_WATCHER, THREAD_PAYLOAD from pyanaconda.constants import THREAD_PAYLOAD_MD, THREAD_STORAGE, THREAD_CHECK_SOFTWARE +from pyanaconda.constants_text import INPUT_PROCESSED
import re
@@ -143,46 +144,47 @@ class SourceSpoke(EditTUISpoke): """ Handle the input; this decides the repo protocol. """ try: num = int(key) - if args == 2: - # network install - self._selection = num - if self._selection == 1: - # closest mirror - self.data.method.method = None - self.apply() - self.close() - return True - elif self._selection in range(2, 5): - self.data.method.method = "url" - newspoke = SpecifyRepoSpoke(self.app, self.data, self.storage, - self.payload, self.instclass, self._selection) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return True - elif self._selection == 5: - # nfs - self.data.method.method = "nfs" - newspoke = SpecifyNFSRepoSpoke(self.app, self.data, self.storage, - self.payload, self.instclass, self._selection, self.errors) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return True - else: - if num == 1: - # iso selected, just set some vars and return to main hub - self.data.method.method = "cdrom" - self.payload.install_device = self._cdrom - self.apply() - self.close() - return True - else: - self.app.switch_screen(self, num) - return None - except (ValueError, IndexError): + except ValueError: return key
+ if args == 2: + # network install + self._selection = num + if self._selection == 1: + # closest mirror + self.data.method.method = None + self.apply() + self.close() + return INPUT_PROCESSED + elif self._selection in range(2, 5): + self.data.method.method = "url" + newspoke = SpecifyRepoSpoke(self.app, self.data, self.storage, + self.payload, self.instclass, self._selection) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return INPUT_PROCESSED + elif self._selection == 5: + # nfs + self.data.method.method = "nfs" + newspoke = SpecifyNFSRepoSpoke(self.app, self.data, self.storage, + self.payload, self.instclass, self._selection, self.errors) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return INPUT_PROCESSED + else: + if num == 1: + # iso selected, just set some vars and return to main hub + self.data.method.method = "cdrom" + self.payload.install_device = self._cdrom + self.apply() + self.close() + return INPUT_PROCESSED + else: + self.app.switch_screen(self, num) + return INPUT_PROCESSED + def getRepoMetadata(self): """ Pull down yum repo metadata """ try: diff --git a/pyanaconda/ui/tui/spokes/storage.py b/pyanaconda/ui/tui/spokes/storage.py index 51ce0ab..d684cef 100644 --- a/pyanaconda/ui/tui/spokes/storage.py +++ b/pyanaconda/ui/tui/spokes/storage.py @@ -33,6 +33,7 @@ from pyanaconda.flags import flags from pyanaconda.kickstart import doKickstartStorage from pyanaconda.threads import threadMgr, AnacondaThread from pyanaconda.constants import THREAD_STORAGE, THREAD_STORAGE_WATCHER +from pyanaconda.constants_text import INPUT_PROCESSED from pyanaconda.i18n import _, P_
from pykickstart.constants import CLEARPART_TYPE_ALL, CLEARPART_TYPE_LINUX, CLEARPART_TYPE_NONE @@ -197,23 +198,22 @@ class StorageSpoke(NormalTUISpoke): def input(self, args, key): """Grab the disk choice and update things"""
- if key == "c": - if self.selected_disks: - newspoke = AutoPartSpoke(self.app, self.data, self.storage, - self.payload, self.instclass) - self.app.switch_screen_modal(newspoke) - self.apply() - self.execute() - self.close() - return None - try: - number = int(key) - self._update_disk_list(self.disks[number -1]) - return None - - except (ValueError, KeyError, IndexError): - return key + keyid = int(key) - 1 + self._update_disk_list(self.disks[keyid]) + return INPUT_PROCESSED + except (ValueError, IndexError): + if key.lower() == "c": + if self.selected_disks: + newspoke = AutoPartSpoke(self.app, self.data, self.storage, + self.payload, self.instclass) + self.app.switch_screen_modal(newspoke) + self.apply() + self.execute() + self.close() + return INPUT_PROCESSED + else: + return key
def apply(self): if not flags.automatedInstall: @@ -347,31 +347,31 @@ class AutoPartSpoke(NormalTUISpoke): def input(self, args, key): """Grab the choice and update things"""
- if key == "c": - newspoke = PartitionSchemeSpoke(self.app, self.data, self.storage, - self.payload, self.instclass) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return False - try: - number = int(key) - self.clearPartType = PARTTYPES[self.parttypelist[number -1]] - self.apply() - self.close() - return False + keyid = int(key) - 1 + except ValueError: + if key.lower() == "c": + newspoke = PartitionSchemeSpoke(self.app, self.data, self.storage, + self.payload, self.instclass) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return INPUT_PROCESSED + else: + return key
- except (ValueError, KeyError, IndexError): - return key + if 0 <= keyid < len(self.parttypelist): + self.clearPartType = PARTTYPES[self.parttypelist[keyid]] + self.apply() + return INPUT_PROCESSED
class PartitionSchemeSpoke(NormalTUISpoke): - """ SPoke to select what partitioning scheme to use on disk(s). """ + """ Spoke to select what partitioning scheme to use on disk(s). """ title = _("Partition Scheme Options") category = "destination"
# set default FS to LVM, for consistency with graphical behavior - _selection = 2 + _selection = 1
def __init__(self, app, data, storage, payload, instclass): NormalTUISpoke.__init__(self, app, data, storage, payload, instclass) @@ -389,7 +389,7 @@ class PartitionSchemeSpoke(NormalTUISpoke): for sch in schemelist: box = CheckboxWidget(title="%i) %s" %(schemelist.index(sch) \ + 1, sch), completed=(schemelist.index(sch) \ - + 1 == self._selection)) + == self._selection)) self._window += [box, ""]
message = _("Select a partition scheme configuration.") @@ -399,18 +399,19 @@ class PartitionSchemeSpoke(NormalTUISpoke): def input(self, args, key): """ Grab the choice and update things. """
- if key == "c" and self._selection: - self.apply() - self.close() - return None - try: - keyid = int(key) - if keyid in range(0, 4): - self._selection = keyid - return None - except (ValueError, IndexError, KeyError): - return key + keyid = int(key) - 1 + except ValueError: + if key.lower() == "c" and self._selection: + self.apply() + self.close() + return INPUT_PROCESSED + else: + return key + + if 0 <= keyid < len(self.partschemes): + self._selection = keyid + return INPUT_PROCESSED
def apply(self): """ Apply our selections. """ diff --git a/pyanaconda/ui/tui/spokes/time_spoke.py b/pyanaconda/ui/tui/spokes/time_spoke.py index 7e3467b..fc9ff36 100644 --- a/pyanaconda/ui/tui/spokes/time_spoke.py +++ b/pyanaconda/ui/tui/spokes/time_spoke.py @@ -24,6 +24,7 @@ from pyanaconda.ui.tui.simpleline import TextWidget, ColumnWidget from pyanaconda.ui.common import FirstbootSpokeMixIn from pyanaconda import timezone from pyanaconda.i18n import _ +from pyanaconda.constants_text import INPUT_PROCESSED
class TimeZoneSpoke(FirstbootSpokeMixIn, NormalTUISpoke): title = _("Timezone settings") @@ -87,46 +88,42 @@ class TimeZoneSpoke(FirstbootSpokeMixIn, NormalTUISpoke): def input(self, args, key): try: keyid = int(key) - 1 - if args: - self._selection = "%s/%s" % (args, self._timezones[args][keyid]) + except ValueError: + if key.lower().replace("_", " ") in self._lower_zones: + index = self._lower_zones.index(key.lower().replace("_", " ")) + self._selection = self._zones[index] self.apply() self.close() - else: - if len(self._timezones[self._regions[keyid]]) == 1: - self._selection = "%s/%s" % (self._regions[keyid], - self._timezones[self._regions[keyid]][0]) + return INPUT_PROCESSED + elif key.lower() in self._lower_regions: + index = self._lower_regions.index(key.lower()) + if len(self._timezones[self._regions[index]]) == 1: + self._selection = "%s/%s" % (self._regions[index], + self._timezones[self._regions[index]][0]) self.apply() self.close() else: - self.app.switch_screen(self, self._regions[keyid]) - return True - except (ValueError, IndexError): - pass - - if key.lower().replace("_", " ") in self._lower_zones: - index = self._lower_zones.index(key.lower().replace("_", " ")) - self._selection = self._zones[index] + self.app.switch_screen(self, self._regions[id]) + return INPUT_PROCESSED + elif key.lower() == "b": + self.app.switch_screen(self, None) + return INPUT_PROCESSED + else: + return key + + if args: + self._selection = "%s/%s" % (args, self._timezones[args][keyid]) self.apply() self.close() - return True - - elif key.lower() in self._lower_regions: - index = self._lower_regions.index(key.lower()) - if len(self._timezones[self._regions[index]]) == 1: - self._selection = "%s/%s" % (self._regions[index], - self._timezones[self._regions[index]][0]) + else: + if len(self._timezones[self._regions[keyid]]) == 1: + self._selection = "%s/%s" % (self._regions[keyid], + self._timezones[self._regions[keyid]][0]) self.apply() self.close() else: - self.app.switch_screen(self, self._regions[id]) - return True - - elif key.lower() == "b": - self.app.switch_screen(self, None) - return True - - else: - return key + self.app.switch_screen(self, self._regions[keyid]) + return INPUT_PROCESSED
def prompt(self, args = None): return _("Please select the timezone.\nUse numbers or type names directly [b to region list, q to quit]: ")
On Tue, 2013-09-10 at 14:54 -0400, Samantha N. Bueno wrote:
This is just some clean-up that needed to get done: -- removes some unnecessary error catching -- fixes up some typos -- fixes some inconsistency with variable naming -- also adds a couple constants to make the end result of input processing in the spokes less confusing
pyanaconda/constants_text.py | 4 ++ pyanaconda/ui/tui/spokes/askvnc.py | 13 +++-- pyanaconda/ui/tui/spokes/network.py | 7 +-- pyanaconda/ui/tui/spokes/software.py | 20 +++++--- pyanaconda/ui/tui/spokes/source.py | 78 +++++++++++++++-------------- pyanaconda/ui/tui/spokes/storage.py | 91 +++++++++++++++++----------------- pyanaconda/ui/tui/spokes/time_spoke.py | 57 ++++++++++----------- 7 files changed, 142 insertions(+), 128 deletions(-)
This makes the code much clearer, thanks! Just please update the pyanaconda/ui/tui/simpleline/base.py:UIObject.input method's docstring to mention the new constants.
On Wed, Sep 11, 2013 at 10:15:17AM +0200, Vratislav Podzimek wrote:
On Tue, 2013-09-10 at 14:54 -0400, Samantha N. Bueno wrote:
This is just some clean-up that needed to get done: -- removes some unnecessary error catching -- fixes up some typos -- fixes some inconsistency with variable naming -- also adds a couple constants to make the end result of input processing in the spokes less confusing
pyanaconda/constants_text.py | 4 ++ pyanaconda/ui/tui/spokes/askvnc.py | 13 +++-- pyanaconda/ui/tui/spokes/network.py | 7 +-- pyanaconda/ui/tui/spokes/software.py | 20 +++++--- pyanaconda/ui/tui/spokes/source.py | 78 +++++++++++++++-------------- pyanaconda/ui/tui/spokes/storage.py | 91 +++++++++++++++++----------------- pyanaconda/ui/tui/spokes/time_spoke.py | 57 ++++++++++----------- 7 files changed, 142 insertions(+), 128 deletions(-)
This makes the code much clearer, thanks! Just please update the pyanaconda/ui/tui/simpleline/base.py:UIObject.input method's docstring to mention the new constants.
Ah yes, thanks for the reminder, as well as the suggestions. Added the docstring change locally.
Samantha
Hrm, I'm not seeing my email, so re-sending. Apologies if this ends up being posted twice. ====
This is just some clean-up that needed to get done: -- removes some unnecessary error catching -- fixes up some typos -- fixes some inconsistency with variable naming -- also adds a couple constants to make the end result of input processing in the spokes less confusing --- pyanaconda/constants_text.py | 4 ++ pyanaconda/ui/tui/spokes/askvnc.py | 13 +++-- pyanaconda/ui/tui/spokes/network.py | 7 +-- pyanaconda/ui/tui/spokes/software.py | 20 +++++--- pyanaconda/ui/tui/spokes/source.py | 78 +++++++++++++++-------------- pyanaconda/ui/tui/spokes/storage.py | 91 +++++++++++++++++----------------- pyanaconda/ui/tui/spokes/time_spoke.py | 57 ++++++++++----------- 7 files changed, 142 insertions(+), 128 deletions(-)
diff --git a/pyanaconda/constants_text.py b/pyanaconda/constants_text.py index b557229..08a8046 100644 --- a/pyanaconda/constants_text.py +++ b/pyanaconda/constants_text.py @@ -52,3 +52,7 @@ TEXT_YES_BUTTON = Translator(TEXT_YES_STR, TEXT_YES_CHECK) TEXT_NO_STR = N_("No") TEXT_NO_CHECK = "no" TEXT_NO_BUTTON = Translator(TEXT_NO_STR, TEXT_NO_CHECK) + +# Make the return calls from the UIScreen input() function more clear +INPUT_PROCESSED = None +INPUT_DISCARDED = False diff --git a/pyanaconda/ui/tui/spokes/askvnc.py b/pyanaconda/ui/tui/spokes/askvnc.py index 9847874..f6e9528 100644 --- a/pyanaconda/ui/tui/spokes/askvnc.py +++ b/pyanaconda/ui/tui/spokes/askvnc.py @@ -22,6 +22,7 @@ from pyanaconda.ui.tui.spokes import NormalTUISpoke from pyanaconda.ui.tui.simpleline import TextWidget, ColumnWidget from pyanaconda.constants import USEVNC, USETEXT +from pyanaconda.constants_text import INPUT_PROCESSED from pyanaconda.i18n import _ import getpass
@@ -68,11 +69,15 @@ class AskVNCSpoke(NormalTUISpoke): """Override input so that we can launch the VNC password spoke"""
try: - number = int(key) - choice = self._choices[number -1] - except (ValueError, KeyError, IndexError): + keyid = int(key) - 1 + except ValueError: return key
+ if 0 <= keyid < len(self._choices): + choice = self._choices[keyid] + else: + return INPUT_PROCESSED + if choice == USETEXT: self._usevnc = False else: @@ -83,7 +88,7 @@ class AskVNCSpoke(NormalTUISpoke):
self.apply() self.close() - return None + return INPUT_PROCESSED
def apply(self): self.data.vnc.enabled = self._usevnc diff --git a/pyanaconda/ui/tui/spokes/network.py b/pyanaconda/ui/tui/spokes/network.py index cdd9209..2b56389 100644 --- a/pyanaconda/ui/tui/spokes/network.py +++ b/pyanaconda/ui/tui/spokes/network.py @@ -29,6 +29,7 @@ from pyanaconda.i18n import _ from pyanaconda import network from pyanaconda.nm import nm_activated_devices, nm_state, nm_devices, nm_device_type_is_ethernet, nm_device_ip_config, nm_activate_device_connection, nm_device_setting_value from pyanaconda.regexes import IPV4_PATTERN_WITHOUT_ANCHORS +from pyanaconda.constants_text import INPUT_PROCESSED
# pylint: disable-msg=E0611 from gi.repository import NetworkManager @@ -182,7 +183,7 @@ class NetworkSpoke(EditTUISpoke): self.app.switch_screen_modal(self.hostname_dialog, Entry(_("Hostname"), "hostname", re.compile(".*$"), True)) self.apply() - return True + return INPUT_PROCESSED elif 2 <= num <= len(self.supported_devices) + 1: # configure device devname = self.supported_devices[num-2] @@ -198,7 +199,7 @@ class NetworkSpoke(EditTUISpoke): ndata.bootProto = "static" if not ndata.gateway or not ndata.netmask: self.errors.append(_("Configuration not saved: gateway or netmask missing in static configuration")) - return True + return INPUT_PROCESSED
if ndata.ipv6 == "ignore": ndata.noipv6 = True @@ -213,7 +214,7 @@ class NetworkSpoke(EditTUISpoke): nm_activate_device_connection(devname, uuid)
self.apply() - return True + return INPUT_PROCESSED else: return key
diff --git a/pyanaconda/ui/tui/spokes/software.py b/pyanaconda/ui/tui/spokes/software.py index fc0298e..9b37f55 100644 --- a/pyanaconda/ui/tui/spokes/software.py +++ b/pyanaconda/ui/tui/spokes/software.py @@ -28,6 +28,7 @@ from pyanaconda.i18n import _
from pyanaconda.constants import THREAD_PAYLOAD, THREAD_PAYLOAD_MD from pyanaconda.constants import THREAD_CHECK_SOFTWARE, THREAD_SOFTWARE_WATCHER +from pyanaconda.constants_text import INPUT_PROCESSED
__all__ = ["SoftwareSpoke"]
@@ -140,16 +141,19 @@ class SoftwareSpoke(NormalTUISpoke):
def input(self, args, key): """ Handle the input; this chooses the desktop environment. """ - if key.lower() == "c" and self._selection in range(len(self.payload.environments)): - self.apply() - try: keyid = int(key) - 1 - if keyid in range(len(self.payload.environments)): - self._selection = keyid - return None - except (ValueError, IndexError): - return key + except ValueError: + if key.lower() == "c" and 0 <= self._selection < len(self.payload.environments): + self.apply() + self.close() + return INPUT_PROCESSED + else: + return key + + if 0 <= keyid < len(self.payload.environments): + self._selection = keyid + return INPUT_PROCESSED
@property def ready(self): diff --git a/pyanaconda/ui/tui/spokes/source.py b/pyanaconda/ui/tui/spokes/source.py index 16f5de4..de8eac4 100644 --- a/pyanaconda/ui/tui/spokes/source.py +++ b/pyanaconda/ui/tui/spokes/source.py @@ -30,6 +30,7 @@ from pyanaconda.image import opticalInstallMedia
from pyanaconda.constants import THREAD_SOURCE_WATCHER, THREAD_SOFTWARE_WATCHER, THREAD_PAYLOAD from pyanaconda.constants import THREAD_PAYLOAD_MD, THREAD_STORAGE, THREAD_CHECK_SOFTWARE +from pyanaconda.constants_text import INPUT_PROCESSED
import re
@@ -143,46 +144,47 @@ class SourceSpoke(EditTUISpoke): """ Handle the input; this decides the repo protocol. """ try: num = int(key) - if args == 2: - # network install - self._selection = num - if self._selection == 1: - # closest mirror - self.data.method.method = None - self.apply() - self.close() - return True - elif self._selection in range(2, 5): - self.data.method.method = "url" - newspoke = SpecifyRepoSpoke(self.app, self.data, self.storage, - self.payload, self.instclass, self._selection) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return True - elif self._selection == 5: - # nfs - self.data.method.method = "nfs" - newspoke = SpecifyNFSRepoSpoke(self.app, self.data, self.storage, - self.payload, self.instclass, self._selection, self.errors) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return True - else: - if num == 1: - # iso selected, just set some vars and return to main hub - self.data.method.method = "cdrom" - self.payload.install_device = self._cdrom - self.apply() - self.close() - return True - else: - self.app.switch_screen(self, num) - return None - except (ValueError, IndexError): + except ValueError: return key
+ if args == 2: + # network install + self._selection = num + if self._selection == 1: + # closest mirror + self.data.method.method = None + self.apply() + self.close() + return INPUT_PROCESSED + elif self._selection in range(2, 5): + self.data.method.method = "url" + newspoke = SpecifyRepoSpoke(self.app, self.data, self.storage, + self.payload, self.instclass, self._selection) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return INPUT_PROCESSED + elif self._selection == 5: + # nfs + self.data.method.method = "nfs" + newspoke = SpecifyNFSRepoSpoke(self.app, self.data, self.storage, + self.payload, self.instclass, self._selection, self.errors) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return INPUT_PROCESSED + else: + if num == 1: + # iso selected, just set some vars and return to main hub + self.data.method.method = "cdrom" + self.payload.install_device = self._cdrom + self.apply() + self.close() + return INPUT_PROCESSED + else: + self.app.switch_screen(self, num) + return INPUT_PROCESSED + def getRepoMetadata(self): """ Pull down yum repo metadata """ try: diff --git a/pyanaconda/ui/tui/spokes/storage.py b/pyanaconda/ui/tui/spokes/storage.py index 51ce0ab..d684cef 100644 --- a/pyanaconda/ui/tui/spokes/storage.py +++ b/pyanaconda/ui/tui/spokes/storage.py @@ -33,6 +33,7 @@ from pyanaconda.flags import flags from pyanaconda.kickstart import doKickstartStorage from pyanaconda.threads import threadMgr, AnacondaThread from pyanaconda.constants import THREAD_STORAGE, THREAD_STORAGE_WATCHER +from pyanaconda.constants_text import INPUT_PROCESSED from pyanaconda.i18n import _, P_
from pykickstart.constants import CLEARPART_TYPE_ALL, CLEARPART_TYPE_LINUX, CLEARPART_TYPE_NONE @@ -197,23 +198,22 @@ class StorageSpoke(NormalTUISpoke): def input(self, args, key): """Grab the disk choice and update things"""
- if key == "c": - if self.selected_disks: - newspoke = AutoPartSpoke(self.app, self.data, self.storage, - self.payload, self.instclass) - self.app.switch_screen_modal(newspoke) - self.apply() - self.execute() - self.close() - return None - try: - number = int(key) - self._update_disk_list(self.disks[number -1]) - return None - - except (ValueError, KeyError, IndexError): - return key + keyid = int(key) - 1 + self._update_disk_list(self.disks[keyid]) + return INPUT_PROCESSED + except (ValueError, IndexError): + if key.lower() == "c": + if self.selected_disks: + newspoke = AutoPartSpoke(self.app, self.data, self.storage, + self.payload, self.instclass) + self.app.switch_screen_modal(newspoke) + self.apply() + self.execute() + self.close() + return INPUT_PROCESSED + else: + return key
def apply(self): if not flags.automatedInstall: @@ -347,31 +347,31 @@ class AutoPartSpoke(NormalTUISpoke): def input(self, args, key): """Grab the choice and update things"""
- if key == "c": - newspoke = PartitionSchemeSpoke(self.app, self.data, self.storage, - self.payload, self.instclass) - self.app.switch_screen_modal(newspoke) - self.apply() - self.close() - return False - try: - number = int(key) - self.clearPartType = PARTTYPES[self.parttypelist[number -1]] - self.apply() - self.close() - return False + keyid = int(key) - 1 + except ValueError: + if key.lower() == "c": + newspoke = PartitionSchemeSpoke(self.app, self.data, self.storage, + self.payload, self.instclass) + self.app.switch_screen_modal(newspoke) + self.apply() + self.close() + return INPUT_PROCESSED + else: + return key
- except (ValueError, KeyError, IndexError): - return key + if 0 <= keyid < len(self.parttypelist): + self.clearPartType = PARTTYPES[self.parttypelist[keyid]] + self.apply() + return INPUT_PROCESSED
class PartitionSchemeSpoke(NormalTUISpoke): - """ SPoke to select what partitioning scheme to use on disk(s). """ + """ Spoke to select what partitioning scheme to use on disk(s). """ title = _("Partition Scheme Options") category = "destination"
# set default FS to LVM, for consistency with graphical behavior - _selection = 2 + _selection = 1
def __init__(self, app, data, storage, payload, instclass): NormalTUISpoke.__init__(self, app, data, storage, payload, instclass) @@ -389,7 +389,7 @@ class PartitionSchemeSpoke(NormalTUISpoke): for sch in schemelist: box = CheckboxWidget(title="%i) %s" %(schemelist.index(sch) \ + 1, sch), completed=(schemelist.index(sch) \ - + 1 == self._selection)) + == self._selection)) self._window += [box, ""]
message = _("Select a partition scheme configuration.") @@ -399,18 +399,19 @@ class PartitionSchemeSpoke(NormalTUISpoke): def input(self, args, key): """ Grab the choice and update things. """
- if key == "c" and self._selection: - self.apply() - self.close() - return None - try: - keyid = int(key) - if keyid in range(0, 4): - self._selection = keyid - return None - except (ValueError, IndexError, KeyError): - return key + keyid = int(key) - 1 + except ValueError: + if key.lower() == "c" and self._selection: + self.apply() + self.close() + return INPUT_PROCESSED + else: + return key + + if 0 <= keyid < len(self.partschemes): + self._selection = keyid + return INPUT_PROCESSED
def apply(self): """ Apply our selections. """ diff --git a/pyanaconda/ui/tui/spokes/time_spoke.py b/pyanaconda/ui/tui/spokes/time_spoke.py index 7e3467b..fc9ff36 100644 --- a/pyanaconda/ui/tui/spokes/time_spoke.py +++ b/pyanaconda/ui/tui/spokes/time_spoke.py @@ -24,6 +24,7 @@ from pyanaconda.ui.tui.simpleline import TextWidget, ColumnWidget from pyanaconda.ui.common import FirstbootSpokeMixIn from pyanaconda import timezone from pyanaconda.i18n import _ +from pyanaconda.constants_text import INPUT_PROCESSED
class TimeZoneSpoke(FirstbootSpokeMixIn, NormalTUISpoke): title = _("Timezone settings") @@ -87,46 +88,42 @@ class TimeZoneSpoke(FirstbootSpokeMixIn, NormalTUISpoke): def input(self, args, key): try: keyid = int(key) - 1 - if args: - self._selection = "%s/%s" % (args, self._timezones[args][keyid]) + except ValueError: + if key.lower().replace("_", " ") in self._lower_zones: + index = self._lower_zones.index(key.lower().replace("_", " ")) + self._selection = self._zones[index] self.apply() self.close() - else: - if len(self._timezones[self._regions[keyid]]) == 1: - self._selection = "%s/%s" % (self._regions[keyid], - self._timezones[self._regions[keyid]][0]) + return INPUT_PROCESSED + elif key.lower() in self._lower_regions: + index = self._lower_regions.index(key.lower()) + if len(self._timezones[self._regions[index]]) == 1: + self._selection = "%s/%s" % (self._regions[index], + self._timezones[self._regions[index]][0]) self.apply() self.close() else: - self.app.switch_screen(self, self._regions[keyid]) - return True - except (ValueError, IndexError): - pass - - if key.lower().replace("_", " ") in self._lower_zones: - index = self._lower_zones.index(key.lower().replace("_", " ")) - self._selection = self._zones[index] + self.app.switch_screen(self, self._regions[id]) + return INPUT_PROCESSED + elif key.lower() == "b": + self.app.switch_screen(self, None) + return INPUT_PROCESSED + else: + return key + + if args: + self._selection = "%s/%s" % (args, self._timezones[args][keyid]) self.apply() self.close() - return True - - elif key.lower() in self._lower_regions: - index = self._lower_regions.index(key.lower()) - if len(self._timezones[self._regions[index]]) == 1: - self._selection = "%s/%s" % (self._regions[index], - self._timezones[self._regions[index]][0]) + else: + if len(self._timezones[self._regions[keyid]]) == 1: + self._selection = "%s/%s" % (self._regions[keyid], + self._timezones[self._regions[keyid]][0]) self.apply() self.close() else: - self.app.switch_screen(self, self._regions[id]) - return True - - elif key.lower() == "b": - self.app.switch_screen(self, None) - return True - - else: - return key + self.app.switch_screen(self, self._regions[keyid]) + return INPUT_PROCESSED
def prompt(self, args = None): return _("Please select the timezone.\nUse numbers or type names directly [b to region list, q to quit]: ")
@property
- def completed(self):
return True # We're always complete
def completed(self):return True # We're always complete
I believe these two lines shouldn't be indented more. Other than that I agree with Brian that we should use comparisons instead of using range. And if we need to use range somewhere, it should be replaced with xrange.
Since it's not directly accessible, the 'completed' property is unnecessary. --- pyanaconda/ui/tui/spokes/storage.py | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/pyanaconda/ui/tui/spokes/storage.py b/pyanaconda/ui/tui/spokes/storage.py index 92b5bf2..84ba1d4 100644 --- a/pyanaconda/ui/tui/spokes/storage.py +++ b/pyanaconda/ui/tui/spokes/storage.py @@ -314,10 +314,6 @@ class AutoPartSpoke(NormalTUISpoke): def indirect(self): return True
- @property - def completed(self): - return True # We're always complete - def refresh(self, args = None): NormalTUISpoke.refresh(self, args) # synchronize our local data store with the global ksdata
On Tue, 2013-09-03 at 08:31 -0400, Samantha N. Bueno wrote:
Since it's not directly accessible, the 'completed' property is unnecessary.
pyanaconda/ui/tui/spokes/storage.py | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/pyanaconda/ui/tui/spokes/storage.py b/pyanaconda/ui/tui/spokes/storage.py index 92b5bf2..84ba1d4 100644 --- a/pyanaconda/ui/tui/spokes/storage.py +++ b/pyanaconda/ui/tui/spokes/storage.py @@ -314,10 +314,6 @@ class AutoPartSpoke(NormalTUISpoke): def indirect(self): return True
- @property
def completed(self):return True # We're always complete
Oh, I see those lines are removed here. But still I think the indentation should not be changed in the previous patch.
On Wed, Sep 04, 2013 at 07:43:49AM +0200, Vratislav Podzimek wrote:
On Tue, 2013-09-03 at 08:31 -0400, Samantha N. Bueno wrote:
Since it's not directly accessible, the 'completed' property is unnecessary.
pyanaconda/ui/tui/spokes/storage.py | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/pyanaconda/ui/tui/spokes/storage.py b/pyanaconda/ui/tui/spokes/storage.py index 92b5bf2..84ba1d4 100644 --- a/pyanaconda/ui/tui/spokes/storage.py +++ b/pyanaconda/ui/tui/spokes/storage.py @@ -314,10 +314,6 @@ class AutoPartSpoke(NormalTUISpoke): def indirect(self): return True
- @property
def completed(self):return True # We're always completeOh, I see those lines are removed here. But still I think the indentation should not be changed in the previous patch.
Yeah, that indentation change was a typo, fixed locally.
Samantha
anaconda-patches@lists.fedorahosted.org