On Thu, 2009-04-09 at 16:58 +0200, Joel Granados wrote:
> This patch contains an error when trying to add a lv. I will adjust.
> But I would still like someone to comment on the idea. To me it just
> seem cleaner when implemented this way.
Basically you have made it so there is only one "if lv.exists:...else"
block instead of several? If so, I definitely like the idea. The dialogs
are all such a mess in this way.
Dave
>
> regards.
> On Thu, Apr 09, 2009 at 04:49:03PM +0200, Joel Granados Moreno wrote:
> > Separate the logic from the actual constructing of the gtk stuff is very
> > helpful when trying to debug, develop these files. This commit doesn't
> > actually change any behavior in the file.
> > ---
> > iw/lvm_dialog_gui.py | 170 +++++++++++++++++++++++++++++++------------------
> > 1 files changed, 107 insertions(+), 63 deletions(-)
> >
> > diff --git a/iw/lvm_dialog_gui.py b/iw/lvm_dialog_gui.py
> > index 431341f..a9223b4 100644
> > --- a/iw/lvm_dialog_gui.py
> > +++ b/iw/lvm_dialog_gui.py
> > @@ -367,23 +367,33 @@ class VolumeGroupEditor:
> > return iter
> >
> > def editLogicalVolume(self, lv, isNew = 0):
> > + # Mixing logical code and gtk code is confusing to me. So I am going
> > + # to do the logic first and then create all the gtk crap!
> > + #
> > + # lv -- whatever self.logvolstore.get_value returns
> > +
> > + #newfstypelabel = None # File system type label & combo
> > + #newfstypeCombo = None
> > + newfslabellabel = None # File system Label label & combo
> > + newfslableCombo = None
> > + #lvnamelabel = None # Logical Volume name label & entry
> > + #lvnameentry = None
> > + #lvsizelabel = None # Logical Volume size label & entry
> > + #lvsizeentry = None
> > + maxsizelabel = None # Maximum size label
> > + #mountCombo = None # Mount Point Combo Box
> > + #tstr = None # String that appears on top of the window
> > +
> > + # Define the string
> > if isNew:
> > tstr = _("Make Logical Volume")
> > else:
> > tstr = _("Edit Logical Volume: %s") % lv['name']
> >
> > - dialog = gtk.Dialog(tstr, self.parent)
> > - gui.addFrame(dialog)
> > - dialog.add_button('gtk-cancel', 2)
> > - dialog.add_button('gtk-ok', 1)
> > - dialog.set_position(gtk.WIN_POS_CENTER)
> > -
> > - maintable = gtk.Table()
> > - maintable.set_row_spacings(5)
> > - maintable.set_col_spacings(5)
> > - row = 0
> > -
> > - tempvg = self.getTempVG()
> > + # Create the mountCombo. This is the box where the mountpoint will
> > + # appear. Note that if the format is swap or Raiddevice, the mount
> > + # point is none-sense.
> > + tempvg = self.getTempVG() # copy of self.vg
> > templv = None
> > templuks = None
> > usedev = None
> > @@ -403,75 +413,106 @@ class VolumeGroupEditor:
> > format = self.luks[lv['name']]
> > else:
> > format = lv['format']
> > -
> > - lbl = createAlignedLabel(_("_Mount Point:"))
> > - maintable.attach(lbl, 0, 1, row,row+1)
> > mountCombo = createMountPointCombo(usedev, excludeMountPoints=["/boot"])
> > - lbl.set_mnemonic_widget(mountCombo)
> > - maintable.attach(mountCombo, 1, 2, row, row + 1)
> > - row += 1
> >
> > +
> > + # Stuff appears differently when the lv exists and when the lv is new.
> > + # here we make that difference. Except for newfslabelCombo, and
> > + # maxsizelabel all vars will have a value != None.
> > if not lv['exists']:
> > - lbl = createAlignedLabel(_("_File System Type:"))
> > - maintable.attach(lbl, 0, 1, row, row + 1)
> > + # File system type lables & combo
> > + newfstypelabel = createAlignedLabel(_("_File System Type:"))
> > newfstypeCombo = createFSTypeMenu(format, fstypechangeCB,mountCombo,
> > ignorefs = ["software RAID", "physical volume (LVM)",
> > "efi", "PPC PReP Boot", "Apple Bootstrap"])
> > - lbl.set_mnemonic_widget(newfstypeCombo)
> > - maintable.attach(newfstypeCombo, 1, 2, row, row + 1)
> > - row += 1
> > + newfstypeCombo.set_mnemonic_widget(newfstypeCombo)
> > +
> > + # Logical Volume name label & entry
> > + lvnamelabel = createAlignedLabel(_("_Logical Volume Name:"))
> > + lvnameentry = gtk.Entry(32)
> > + lvnamelabel.set_mnemonic_widget(lvnameentry)
> > + if lv['name']:
> > + lvnameentry.set_text(lv['name'])
> > + else:
> > + lvnameentry.set_text(self.storage.createSuggestedLVName(self.getTempVG()))
> > +
> > + # Logical Volume size label & entry
> > + lvsizelabel = createAlignedLabel(_("_Size (MB):"))
> > + lvsizeentry = gtk.Entry(16)
> > + lvsizelabel.set_mnemonic_widget(lvsizeentry)
> > + lvsizeentry.set_text("%Ld" % lv['size'])
> > +
> > + # Maximum size label
> > + maxsizelabel = createAlignedLabel(_("(Max size is %s MB)") %
> > + min(lvm.getMaxLVSize(), lv['size'] + tempvg.freeSpace))
> > +
> > else:
> > - maintable.attach(createAlignedLabel(_("Original File System Type:")),
> > - 0, 1, row, row + 1)
> > + # File system type lable & combo
> > + newfstypelabel = createAlignedLabel(_("Original File System Type:"))
> > if format.type:
> > newfstypeCombo = gtk.Label(format.name)
> > else:
> > newfstypeCombo = gtk.Label(_("Unknown"))
> >
> > - maintable.attach(newfstypeCombo, 1, 2, row, row + 1)
> > - row += 1
> > -
> > + # File system label label & combo
> > if getattr(format, "label", None):
> > - maintable.attach(createAlignedLabel(_("Original File System "
> > - "Label:")),
> > - 0, 1, row, row + 1)
> > - maintable.attach(gtk.Label(format.label),
> > - 1, 2, row, row + 1)
> > - row += 1
> > + newfslabellabel = createAlignedLabel(_("Original File System "
> > + "Label:"))
> > + newfslableCombo = gtk.Label(format.label)
> >
> > - if not lv['exists']:
> > - lbl = createAlignedLabel(_("_Logical Volume Name:"))
> > - lvnameEntry = gtk.Entry(32)
> > - lbl.set_mnemonic_widget(lvnameEntry)
> > - if lv['name']:
> > - lvnameEntry.set_text(lv['name'])
> > - else:
> > - lvnameEntry.set_text(self.storage.createSuggestedLVName(self.getTempVG()))
> > - else:
> > - lbl = createAlignedLabel(_("Logical Volume Name:"))
> > - lvnameEntry = gtk.Label(lv['name'])
> > + # Logical Volume name label & entry
> > + lvnamelabel = createAlignedLabel(_("Logical Volume Name:"))
> > + lvnameentry = gtk.Label(lv['name'])
> >
> > - maintable.attach(lbl, 0, 1, row, row + 1)
> > - maintable.attach(lvnameEntry, 1, 2, row, row + 1)
> > + # Logical Volume size label & entry
> > + lvsizelabel = createAlignedLabel(_("Size (MB):"))
> > + lvsizeentry = gtk.Label(str(lv['size']))
> > +
> > +
> > + # Here is where the gtk crap begins.
> > + dialog = gtk.Dialog(tstr, self.parent)
> > + gui.addFrame(dialog)
> > + dialog.add_button('gtk-cancel', 2)
> > + dialog.add_button('gtk-ok', 1)
> > + dialog.set_position(gtk.WIN_POS_CENTER)
> > +
> > + # Initialize main table
> > + maintable = gtk.Table()
> > + maintable.set_row_spacings(5)
> > + maintable.set_col_spacings(5)
> > + row = 0
> > +
> > + # Add the mountCombo that we previously created
> > + lbl = createAlignedLabel(_("_Mount Point:"))
> > + maintable.attach(lbl, 0, 1, row,row+1)
> > + lbl.set_mnemonic_widget(mountCombo)
> > + maintable.attach(mountCombo, 1, 2, row, row + 1)
> > row += 1
> >
> > - if not lv['exists']:
> > - lbl = createAlignedLabel(_("_Size (MB):"))
> > - sizeEntry = gtk.Entry(16)
> > - lbl.set_mnemonic_widget(sizeEntry)
> > - sizeEntry.set_text("%Ld" % lv['size'])
> > - else:
> > - lbl = createAlignedLabel(_("Size (MB):"))
> > - sizeEntry = gtk.Label(str(lv['size']))
> > + # Add the filesystem combo labels.
> > + maintable.attach(newfstypelabel, 0, 1, row, row + 1)
> > + maintable.attach(newfstypeCombo, 1, 2, row, row + 1)
> > + row += 1
> > +
> > + # If there is a File system lable, add it.
> > + if newfslabellabel is not None and newfslableCombo is not None:
> > + maintable.attach(newfslabellabel, 0, 1, row, row + 1)
> > + maintable.attach(newfslableCombo, 1, 2, row, row + 1)
> > + row += 1
> >
> > - maintable.attach(lbl, 0, 1, row, row+1)
> > - maintable.attach(sizeEntry, 1, 2, row, row + 1)
> > + # Add the logical volume name
> > + maintable.attach(lvnamelabel, 0, 1, row, row + 1)
> > + maintable.attach(lvnameentry, 1, 2, row, row + 1)
> > row += 1
> >
> > - if not lv['exists']:
> > - maxlv = min(lvm.getMaxLVSize(), lv['size'] + tempvg.freeSpace)
> > - maxlabel = createAlignedLabel(_("(Max size is %s MB)") % (maxlv,))
> > - maintable.attach(maxlabel, 1, 2, row, row + 1)
> > + # Add the logical volume size
> > + maintable.attach(lvsizelabel, 0, 1, row, row + 1)
> > + maintable.attach(lvsizeentry, 1, 2, row, row + 1)
> > + row += 1
> > +
> > + # If there is a maxsize, add it.
> > + if maxsizelabel is not None:
> > + maintable.attach(maxsizelabel, 1, 2, row, row + 1)
> >
> > self.fsoptionsDict = {}
> > if lv['exists']:
> > @@ -487,7 +528,10 @@ class VolumeGroupEditor:
> > templuks = None
> > break
> >
> > - (row, self.fsoptionsDict) = createPreExistFSOptionSection(reallv, maintable, row, mountCombo, self.storage, ignorefs = ["software RAID", "physical volume (LVM)", "vfat"], luksdev=templuks)
> > + (row, self.fsoptionsDict) = createPreExistFSOptionSection(reallv,
> > + maintable, row, mountCombo, self.storage,
> > + ignorefs = ["software RAID", "physical volume (LVM)", "vfat"],
> > + luksdev=templuks)
> >
> > # checkbutton for encryption using dm-crypt/LUKS
> > if not lv['exists']:
> > @@ -532,7 +576,7 @@ class VolumeGroupEditor:
> > mountpoint = mountCombo.get_children()[0].get_text().strip()
> >
> > # validate logical volume name
> > - lvname = lvnameEntry.get_text().strip()
> > + lvname = lvnameentry.get_text().strip()
> > if not templv.exists:
> > err = sanityCheckLogicalVolumeName(lvname)
> > if err:
> > --
> > 1.6.0.6
> >
> > _______________________________________________
> > Anaconda-devel-list mailing list
> > Anaconda-devel-list@xxxxxxxxxx
> > https://www.redhat.com/mailman/listinfo/anaconda-devel-list
>
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
