Clean up FileChooser usage a bit

* Move browse_reason handling entirely into storagebrowser.py
* Open code some of the browse_local logic at the few callers

Signed-off-by: Cole Robinson <crobinso@redhat.com>

10 files changed, 117 insertions, 141 deletions

diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
index a1dd3261..df902374 100644
--- a/virtManager/addhardware.py
+++ b/virtManager/addhardware.py
@@ -1590,8 +1590,8 @@ class vmmAddHardware(vmmGObjectUI):
                 textent.set_text(path)
 
         reason = (isdir and
-                  self.config.CONFIG_DIR_FS or
-                  self.config.CONFIG_DIR_IMAGE)
+                  vmmStorageBrowser.REASON_FS or
+                  vmmStorageBrowser.REASON_IMAGE)
         if self._storagebrowser is None:
             self._storagebrowser = vmmStorageBrowser(self.conn)
 
diff --git a/virtManager/config.py b/virtManager/config.py
index 8379697c..2c81b061 100644
--- a/virtManager/config.py
+++ b/virtManager/config.py
@@ -133,51 +133,6 @@ class _SettingsWrapper(object):
 
 
 class vmmConfig(object):
-    # key names for saving last used paths
-    CONFIG_DIR_IMAGE = "image"
-    CONFIG_DIR_ISO_MEDIA = "isomedia"
-    CONFIG_DIR_FLOPPY_MEDIA = "floppymedia"
-    CONFIG_DIR_SCREENSHOT = "screenshot"
-    CONFIG_DIR_FS = "fs"
-
-    # Metadata mapping for browse types. Prob shouldn't go here, but works
-    # for now.
-    browse_reason_data = {
-        CONFIG_DIR_IMAGE: {
-            "enable_create":  True,
-            "storage_title":  _("Locate or create storage volume"),
-            "local_title":    _("Locate existing storage"),
-            "dialog_type":    Gtk.FileChooserAction.SAVE,
-            "choose_button":  Gtk.STOCK_OPEN,
-            "gsettings_key": "image",
-        },
-
-        CONFIG_DIR_SCREENSHOT: {
-            "gsettings_key": "screenshot",
-        },
-
-        CONFIG_DIR_ISO_MEDIA: {
-            "enable_create":  False,
-            "storage_title":  _("Locate ISO media volume"),
-            "local_title":    _("Locate ISO media"),
-            "gsettings_key": "media",
-        },
-
-        CONFIG_DIR_FLOPPY_MEDIA: {
-            "enable_create":  False,
-            "storage_title":  _("Locate floppy media volume"),
-            "local_title":    _("Locate floppy media"),
-            "gsettings_key": "media",
-        },
-
-        CONFIG_DIR_FS: {
-            "enable_create":  False,
-            "storage_title":  _("Locate directory volume"),
-            "local_title":    _("Locate directory volume"),
-            "dialog_type":    Gtk.FileChooserAction.SELECT_FOLDER,
-        },
-    }
-
     CONSOLE_SCALE_NEVER = 0
     CONSOLE_SCALE_FULLSCREEN = 1
     CONSOLE_SCALE_ALWAYS = 2
@@ -627,23 +582,15 @@ class vmmConfig(object):
 
 
     # Default directory location dealings
-    def get_default_directory(self, conn, _type):
-        ignore = conn
-        browsedata = self.browse_reason_data.get(_type, {})
-        key = browsedata.get("gsettings_key", None)
-        path = None
-
-        if key:
-            path = self.conf.get("/paths/%s-default" % key)
-
-        log.debug("directory for type=%s returning=%s", _type, path)
+    def get_default_directory(self, gsettings_key):
+        path = self.conf.get("/paths/%s-default" % gsettings_key)
+        log.debug("directory for gsettings_key=%s returning=%s",
+                  gsettings_key, path)
         return path
 
