Re: [PATCH 2/4] Define default autopart type as a constant

On Wed, May 28, 2014 at 10:31:08AM +0200, Vratislav Podzimek wrote: > Also preserve the chosen autopart type between runs of the PartitionScheme TUI > spoke. > > Signed-off-by: Vratislav Podzimek <vpodzime(a)redhat.com&gt; > --- > pyanaconda/constants.py | 2 ++ > pyanaconda/ui/gui/spokes/lib/accordion.py | 12 ++++++++---- > pyanaconda/ui/tui/spokes/storage.py | 18 ++++++++++-------- > 3 files changed, 20 insertions(+), 12 deletions(-) > > diff --git a/pyanaconda/constants.py b/pyanaconda/constants.py > index eff9966..db786b5 100644 > --- a/pyanaconda/constants.py > +++ b/pyanaconda/constants.py > @@ -164,3 +164,5 @@ TAR_SUFFIX = (".tar", ".tbz", ".tgz", ".txz", ".tar.bz2", "tar.gz", "tar.xz") > > # cmdline arguments that append instead of overwrite > CMDLINE_APPEND = ["modprobe.blacklist"] > + > +DEFAULT_AUTOPART_TYPE = AUTOPART_TYPE_LVM > diff --git a/pyanaconda/ui/gui/spokes/lib/accordion.py b/pyanaconda/ui/gui/spokes/lib/accordion.py > index 6a9f8a8..5cabcf0 100644 > --- a/pyanaconda/ui/gui/spokes/lib/accordion.py > +++ b/pyanaconda/ui/gui/spokes/lib/accordion.py > @@ -26,6 +26,7 @@ from pyanaconda.product import productName, productVersion > from pyanaconda.ui.gui.utils import escape_markup, really_hide, really_show > from pykickstart.constants import AUTOPART_TYPE_PLAIN, AUTOPART_TYPE_BTRFS > from pykickstart.constants import AUTOPART_TYPE_LVM, AUTOPART_TYPE_LVM_THINP > +from pyanaconda.constants import DEFAULT_AUTOPART_TYPE > > from gi.repository.AnacondaWidgets import MountpointSelector > from gi.repository import Gtk > @@ -327,10 +328,13 @@ class CreateNewPage(Page): > self._createBox.attach(label, 0, 4, 2, 1) > label.set_mnemonic_widget(combo) > > - store.append([_("Standard Partition"), AUTOPART_TYPE_PLAIN]) > - store.append([_("BTRFS"), AUTOPART_TYPE_BTRFS]) > - default = store.append([_("LVM"), AUTOPART_TYPE_LVM]) > - store.append([_("LVM Thin Provisioning"), AUTOPART_TYPE_LVM_THINP]) > + for item in ([_("Standard Partition"), AUTOPART_TYPE_PLAIN], > + [_("BTRFS"), AUTOPART_TYPE_BTRFS], > + [_("LVM"), AUTOPART_TYPE_LVM], > + [_("LVM Thin Provisioning"), AUTOPART_TYPE_LVM_THINP]): May as well use a tuple of tuples here instead of lists since they aren't ever changed, are they?

> + itr = store.append(item) > + if item[1] == DEFAULT_AUTOPART_TYPE: > + default = itr > > combo.set_margin_left(18) > combo.set_margin_right(18) > diff --git a/pyanaconda/ui/tui/spokes/storage.py b/pyanaconda/ui/tui/spokes/storage.py > index baf1674..032676a 100644 > --- a/pyanaconda/ui/tui/spokes/storage.py > +++ b/pyanaconda/ui/tui/spokes/storage.py > @@ -39,7 +39,7 @@ from blivet.devicelibs.dasd import format_dasd, make_unformatted_dasd_list > 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, THREAD_DASDFMT > +from pyanaconda.constants import THREAD_STORAGE, THREAD_STORAGE_WATCHER, THREAD_DASDFMT, DEFAULT_AUTOPART_TYPE > from pyanaconda.constants_text import INPUT_PROCESSED > from pyanaconda.i18n import _, P_, N_ > from pyanaconda.bootloader import BootLoaderError > @@ -501,15 +501,17 @@ class PartitionSchemeSpoke(NormalTUISpoke): > title = N_("Partition Scheme Options") > category = SystemCategory > > - # set default FS to LVM, for consistency with graphical behavior > - _selection = 1 > - > def __init__(self, app, data, storage, payload, instclass): > NormalTUISpoke.__init__(self, app, data, storage, payload, instclass) > - self.partschemes = OrderedDict([("Standard Partition", AUTOPART_TYPE_PLAIN), > - ("LVM", AUTOPART_TYPE_LVM), > - ("LVM Thin Provisioning", AUTOPART_TYPE_LVM_THINP), > - ("BTRFS", AUTOPART_TYPE_BTRFS)]) > + self.partschemes = OrderedDict() > + pre_select = self.data.autopart.type or DEFAULT_AUTOPART_TYPE > + for i, item in enumerate([("Standard Partition", AUTOPART_TYPE_PLAIN), > + ("LVM", AUTOPART_TYPE_LVM), > + ("LVM Thin Provisioning", AUTOPART_TYPE_LVM_THINP), > + ("BTRFS", AUTOPART_TYPE_BTRFS)]): Does the order really matter? We shouldn't have to maintain this list in multiple places if it doesn't, or if we can change the other order to match.