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

[Cole Robinson]

Michal Novotny wrote:

<snip>

>>>
>>> diff -r a996e00b3bb6 -r b599a65ca061 src/virtManager/config.py
>>> --- a/src/virtManager/config.py	Thu Jun 18 10:54:21 2009 -0400
>>> +++ b/src/virtManager/config.py	Thu Jun 18 18:16:21 2009 +0200
>>> @@ -261,6 +261,30 @@
>>>           self.conf.set_bool(self.conf_dir + "/vmlist-fields/network_traffic", state)
>>>
>>>
>>> +    def get_default_directory(self, conn, _type):
>>> +        if _type not in ["image", "media", "save", "restore", "screenshot"]:
>>> +            logging.error("Unknown type for get_default_directory: %s" % _type)
>>> +            return
>>> +
>>> +        try:
>>> +            path = self.conf.get_value( self.conf_dir + "/paths/default-%s-path" % _type )
>>> +        except:
>>> +            path = None
>>> +
>>> +        if not path or len(path) == 0:
>>> +            if _type == "image" or _type == "media":
>>> +                path = self.get_default_image_dir(conn)
>>> +            if _type == "save":
>>> +                path = self.get_default_save_dir(conn)
>>> +
>>> +        return path
>>> +
>>> +    def set_default_directory(self, folder, _type):
>>> +        if _type not in ["image", "media", "save", "restore", "screenshot"]:
>>> +            logging.error("Unknown type for set_default_directory: %s" % _type)
>>> +            return
>>> +
>>> +        self.conf.set_value( self.conf_dir + "/paths/default-%s-path" % _type, folder)
>>>
>>>      
>> Rather than have the user pass a string in like "image", "media", ..., how
>> about defining some constants in the config class:
>>
>> CONFIG_DIR_IMAGE = 1
>> CONFIG_DIR_SAVE  = 2
>> ...
>>
>> (They can even be the string values you use above anyways). Then have a
>> separate function which converts that to the requested gconf path. This way
>> you don't have to duplicate the ["image", "media", ...] whitelist in two
>> places, and I think will make the calling code more readable.
>>
>>    
> Well, the idea is good. I know what you mean and I'll use string values 
> there to preserve most of those codes.
>> Also, to simplify a lot of the code, you could push down the gconf fetching/
>> storing into util.browse_local: add an argument which takes one of the above
>> CONFIG_DIR values, sets the start folder, and then records the directory that
>> the user chooses. That way, all callers would need to pass one value, and
>> wouldn't need to play with the result.
>>    
> Well, I don't know what do you mean by that. Do you mean to import gconf 
> directly into util.py and directly call gconf in browse_local? You wrote 
> something about altering util.browse_local() ... could you provide me 
> the prototype you mean?

Sure, here's an untested patch:

> diff -r 2e0e047d21f0 src/virtManager/util.py
> --- a/src/virtManager/util.py	Mon Jun 22 14:24:09 2009 -0400
> +++ b/src/virtManager/util.py	Mon Jun 22 14:43:11 2009 -0400
> @@ -90,25 +90,33 @@
>      return ret
>  
>  
> -def browse_local(parent, dialog_name, start_folder=None, _type=None,
> -                 dialog_type=gtk.FILE_CHOOSER_ACTION_OPEN, confirm_func=None):
> +def browse_local(parent, dialog_name, config, conn, start_folder=None,
> +                 _type=None, dialog_type=gtk.FILE_CHOOSER_ACTION_OPEN,
> +                 confirm_func=None, browse_reason=None):
>      """
>      Helper function for launching a filechooser
>  
>      @param parent: Parent window for the filechooser
> +    @param config: vmmConfig used by the calling class
> +    @param conn: vmmConnection used by the calling class
>      @param dialog_name: String to use in the title bar of the filechooser.
>      @param start_folder: Folder the filechooser is viewing at startup
>      @param _type: File extension to filter by (e.g. "iso", "png")
>      @param dialog_type: Maps to FileChooserDialog 'action'
>      @param confirm_func: Optional callback function if file is chosen.
> +    @param browse_reason: The vmmConfig.CONFIG_DIR* reason we are browsing.
> +        If set, this will override the 'folder' parameter with the gconf
> +        value, and store the user chosen path.
>      """
> -
>      overwrite_confirm = False
>      choose_button = gtk.STOCK_OPEN
>      if dialog_type == gtk.FILE_CHOOSER_ACTION_SAVE:
>          choose_button = gtk.STOCK_SAVE
>          overwrite_confirm = True
>  
> +    if browse_reason:
> +        start_folder = config.get_default_directory(conn, browse_reason)
> +
>      fcdialog = gtk.FileChooserDialog(dialog_name, parent,
>                                       dialog_type,
>                                       (gtk.STOCK_CANCEL, gtk.RESPONSE_CANCEL,
> @@ -142,10 +150,15 @@
>      if(response == gtk.RESPONSE_ACCEPT):
>          filename = fcdialog.get_filename()
>          fcdialog.destroy()
> -        return filename
> +        ret = filename
>      else:
>          fcdialog.destroy()
> -        return None
> +        ret = None
> +
> +    if ret and browse_reason:
> +        config.set_default_directory(os.path.dirname(ret), browse_reason)
> +
> +    return ret
>  
>  def dup_conn(config, conn, libconn=None, return_conn_class=False):
>  

This way, any caller of 'util.browse_local' doesn't need to store the
dirname in gconf: browse_local will do it for us. It sucks that we have
to pass conn and config to the helper, but that info is typically easy
to access from the callers, so shouldn't be a big change.

Thanks,
Cole