On Thu, Apr 09, 2009 at 12:14:27PM -0500, David Lehman wrote:
> 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.
Yep, your correct. will push then.
>
> 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
--
Joel Andres Granados
Brno, Czech Republic, Red Hat.
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
