The first patch exists because I thought it would help with the second. Ultimately it didn't, but I think it's a step in the right direction, so I'm including it.
mulhern (2): Make size_from_input() and size_from_entry() methods handier. Changes around handling of size entries in custom spoke.
pyanaconda/storage_utils.py | 13 ++++++++----- pyanaconda/ui/gui/spokes/custom.py | 18 ++++++++++-------- .../ui/gui/spokes/lib/custom_storage_helpers.py | 21 +++++++++++++++++++-- 3 files changed, 37 insertions(+), 15 deletions(-)
* In size_from_input(), do not round up to any lower bound, just get the size. It's more general that way. * Parameterize size_from_entry() on a lower bound. * Add method docstrings.
Should not change any observable behavior, as size_from_input() seems to be only invoked by size_from_entry() method.
Signed-off-by: mulhern amulhern@redhat.com --- pyanaconda/storage_utils.py | 13 ++++++++----- .../ui/gui/spokes/lib/custom_storage_helpers.py | 21 +++++++++++++++++++-- 2 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/pyanaconda/storage_utils.py b/pyanaconda/storage_utils.py index 7d26e78..16a1d5a 100644 --- a/pyanaconda/storage_utils.py +++ b/pyanaconda/storage_utils.py @@ -91,7 +91,14 @@ NAMED_DEVICE_TYPES = (DEVICE_TYPE_BTRFS, DEVICE_TYPE_LVM, DEVICE_TYPE_MD, DEVICE CONTAINER_DEVICE_TYPES = (DEVICE_TYPE_LVM, DEVICE_TYPE_BTRFS, DEVICE_TYPE_LVM_THINP)
def size_from_input(input_str): - """Get size from user's input""" + """ Get a Size object from an input string. + + :param str input_str: a string forming some representation of a size + :returns: a Size object corresponding to input_str + :rtype: :class:`blivet.size.Size` or NoneType + + If no units specified, defaults to MiB. + """
if not input_str: # Nothing to parse @@ -106,10 +113,6 @@ def size_from_input(input_str): size = Size(input_str) except ValueError: return None - else: - # Minimium size for ui-created partitions is 1MiB. - if size.convertTo(spec="MiB") < 1: - size = Size("1 MiB")
return size
diff --git a/pyanaconda/ui/gui/spokes/lib/custom_storage_helpers.py b/pyanaconda/ui/gui/spokes/lib/custom_storage_helpers.py index 4d0948c..a6a2263 100644 --- a/pyanaconda/ui/gui/spokes/lib/custom_storage_helpers.py +++ b/pyanaconda/ui/gui/spokes/lib/custom_storage_helpers.py @@ -78,9 +78,26 @@ CONTAINER_TYPES = {DEVICE_TYPE_LVM: ContainerType(N_("Volume Group"), N_(" # These cannot be specified as mountpoints system_mountpoints = ["/dev", "/proc", "/run", "/sys"]
-def size_from_entry(entry): +def size_from_entry(entry, lower_bound=Size("1 MiB")): + """ Get a Size object from an entry field. + + :param lower_bound: lower bound for size returned, + :type lower_bound: :class:`blivet.size.Size` or NoneType + :returns: a Size object corresponding to the text in the entry field + :rtype: :class:`blivet.size.Size` or NoneType + + If no units specified in the field, defaults to MiB. + + Rounds up to lower_bound, if value in entry field corresponds + to a smaller value. The default for lower_bound is 1 MiB. + """ size_text = entry.get_text().decode("utf-8").strip() - return size_from_input(size_text) + size = size_from_input(size_text) + if size is None: + return None + if lower_bound is not None and size < lower_bound: + return lower_bound + return size
def populate_mountpoint_store(store, used_mountpoints): # sure, add whatever you want to this list. this is just a start.
diff --git a/pyanaconda/ui/gui/spokes/lib/custom_storage_helpers.py b/pyanaconda/ui/gui/spokes/lib/custom_storage_helpers.py index 4d0948c..a6a2263 100644 --- a/pyanaconda/ui/gui/spokes/lib/custom_storage_helpers.py +++ b/pyanaconda/ui/gui/spokes/lib/custom_storage_helpers.py @@ -78,9 +78,26 @@ CONTAINER_TYPES = {DEVICE_TYPE_LVM: ContainerType(N_("Volume Group"), N_(" # These cannot be specified as mountpoints system_mountpoints = ["/dev", "/proc", "/run", "/sys"]
-def size_from_entry(entry): +def size_from_entry(entry, lower_bound=Size("1 MiB")):
This strikes me as dangerous for the same reason that this would be:
def size_from_entry(entry, lower_bound=[]):
Perhaps the best technique here to avoid problems is set lower_bound=None and then set the real default in the both of the method.
- Chris
* Make the max places displayed on this spoke a named constant. * Get rid of _device_size_text attribute. It looks like that state is unnecessary and CustomSpoke has plenty of state as it is. * Still assume that if the entry text has not changed at all from the text displayed, then no change was intended. * Also, if the entry text has changed, but it represents exactly the same size as the current device's size, consider this no change. Assuming the size was actually 1 MiB, and that the string representation was "1 MiB", if the user enters "1" (which is interpreted as 1 MiB), or "1024 KiB", or "1 MiB ", or even, "1.00 MiB", then changed_size will be False. Previously any of these would have been interpreted as an actual change.
This is still a bit paradoxical, because not all digits are displayed. The value displayed represents a range of possible values, of which the actual size value is just one. This means that, if the actual value is 1.001 MiB, and the displayed value is "1 MiB", and the user enters "1.0 MiB", then the value will be updated (but the user won't see the change, because the value will still be represented as "1 MiB"). This patch doesn't change that fact, though. It's hard to do much different w/out getting a whole lot more sophisticated. --- pyanaconda/ui/gui/spokes/custom.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index 9fdfbc2..6e7038a 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -141,6 +141,9 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): category = SystemCategory title = N_("MANUAL PARTITIONING")
+ # The maximum number of places to show when displaying a size + MAX_SIZE_PLACES = 2 + def __init__(self, data, storage, payload, instclass): StorageChecker.__init__(self, min_ram=isys.MIN_GUI_RAM) NormalSpoke.__init__(self, data, storage, payload, instclass) @@ -157,7 +160,6 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._fs_types = [] # list of supported fstypes self._free_space = Size(0)
- self._device_size_text = None self._device_disks = [] self._device_container_name = None self._device_container_raid_level = None @@ -918,15 +920,16 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): # SIZE old_size = device.size
- # we are interested in size human readable representation change because - # that's what the user sees - same_size = self._device_size_text == self._sizeEntry.get_text() - if same_size: + # If the size text hasn't changed at all from that displayed, + # assume no change intended. + if old_size.humanReadable(max_places=self.MAX_SIZE_PLACES) == self._sizeEntry.get_text(): size = old_size else: + # Note that this size may not correspond to user's entry, + # as size_from_entry has a default lower limit. size = size_from_entry(self._sizeEntry) changed_size = ((use_dev.resizable or not use_dev.exists) and - not same_size) + size != old_size) old_device_info["size"] = old_size new_device_info["size"] = size
@@ -1460,8 +1463,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._labelEntry.set_text("") fancy_set_sensitive(self._labelEntry, True)
- self._device_size_text = device.size.humanReadable(max_places=2) - self._sizeEntry.set_text(self._device_size_text) + self._sizeEntry.set_text(device.size.humanReadable(max_places=self.MAX_SIZE_PLACES))
self._reformatCheckbox.set_active(not device.format.exists) fancy_set_sensitive(self._reformatCheckbox,
anaconda-patches@lists.fedorahosted.org