Subject: Clean up FileChooser usage a bit From: Cole Robinson crobinso@redhat.com Tue Dec 13 15:09:35 2022 -0500 Date: Wed Dec 14 12:31:17 2022 -0500: Git: cbc5b897077671a675faf48603d9714527d84c83 * 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 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 #