-    def set_default_directory(self, folder, _type):
-        browsedata = self.browse_reason_data.get(_type, {})
-        key = browsedata.get("gsettings_key", None)
-        if not key:
+    def set_default_directory(self, gsettings_key, folder):
+        if not folder or folder.startswith("/dev"):
             return  # pragma: no cover
-
-        log.debug("saving directory for type=%s to %s", key, folder)
-        self.conf.set("/paths/%s-default" % key, folder)
+        log.debug("saving directory for gsettings_key=%s to %s",
+                  gsettings_key, folder)
+        self.conf.set("/paths/%s-default" % gsettings_key, folder)
diff --git a/virtManager/createpool.py b/virtManager/createpool.py
index 66457b51..a3e9a99a 100644
--- a/virtManager/createpool.py
+++ b/virtManager/createpool.py
@@ -381,9 +381,8 @@ class vmmCreatePool(vmmGObjectUI):
         self._show_options_by_pool()
 
     def _browse_source_cb(self, src):
-        source = self.err.browse_local(self.conn,
+        source = self.err.browse_local(
                 _("Choose source path"),
-                dialog_type=Gtk.FileChooserAction.OPEN,
                 start_folder="/dev")
         if source:
             self.widget("pool-source-path").get_child().set_text(source)
@@ -394,7 +393,7 @@ class vmmCreatePool(vmmGObjectUI):
         if current:
             startfolder = os.path.dirname(current)
 
-        target = self.err.browse_local(self.conn,
+        target = self.err.browse_local(
                 _("Choose target directory"),
                 dialog_type=Gtk.FileChooserAction.SELECT_FOLDER,
                 start_folder=startfolder)
diff --git a/virtManager/createvm.py b/virtManager/createvm.py
index 7e5ded68..95aff71b 100644
--- a/virtManager/createvm.py
+++ b/virtManager/createvm.py
@@ -1280,11 +1280,11 @@ class vmmCreateVM(vmmGObjectUI):
 
     def _browse_file(self, cbwidget, cb=None, is_media=False, is_dir=False):
         if is_media:
-            reason = self.config.CONFIG_DIR_ISO_MEDIA
+            reason = vmmStorageBrowser.REASON_ISO_MEDIA
         elif is_dir:
-            reason = self.config.CONFIG_DIR_FS
+            reason = vmmStorageBrowser.REASON_FS
         else:
-            reason = self.config.CONFIG_DIR_IMAGE
+            reason = vmmStorageBrowser.REASON_IMAGE
 
         if cb:
             callback = cb
diff --git a/virtManager/createvol.py b/virtManager/createvol.py
index 58453038..ea82964a 100644
--- a/virtManager/createvol.py
+++ b/virtManager/createvol.py
@@ -208,7 +208,7 @@ class vmmCreateVolume(vmmGObjectUI):
             self._storage_browser.set_finish_cb(cb)
             self._storage_browser.topwin.set_modal(self.topwin.get_modal())
             self._storage_browser.set_browse_reason(
-                self.config.CONFIG_DIR_IMAGE)
+                vmmStorageBrowser.REASON_IMAGE)
 
         self._storage_browser.show(self.topwin)
 
diff --git a/virtManager/details/details.py b/virtManager/details/details.py
index 757e18ad..1970d0aa 100644
--- a/virtManager/details/details.py
+++ b/virtManager/details/details.py
@@ -1089,7 +1089,7 @@ class vmmDetails(vmmGObjectUI):
 
     def _browse_file(self, callback, reason=None):
         if not reason:
-            reason = self.config.CONFIG_DIR_IMAGE
+            reason = vmmStorageBrowser.REASON_IMAGE
 
         if self.storage_browser is None:
             self.storage_browser = vmmStorageBrowser(self.conn)
@@ -1235,9 +1235,9 @@ class vmmDetails(vmmGObjectUI):
     def _disk_source_browse_clicked_cb(self, src):
         disk = self._get_hw_row()[HW_LIST_COL_DEVICE]
         if disk.is_floppy():
-            reason = self.config.CONFIG_DIR_FLOPPY_MEDIA
+            reason = vmmStorageBrowser.REASON_FLOPPY_MEDIA
         else:
-            reason = self.config.CONFIG_DIR_ISO_MEDIA
+            reason = vmmStorageBrowser.REASON_ISO_MEDIA
 
         def cb(ignore, path):
             self._mediacombo.set_path(path)
diff --git a/virtManager/device/fsdetails.py b/virtManager/device/fsdetails.py
index b9956e1d..555c745a 100644
--- a/virtManager/device/fsdetails.py
+++ b/virtManager/device/fsdetails.py
@@ -268,8 +268,8 @@ class vmmFSDetails(vmmGObjectUI):
                 textent.set_text(path)
 
         reason = (isdir and
-                  self.config.CONFIG_DIR_FS or
-                  self.config.CONFIG_DIR_IMAGE)
+                  vmmStorageBrowser.REASON_FS or
+                  vmmStorageBrowser.REASON_IMAGE)
 
         if self._storage_browser is None:
             self._storage_browser = vmmStorageBrowser(self.conn)
