Review: Needs Fixing
See below
If we need to keep adding str to things should Application methods be
internally wrapped with str not adding str all over the code.
Diff comments:
> === modified file 'openlp/core/__init__.py'
> --- openlp/core/__init__.py 2017-05-30 18:42:35 +0000
> +++ openlp/core/__init__.py 2017-08-02 06:25:28 +0000
> @@ -181,7 +181,7 @@
> """
> Check if the data folder path exists.
> """
> - data_folder_path = AppLocation.get_data_path()
> + data_folder_path = str(AppLocation.get_data_path())
Why do we have to add back in the str this harks back to the bad old days
> if not os.path.exists(data_folder_path):
> log.critical('Database was not found in: ' + data_folder_path)
> status = QtWidgets.QMessageBox.critical(None,
> translate('OpenLP', 'Data Directory Error'),
>
> === modified file 'openlp/core/common/applocation.py'
> --- openlp/core/common/applocation.py 2016-12-31 11:01:36 +0000
> +++ openlp/core/common/applocation.py 2017-08-02 06:25:28 +0000
> @@ -84,84 +87,97 @@
> def get_data_path():
> """
> Return the path OpenLP stores all its data under.
> +
> + :return: The data path to use.
> + :rtype: pathlib.Path
> """
> # Check if we have a different data location.
> if Settings().contains('advanced/data path'):
> - path = Settings().value('advanced/data path')
> + path = Path(Settings().value('advanced/data path'))
> else:
> path = AppLocation.get_directory(AppLocation.DataDir)
> - check_directory_exists(path)
> - return os.path.normpath(path)
> + check_directory_exists(str(path))
> + return path
>
> @staticmethod
> - def get_files(section=None, extension=None):
> + def get_files(section=None, extension=''):
> """
> Get a list of files from the data files path.
>
> - :param section: Defaults to *None*. The section of code getting the
> files - used to load from a section's
> - data subdirectory.
> - :param extension:
> - Defaults to *None*. The extension to search for. For example::
> -
> - '.png'
> + :param section: Defaults to *None*. The section of code getting the
> files - used to load from a section's data
> + subdirectory.
> + :type section: None | str
> +
> + :param extension: Defaults to ''. The extension to search for. For
> example::
> + '.png'
> + :type extension: str
> +
> + :return: List of files found.
> + :rtype: list[pathlib.Path]
> """
> path = AppLocation.get_data_path()
> if section:
> - path = os.path.join(path, section)
> + path = path / section
is this right
> try:
> - files = os.listdir(path)
> + file_paths = path.glob('*' + extension)
> + return [file_path.relative_to(path) for file_path in file_paths]
> except OSError:
> return []
> - if extension:
> - return [filename for filename in files if extension ==
> os.path.splitext(filename)[1]]
> - else:
> - # no filtering required
> - return files
>
> @staticmethod
> def get_section_data_path(section):
> """
> Return the path a particular module stores its data under.
> +
> + :type section: str
> +
> + :rtype: pathlib.Path
> """
> - data_path = AppLocation.get_data_path()
> - path = os.path.join(data_path, section)
> - check_directory_exists(path)
> + path = AppLocation.get_data_path() / section
> + check_directory_exists(str(path))
> return path
>
>
> def _get_os_dir_path(dir_type):
> """
> Return a path based on which OS and environment we are running in.
> +
> + :param dir_type: AppLocation Enum of the requested path type
> + :type dir_type: AppLocation Enum
> +
> + :return: The requested path
> + :rtype: pathlib.Path
> """
> # If running from source, return the language directory from the source
> directory
> if dir_type == AppLocation.LanguageDir:
> - directory =
> os.path.abspath(os.path.join(os.path.dirname(openlp.__file__), '..',
> 'resources'))
> - if os.path.exists(directory):
> + directory =
> Path(os.path.abspath(os.path.join(os.path.dirname(openlp.__file__), '..',
> 'resources')))
> + if directory.exists():
> return directory
> if is_win():
> + openlp_folder_path = Path(os.getenv('APPDATA'), 'openlp')
> if dir_type == AppLocation.DataDir:
> - return os.path.join(str(os.getenv('APPDATA')), 'openlp', 'data')
> + return openlp_folder_path / 'data'
> elif dir_type == AppLocation.LanguageDir:
> return os.path.dirname(openlp.__file__)
> - return os.path.join(str(os.getenv('APPDATA')), 'openlp')
> + return openlp_folder_path
> elif is_macosx():
> + openlp_folder_path = Path(os.getenv('HOME'), 'Library', 'Application
> Support', 'openlp')
> if dir_type == AppLocation.DataDir:
> - return os.path.join(str(os.getenv('HOME')), 'Library',
> 'Application Support', 'openlp', 'Data')
> + return openlp_folder_path / 'Data'
> elif dir_type == AppLocation.LanguageDir:
> return os.path.dirname(openlp.__file__)
> - return os.path.join(str(os.getenv('HOME')), 'Library', 'Application
> Support', 'openlp')
> + return openlp_folder_path
> else:
> if dir_type == AppLocation.LanguageDir:
> - for prefix in ['/usr/local', '/usr']:
> - directory = os.path.join(prefix, 'share', 'openlp')
> - if os.path.exists(directory):
> - return directory
> - return os.path.join('/usr', 'share', 'openlp')
> + directory = Path('/usr', 'local', 'share', 'openlp')
> + if directory.exists():
> + return directory
> + return Path('/usr', 'share', 'openlp')
> if XDG_BASE_AVAILABLE:
> - if dir_type == AppLocation.DataDir:
> - return os.path.join(str(BaseDirectory.xdg_data_home),
> 'openlp')
> + if dir_type == AppLocation.DataDir or dir_type ==
> AppLocation.CacheDir:
> + return Path(BaseDirectory.xdg_data_home, 'openlp')
> elif dir_type == AppLocation.CacheDir:
> - return os.path.join(str(BaseDirectory.xdg_cache_home),
> 'openlp')
> + return Path(BaseDirectory.xdg_cache_home, 'openlp')
> if dir_type == AppLocation.DataDir:
> - return os.path.join(str(os.getenv('HOME')), '.openlp', 'data')
> - return os.path.join(str(os.getenv('HOME')), '.openlp')
> + return Path(os.getenv('HOME'), '.openlp', 'data')
> + return Path(os.getenv('HOME'), '.openlp')
--
https://code.launchpad.net/~phill-ridout/openlp/pathlib1/+merge/328435
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
