[anaconda:master 4/4] Refactor handling of fsCombo considerations.

mulhern amulhern at redhat.com
Wed Oct 1 00:06:38 UTC 2014


Previously there was a method called _resolve_btrfs_restrictions().
Now there is a method called _handle_fstype_combo() which behaves
slightly differently.

Changes are:

* Make the method dependent on the device_type chosen, rather than
a boolean result. Everything about the fsCombo changes is really
just dependent on this value, so it's better to make that explicit by the
parameter.
* Remove should_be_btrfs and make whether or not to display and lock
btrfs in fsCombo dependent only on whether btrfs device type is selected.
Checking whether btrfs is supported in this method is a bad idea, because:
 - If the device already exists and is btrfs, then the fsCombo should
be locked to btrfs and nothing else.
 - If the device type has already been changed to btrfs, then only btrfs
filesystem is allowed, so locking the fstype to whatever the user had
previously chosen before they made the device type change isn't really
much better than displaying btrfs filesystem and locking it to that.
Neither one is something that anaconda/blivet can ultimately do.
* Move setting of sensitivity of fsCombo in with the other manipulations
of fsCombo. They are all connected.
* Make method slightly more robust. There are a bunch of situations that,
by close reasoning, you can prove could not happen in the current GUI,
but that could change. So, situation where device type is now BTRFS and
btrfs entry is already in fsCombo is handled. Situation where device type is
not BTRFS and btrfs is not in fsCombo is handled explicitly, rather than
implicitly. Situation where btrfs is in combo and some other fs that is
past btrfs in the combo is selected is now handled, so that user won't see
selected fs spontaneously shift to a different one on btrfs removal.

Signed-off-by: mulhern <amulhern at redhat.com>
---
 pyanaconda/ui/gui/spokes/custom.py | 82 ++++++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 34 deletions(-)

diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py
index f46c6f6..01d0081 100644
--- a/pyanaconda/ui/gui/spokes/custom.py
+++ b/pyanaconda/ui/gui/spokes/custom.py
@@ -2389,27 +2389,55 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
         container_exists = getattr(container, "exists", False)
         self._modifyContainerButton.set_sensitive(not container_exists)
 
-    def _resolve_btrfs_restrictions(self, should_be_btrfs):
-        model = self._fsCombo.get_model()
-        btrfs_pos = None
-        for idx, data in enumerate(model):
-            if data[0] == "btrfs":
-                btrfs_pos = idx
-
-        active_index = self._fsCombo.get_active()
-        current_fstype = self._fsCombo.get_active_text()
-        if btrfs_pos and not should_be_btrfs:
-            self._fsCombo.remove(btrfs_pos)
-            if current_fstype == "btrfs":
-                for i, row in enumerate(model):
-                    if row[0] == self.storage.defaultFSType:
-                        active_index = i
-                        break
-        elif should_be_btrfs and not btrfs_pos:
-            self._fsCombo.append_text("btrfs")
-            active_index = len(self._fsCombo.get_model()) - 1
+    def _handle_fstype_combo(self, device_type):
+        """ Set up device type dependent portion of filesystem combo.
+
+            :param int device_type: an int representing the device type
+
+            Generally speaking, the filesystem combo can be set up without
+            reference to the device type because the choice of filesystem
+            combo and of device type is orthogonal.
+
+            However, choice of btrfs device type requires choice of btrfs
+            filesystem type, and choice of any other device type precludes
+            choice of btrfs filesystem type.
+
+            Preconditions are:
+            * the filesystem combo contains at least the default filesystem
+            * the default filesystem is not the same as btrfs
+            * if device_type is DEVICE_TYPE_BTRFS, btrfs is supported
+        """
+        # Find unique instance of btrfs in fsCombo, if any.
+        btrfs_idx = next((idx for idx, data in enumerate(self._fsCombo.get_model()) if data[0] == "btrfs"), None)
+
+        if device_type == DEVICE_TYPE_BTRFS:
+            # If no btrfs entry, add one, and select the new entry
+            if btrfs_idx is None:
+                self._fsCombo.append_text("btrfs")
+                active_index = len(self._fsCombo.get_model()) - 1
+            # Otherwise, select the already located btrfs entry
+            else:
+                active_index = btrfs_idx
+        else:
+            # Get the currently active index
+            active_index = self._fsCombo.get_active()
+
+            # If there is a btrfs entry, remove and adjust active_index
+            if btrfs_idx is not None:
+                self._fsCombo.remove(btrfs_idx)
+
+                # If btrfs previously selected, select default filesystem
+                if active_index == btrfs_idx:
+                    active_index = next(idx for idx, data in enumerate(self._fsCombo.get_model()) if data[0] == self.storage.defaultFSType)
+                # Otherwise, shift index left by one if after removed entry
+                elif active_index > btrfs_idx:
+                    active_index = active_index - 1
+            # If there is no btrfs entry, stick with user's previous choice
+            else:
+                pass
 
         self._fsCombo.set_active(active_index)
+        fancy_set_sensitive(self._fsCombo, self._reformatCheckbox.get_active() and device_type != DEVICE_TYPE_BTRFS)
 
     def on_device_type_changed(self, combo):
         if not self._initialized:
@@ -2425,17 +2453,6 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
         if new_type is None:
             return
 
-        # if device type is not btrfs we want to make sure btrfs is not in the
-        # fstype combo
-        should_be_btrfs = False
-        fs_type_sensitive = True
-
-        if new_type == DEVICE_TYPE_BTRFS:
-            # add btrfs to the fstype combo and lock it in
-            test_fmt = getFormat("btrfs")
-            should_be_btrfs = test_fmt.supported and test_fmt.formattable
-            fs_type_sensitive = False
-
         # lvm uses the RHS to set disk set. no foolish minds here.
         exists = self._current_selector and self._current_selector.device.exists
         self._configButton.set_sensitive(not exists and new_type not in CONTAINER_DEVICE_TYPES)
@@ -2452,10 +2469,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
         self._nameEntry.set_text(self._device_name_dict[new_type])
         fancy_set_sensitive(self._sizeEntry, new_type != DEVICE_TYPE_BTRFS)
 
-        fancy_set_sensitive(self._fsCombo, self._reformatCheckbox.get_active() and
-                                           fs_type_sensitive)
-
-        self._resolve_btrfs_restrictions(should_be_btrfs)
+        self._handle_fstype_combo(new_type)
 
     def clear_errors(self):
         self._error = None
-- 
1.9.3



More information about the anaconda-patches mailing list