diff --git a/virtManager/error.py b/virtManager/error.py
index 8d78efae..593c89ca 100644
--- a/virtManager/error.py
+++ b/virtManager/error.py
@@ -3,6 +3,7 @@
 # This work is licensed under the GNU GPLv2 or later.
 # See the COPYING file in the top-level directory.
 
+import os
 import sys
 import textwrap
 import traceback
@@ -231,49 +232,38 @@ class vmmErrorDialog(vmmGObject):
 
         return response
 
-    def browse_local(self, conn, dialog_name, start_folder=None,
+    def browse_local(self, dialog_name, start_folder=None,
                      _type=None, dialog_type=None,
-                     browse_reason=None,
-                     choose_button=None, default_name=None):
+                     choose_button=None, default_name=None,
+                     confirm_overwrite=False):
         """
         Helper function for launching a filechooser
 
         @dialog_name: String to use in the title bar of the filechooser.
-        @conn: vmmConnection used by calling class
         @start_folder: Folder the filechooser is viewing at startup
         @_type: File extension to filter by (e.g. "iso", "png")
         @dialog_type: Maps to FileChooserDialog 'action'
-        @browse_reason: The vmmConfig.CONFIG_DIR* reason we are browsing.
-            If set, this will override the 'folder' parameter with the gsettings
-            value, and store the user chosen path.
         """
-        import os
-
-        # Initial setup
-        overwrite_confirm = False
-        dialog_type = dialog_type or Gtk.FileChooserAction.OPEN
-
-        if dialog_type == Gtk.FileChooserAction.SAVE:
-            if choose_button is None:
-                choose_button = Gtk.STOCK_SAVE
-                overwrite_confirm = True
-
+        if dialog_type is None:
+            dialog_type = Gtk.FileChooserAction.OPEN
         if choose_button is None:
             choose_button = Gtk.STOCK_OPEN
 
+        buttons = (Gtk.STOCK_CANCEL,
+                   Gtk.ResponseType.CANCEL,
+                   choose_button,
+                   Gtk.ResponseType.ACCEPT)
+
         fcdialog = Gtk.FileChooserDialog(title=dialog_name,
                                     parent=self.get_parent(),
                                     action=dialog_type,
-                                    buttons=(Gtk.STOCK_CANCEL,
-                                             Gtk.ResponseType.CANCEL,
-                                             choose_button,
-                                             Gtk.ResponseType.ACCEPT))
+                                    buttons=buttons)
         fcdialog.set_default_response(Gtk.ResponseType.ACCEPT)
 
         if default_name:
             fcdialog.set_current_name(default_name)
 
