Review: Needs Fixing I've added some inline comments for some things I think need to be fixed up. Mostly small stuff.
Diff comments: > > === added file 'openlp/core/ui/lib/filedialogpatches.py' > --- openlp/core/ui/lib/filedialogpatches.py 1970-01-01 00:00:00 +0000 > +++ openlp/core/ui/lib/filedialogpatches.py 2017-08-04 22:00:43 +0000 > @@ -0,0 +1,118 @@ > +# -*- coding: utf-8 -*- > +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 > softtabstop=4 > + > +############################################################################### > +# OpenLP - Open Source Lyrics Projection > # > +# > --------------------------------------------------------------------------- # > +# Copyright (c) 2008-2017 OpenLP Developers > # > +# > --------------------------------------------------------------------------- # > +# This program is free software; you can redistribute it and/or modify it > # > +# under the terms of the GNU General Public License as published by the Free > # > +# Software Foundation; version 2 of the License. > # > +# > # > +# This program is distributed in the hope that it will be useful, but > WITHOUT # > +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > # > +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > # > +# more details. > # > +# > # > +# You should have received a copy of the GNU General Public License along > # > +# with this program; if not, write to the Free Software Foundation, Inc., 59 > # > +# Temple Place, Suite 330, Boston, MA 02111-1307 USA > # > +############################################################################### > +""" Patch the QFileDialog so it accepts and returns Path objects""" > +from functools import wraps > +from pathlib import Path > + > +from PyQt5 import QtWidgets > + > +from openlp.core.common.path import path_to_str, str_to_path > +from openlp.core.lib import replace_params > + > + > +class PQFileDialog(QtWidgets.QFileDialog): Can we just call this, "FileDialog"? You said you're replacing the other dialog, let's leave off the PQ, we don't need those prefixes in our code. > + @classmethod > + @wraps(QtWidgets.QFileDialog.getExistingDirectory) > + def getExistingDirectory(cls, *args, **kwargs): > + """ > + Reimplement `getExistingDirectory` so that it can be called with, > and return Path objects > + > + :type parent: QtWidgets.QWidget or None > + :type caption: str > + :type directory: pathlib.Path > + :type options: QtWidgets.QFileDialog.Options > + :rtype: tuple[Path, str] > + """ > + > + args, kwargs = replace_params(args, kwargs, ((2, 'directory', > path_to_str),)) > + > + return_value = super().getExistingDirectory(*args, **kwargs) > + > + # getExistingDirectory returns a str that represents the path. The > string is empty if the user cancels the > + # dialog. > + return str_to_path(return_value) > + > + @classmethod > + @wraps(QtWidgets.QFileDialog.getOpenFileName) > + def getOpenFileName(cls, *args, **kwargs): > + """ > + Reimplement `getOpenFileName` so that it can be called with, and > return Path objects > + > + :type parent: QtWidgets.QWidget or None > + :type caption: str > + :type directory: pathlib.Path > + :type filter: str > + :type initialFilter: str > + :type options: QtWidgets.QFileDialog.Options > + :rtype: tuple[Path, str] > + """ > + > + args, kwargs = replace_params(args, kwargs, ((2, 'directory', > path_to_str),)) > + > + return_value = super().getOpenFileName(*args, **kwargs) filename, is_ok = super().getOpenFileName(*args, **kwargs) > + > + # getOpenFileName returns a tuple. The first item is a of str's that > represents the path. The string is empty if > + # the user cancels the dialog. > + return str_to_path(return_value[0]), return_value[1] return str_to_path(filename), is_ok > + > + @classmethod > + def getOpenFileNames(cls, *args, **kwargs): > + """ > + Reimplement `getOpenFileNames` so that it can be called with, and > return Path objects > + > + :type parent: QtWidgets.QWidget or None > + :type caption: str > + :type directory: pathlib.Path > + :type filter: str > + :type initialFilter: str > + :type options: QtWidgets.QFileDialog.Options > + :rtype: tuple[list[Path], str] > + """ > + args, kwargs = replace_params(args, kwargs, ((2, 'directory', > path_to_str),)) > + > + return_value = super().getOpenFileNames(*args, **kwargs) paths, is_ok = super().getOpenFileName(*args, **kwargs) > + > + # getSaveFileName returns a tuple. The first item is a list of str's > that represents the path. The list is > + # empty if the user cancels the dialog. > + paths = [str_to_path(path) for path in return_value[0]] paths = [str_to_path(path) for path in paths] > + return paths, return_value[1] > + > + @classmethod > + def getSaveFileName(cls, *args, **kwargs): > + """ > + Reimplement `getSaveFileName` so that it can be called with, and > return Path objects > + > + :type parent: QtWidgets.QWidget or None > + :type caption: str > + :type directory: pathlib.Path > + :type filter: str > + :type initialFilter: str > + :type options: QtWidgets.QFileDialog.Options > + :rtype: tuple[Path or None, str] > + """ > + args, kwargs = replace_params(args, kwargs, ((2, 'directory', > path_to_str),)) > + > + return_value = super().getSaveFileName(*args, **kwargs) filename, is_ok = super().getSaveFileName(*args, **kwargs) > + > + # getSaveFileName returns a tuple. The first item represents the > path as a str. The string is empty if the user > + # cancels the dialog. > + return str_to_path(return_value[0]), return_value[1] return str_to_path(filename), is_ok > > === modified file 'openlp/core/ui/lib/pathedit.py' (properties changed: +x to > -x) > --- openlp/core/ui/lib/pathedit.py 2017-07-04 23:13:51 +0000 > +++ openlp/core/ui/lib/pathedit.py 2017-08-04 22:00:43 +0000 > @@ -38,11 +40,12 @@ > The :class:`~openlp.core.ui.lib.pathedit.PathEdit` class subclasses > QWidget to create a custom widget for use when > a file or directory needs to be selected. > """ > - pathChanged = QtCore.pyqtSignal(str) > + Don't need this open line. > + pathChanged = QtCore.pyqtSignal(Path) > > def __init__(self, parent=None, path_type=PathType.Files, > default_path=None, dialog_caption=None, show_revert=True): > """ > - Initalise the PathEdit widget > + Initialise the PathEdit widget > > :param parent: The parent of the widget. This is just passed to the > super method. > :type parent: QWidget or None > > === modified file 'openlp/core/ui/themeform.py' > --- openlp/core/ui/themeform.py 2017-05-30 18:50:39 +0000 > +++ openlp/core/ui/themeform.py 2017-08-04 22:00:43 +0000 > @@ -28,12 +28,14 @@ > from PyQt5 import QtCore, QtGui, QtWidgets > > from openlp.core.common import Registry, RegistryProperties, UiStrings, > translate, get_images_filter, is_not_image_file > +from openlp.core.common.path import path_to_str, str_to_path > from openlp.core.lib.theme import BackgroundType, BackgroundGradientType > from openlp.core.lib.ui import critical_error_message_box > from openlp.core.ui import ThemeLayoutForm > from openlp.core.ui.media.webkitplayer import VIDEO_EXT > from .themewizard import Ui_ThemeWizard > > + Don't need this open line. > log = logging.getLogger(__name__) > > > > === modified file 'openlp/core/ui/themewizard.py' > --- openlp/core/ui/themewizard.py 2017-05-22 18:22:43 +0000 > +++ openlp/core/ui/themewizard.py 2017-08-04 22:00:43 +0000 > @@ -30,6 +30,8 @@ > from openlp.core.lib.ui import add_welcome_page, > create_valign_selection_widgets > from openlp.core.ui.lib import ColorButton, PathEdit > > +from pathlib import Path Isn't pathlib part of the Python standard library? If it is, its import belongs at the top, not the bottom. https://wiki.openlp.org/Development:Coding_Standards#Import_Order > + > > class Ui_ThemeWizard(object): > """ -- https://code.launchpad.net/~phill-ridout/openlp/pathlib2/+merge/328616 Your team OpenLP Core is subscribed to branch lp:openlp. _______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
