Michal Novotny wrote:
> Hi,
> I found a typo in my patch which prevented remembering last used
> directories being
> remembered correctly. Also, 'is_media' variable have been changed to
> 'browse_reason'
> which is the argument passed directly to util.browse_local() function so
> please review.
>
Comments below.
> # HG changeset patch
> # User Michal Novotny <minovotn@xxxxxxxxxx>
> # Date 1245935008 -7200
> # Node ID 39b81ca60edc70066cc44cf760865c2d8f03ac8f
> # Parent aa4f30fce78b6805c94c70729139bb326b023bcb
> Fix typo in remember-paths and change 'is_media' to 'browse_reason'
>
> This is the patch that fixes a bug introduced by my previous patch for
> remembering last used paths in rev. aa4f30fce78b. Also, 'is_media'
> variable is no longer used there and is replaced by 'browse_reason'
> wherever applicable.
>
> diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/addhardware.py
> --- a/src/virtManager/addhardware.py Tue Jun 23 19:30:12 2009 -0400
> +++ b/src/virtManager/addhardware.py Thu Jun 25 15:03:28 2009 +0200
> @@ -678,9 +678,8 @@
>
> def browse_storage_file_address(self, src, ignore=None):
> textent = self.window.get_widget("storage-file-address")
> - folder = self.config.get_default_image_dir(self.vm.get_connection())
> filename = self._browse_file(_("Locate or Create New Storage File"),
> - textent, folder=folder,
> + textent, folder=None,
> confirm_overwrite=True)
> if filename != None:
> textent.set_text(filename)
> @@ -699,17 +698,23 @@
> if path:
> textent.set_text(path)
>
> + if folder == None:
> + reason = self.config.CONFIG_DIR_IMAGE
> + else:
> + reason = None
> +
> conn = self.vm.get_connection()
> if self.storage_browser == None:
> - self.storage_browser = vmmStorageBrowser(self.config, conn, False)
> + self.storage_browser = vmmStorageBrowser(self.config, conn,
> + reason)
There doesn't need to be any reason argument here, because (see below)
> if self._browse_cb_id:
> self.storage_browser.disconnect(self._browse_cb_id)
>
> self._browse_cb_id = self.storage_browser.connect("storage-browse-finish", set_storage_cb)
> self.storage_browser.local_args = { "dialog_name": dialog_name,
> "confirm_func": confirm_func,
> - "browse_reason":
> - self.config.CONFIG_DIR_IMAGE }
> + "browse_reason": reason,
> + "start_folder": folder }
The user specifies it in local_args here. These local_args are what is
passed straight through to the local browser. The reason/is_media arg
can be completely dropped from StorageBrowser: just have the caller
always pass the appropriate browse_reason in local_args.
> self.storage_browser.show(conn)
> return None
>
> diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/choosecd.py
> --- a/src/virtManager/choosecd.py Tue Jun 23 19:30:12 2009 -0400
> +++ b/src/virtManager/choosecd.py Thu Jun 25 15:03:28 2009 +0200
> @@ -150,7 +150,7 @@
> def _browse_file(self, dialog_name):
> if self.storage_browser == None:
> self.storage_browser = vmmStorageBrowser(self.config, self.conn,
> - True)
> + self.config.CONFIG_DIR_MEDIA)
> self.storage_browser.connect("storage-browse-finish",
> self.set_storage_path)
> self.storage_browser.local_args = { "dialog_name": dialog_name,
> diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/create.py
> --- a/src/virtManager/create.py Tue Jun 23 19:30:12 2009 -0400
> +++ b/src/virtManager/create.py Thu Jun 25 15:03:28 2009 +0200
> @@ -997,7 +997,7 @@
> def browse_iso(self, ignore1=None, ignore2=None):
> self._browse_file(_("Locate ISO Image"),
> self.set_iso_storage_path,
> - is_media=True)
> + browse_reason=self.config.CONFIG_DIR_MEDIA)
> self.window.get_widget("install-local-box").activate()
>
> def toggle_enable_storage(self, src):
> @@ -1006,7 +1006,7 @@
> def browse_storage(self, ignore1):
> self._browse_file(_("Locate existing storage"),
> self.set_disk_storage_path,
> - is_media=False)
> + browse_reason=self.config.CONFIG_DIR_IMAGE)
>
> def toggle_storage_select(self, src):
> act = src.get_active()
> @@ -1650,20 +1650,16 @@
> logging.exception("Error detecting distro.")
> self.detectedDistro = (None, None)
>
> - def _browse_file(self, dialog_name, callback, folder=None, is_media=False):
> + def _browse_file(self, dialog_name, callback, folder=None, browse_reason=False):
> if self.storage_browser == None:
> self.storage_browser = vmmStorageBrowser(self.config, self.conn,
> - is_media)
> + browse_reason)
> self.storage_browser.connect("storage-browse-finish",
> callback)
> - if is_media:
> - reason = self.config.CONFIG_DIR_MEDIA
> - else:
> - reason = self.config.CONFIG_DIR_IMAGE
>
> self.storage_browser.local_args = { "dialog_name": dialog_name,
> "start_folder": folder,
> - "browse_reason": reason}
> + "browse_reason": browse_reason}
> self.storage_browser.show(self.conn)
>
> def show_help(self, ignore):
> diff -r aa4f30fce78b -r 39b81ca60edc src/virtManager/storagebrowse.py
> --- a/src/virtManager/storagebrowse.py Tue Jun 23 19:30:12 2009 -0400
> +++ b/src/virtManager/storagebrowse.py Thu Jun 25 15:03:28 2009 +0200
> @@ -38,7 +38,7 @@
> gobject.TYPE_NONE, [str]),
> }
>
> - def __init__(self, config, conn, is_media=False):
> + def __init__(self, config, conn, browse_reason=False):
> self.__gobject_init__()
> self.window = gtk.glade.XML(config.get_glade_dir() + \
> "/vmm-storage-browse.glade",
> @@ -58,14 +58,9 @@
> # Add Volume wizard
> self.addvol = None
>
> - if is_media:
> - reason = self.config.CONFIG_DIR_MEDIA
> - else:
> - reason = self.config.CONFIG_DIR_IMAGE
> -
> # Arguments to pass to util.browse_local for local storage
> self.local_args = {"dialog_name": _("Choose local storage"),
> - "browse_reason": reason, }
> + "browse_reason": browse_reason }
>
All of this browse_reason handling can go away as a result
The rest looks fine.
Thanks,
Cole
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