-        fcdialog.set_do_overwrite_confirmation(overwrite_confirm)
+        fcdialog.set_do_overwrite_confirmation(confirm_overwrite)
 
         # Set file match pattern (ex. *.png)
         if _type is not None:
@@ -289,11 +279,6 @@ class vmmErrorDialog(vmmGObject):
                 f.set_name(name)
             fcdialog.set_filter(f)
 
-        # Set initial dialog folder
-        if browse_reason:
-            start_folder = self.config.get_default_directory(
-                conn, browse_reason)
-
         if start_folder is not None:
             if os.access(start_folder, os.R_OK):
                 fcdialog.set_current_folder(start_folder)
@@ -304,10 +289,6 @@ class vmmErrorDialog(vmmGObject):
             ret = fcdialog.get_filename()
         fcdialog.destroy()
 
-        # Store the chosen directory in gsettings if necessary
-        if ret and browse_reason and not ret.startswith("/dev"):
-            self.config.set_default_directory(
-                os.path.dirname(ret), browse_reason)
         return ret
 
 
diff --git a/virtManager/storagebrowse.py b/virtManager/storagebrowse.py
index b5fa9a2e..c5a26519 100644
--- a/virtManager/storagebrowse.py
+++ b/virtManager/storagebrowse.py
@@ -4,6 +4,10 @@
 # This work is licensed under the GNU GPLv2 or later.
 # See the COPYING file in the top-level directory.
 
+import os
+
+from gi.repository import Gtk
+
 from virtinst import log
 
 from .lib import uiutil
@@ -11,15 +15,53 @@ from .baseclass import vmmGObjectUI
 from .hoststorage import vmmHostStorage
 
 
+class _BrowseReasonMetadata:
+    def __init__(self, browse_reason):
+        self.enable_create = False
+        self.storage_title = None
+        self.local_title = None
+        self.gsettings_key = None
+        self.dialog_type = None
+
+        if browse_reason == vmmStorageBrowser.REASON_IMAGE:
+            self.enable_create = True
+            self.local_title = _("Locate existing storage")
+            self.storage_title = _("Locate or create storage volume")
+            self.dialog_type = Gtk.FileChooserAction.SAVE
+            self.gsettings_key = "image"
+
+        if browse_reason == vmmStorageBrowser.REASON_ISO_MEDIA:
+            self.local_title = _("Locate ISO media")
+            self.storage_title = _("Locate ISO media volume")
+            self.gsettings_key = "media"
+
+        if browse_reason == vmmStorageBrowser.REASON_FLOPPY_MEDIA:
+            self.local_title = _("Locate floppy media")
+            self.storage_title = _("Locate floppy media volume")
+            self.gsettings_key = "media"
+
+        if browse_reason == vmmStorageBrowser.REASON_FS:
+            self.local_title = _("Locate directory volume")
+            self.storage_title = _("Locate directory volume")
+            self.dialog_type = Gtk.FileChooserAction.SELECT_FOLDER
+
+        if browse_reason is None:
+            self.enable_create = True
+            self.storage_title = _("Choose Storage Volume")
+
+
 class vmmStorageBrowser(vmmGObjectUI):
+    REASON_IMAGE = "image"
+    REASON_ISO_MEDIA = "isomedia"
+    REASON_FLOPPY_MEDIA = "floppymedia"
+    REASON_FS = "fs"
+
     def __init__(self, conn):
         vmmGObjectUI.__init__(self, "storagebrowse.ui", "vmm-storage-browse")
         self.conn = conn
 
         self._first_run = False
         self._finish_cb = None
