[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [et-mgmt-tools] [PATCH] virt-manager: Make virt-manager remember last image directory



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 redhat com>
> # 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]