-
-        # Passed to browse_local
         self._browse_reason = None
 
         self.storagelist = vmmHostStorage(self.conn, self.builder, self.topwin,
@@ -103,15 +145,10 @@ class vmmStorageBrowser(vmmGObjectUI):
 
     def set_browse_reason(self, reason):
         self._browse_reason = reason
-        data = self.config.browse_reason_data.get(self._browse_reason, {})
-        allow_create = True
-        title = _("Choose Storage Volume")
-        if data:
-            allow_create = data["enable_create"]
-            title = data["storage_title"]
+        data = _BrowseReasonMetadata(self._browse_reason)
 
-        self.topwin.set_title(title)
-        self.storagelist.widget("vol-add").set_sensitive(allow_create)
+        self.topwin.set_title(data.storage_title)
+        self.storagelist.widget("vol-add").set_sensitive(data.enable_create)
 
 
     #############
@@ -128,7 +165,7 @@ class vmmStorageBrowser(vmmGObjectUI):
         self._finish(volume.get_target_path())
 
     def _vol_sensitive_cb(self, fmt):
-        if ((self._browse_reason == self.config.CONFIG_DIR_FS) and
+        if ((self._browse_reason == vmmStorageBrowser.REASON_FS) and
             fmt != 'dir'):
             return False
         return True
@@ -139,22 +176,27 @@ class vmmStorageBrowser(vmmGObjectUI):
     ####################
 
     def _browse_local(self):
-        dialog_type = None
-        dialog_name = None
-        choose_button = None
-
-        data = self.config.browse_reason_data.get(self._browse_reason)
-        if data:
-            dialog_name = data["local_title"] or None
-            dialog_type = data.get("dialog_type")
-            choose_button = data.get("choose_button")
-
-        filename = self.err.browse_local(self.conn,
-            dialog_type=dialog_type, browse_reason=self._browse_reason,
-            dialog_name=dialog_name, choose_button=choose_button)
-        if filename:
-            log.debug("Browse local chose path=%s", filename)
-            self._finish(filename)
+        data = _BrowseReasonMetadata(self._browse_reason)
+        gsettings_key = data.gsettings_key
+
+        if gsettings_key:
+            start_folder = self.config.get_default_directory(gsettings_key)
+
+        filename = self.err.browse_local(
+            dialog_type=data.dialog_type,
+            dialog_name=data.local_title,
+            start_folder=start_folder)
+
+        if not filename:
+            return
+
+        log.debug("Browse local chose path=%s", filename)
+
+        if gsettings_key:
+            self.config.set_default_directory(
+                gsettings_key, os.path.dirname(filename))
+
+        self._finish(filename)
 
     def _finish(self, path):
         if self._finish_cb:
diff --git a/virtManager/vmwindow.py b/virtManager/vmwindow.py
index 3ac4a6a4..d5549454 100644
--- a/virtManager/vmwindow.py
+++ b/virtManager/vmwindow.py
@@ -548,24 +548,31 @@ class vmmVMWindow(vmmGObjectUI):
             ret = ret.buffer  # pragma: no cover
 
         import datetime
+        import os
         now = str(datetime.datetime.now()).split(".")[0].replace(" ", "_")
         default = "Screenshot_%s_%s.png" % (self.vm.get_name(), now)
 
-        path = self.err.browse_local(
-            self.vm.conn, _("Save Virtual Machine Screenshot"),
+        start_folder = self.config.get_default_directory("screenshot")
+
+        filename = self.err.browse_local(
+            _("Save Virtual Machine Screenshot"),
             _type=("png", _("PNG files")),
             dialog_type=Gtk.FileChooserAction.SAVE,
-            browse_reason=self.config.CONFIG_DIR_SCREENSHOT,
-            default_name=default)
-        if not path:  # pragma: no cover
+            choose_button=Gtk.STOCK_SAVE,
+            start_folder=start_folder,
+            default_name=default,
+            confirm_overwrite=True)
+        if not filename:  # pragma: no cover
             log.debug("No screenshot path given, skipping save.")
             return
 
-        filename = path
         if not filename.endswith(".png"):
             filename += ".png"  # pragma: no cover
         open(filename, "wb").write(ret)
 
+        self.config.set_default_directory(
+                "screenshot", os.path.dirname(filename))
+
 
     ########################
     # Details page refresh #