From d37197c4f7e3e7fa083930aaac0841d1895db134 Mon Sep 17 00:00:00 2001 From: Andrew Senetar Date: Sat, 30 Apr 2022 19:52:09 -0500 Subject: [PATCH] Move mac/win to subpackages & fix #64 - Move macOS and Windows implementations to sub packagese to improve organization - Fix #64 in legacy windows implementation by mapping results to standard error codes --- send2trash/__init__.py | 4 +- send2trash/{plat_osx.py => mac/__init__.py} | 6 +-- .../{plat_osx_ctypes.py => mac/legacy.py} | 4 +- .../{plat_osx_pyobjc.py => mac/modern.py} | 4 +- .../{ => win}/IFileOperationProgressSink.py | 0 send2trash/{plat_win.py => win/__init__.py} | 6 +-- .../{plat_win_legacy.py => win/legacy.py} | 45 +++++++++++++++++-- .../{plat_win_modern.py => win/modern.py} | 9 ++-- tests/test_plat_other.py | 41 ++++++++++++++--- tests/test_plat_win.py | 4 +- 10 files changed, 98 insertions(+), 25 deletions(-) rename send2trash/{plat_osx.py => mac/__init__.py} (80%) rename send2trash/{plat_osx_ctypes.py => mac/legacy.py} (96%) rename send2trash/{plat_osx_pyobjc.py => mac/modern.py} (93%) rename send2trash/{ => win}/IFileOperationProgressSink.py (100%) rename send2trash/{plat_win.py => win/__init__.py} (79%) rename send2trash/{plat_win_legacy.py => win/legacy.py} (68%) rename send2trash/{plat_win_modern.py => win/modern.py} (93%) diff --git a/send2trash/__init__.py b/send2trash/__init__.py index 4c3f7e9..5b01f31 100644 --- a/send2trash/__init__.py +++ b/send2trash/__init__.py @@ -9,9 +9,9 @@ import sys from .exceptions import TrashPermissionError # noqa: F401 if sys.platform == "darwin": - from .plat_osx import send2trash + from .mac import send2trash elif sys.platform == "win32": - from .plat_win import send2trash + from .win import send2trash else: try: # If we can use gio, let's use it diff --git a/send2trash/plat_osx.py b/send2trash/mac/__init__.py similarity index 80% rename from send2trash/plat_osx.py rename to send2trash/mac/__init__.py index 9ef4d6c..7c10029 100644 --- a/send2trash/plat_osx.py +++ b/send2trash/mac/__init__.py @@ -11,10 +11,10 @@ from sys import version_info macos_ver = tuple(int(part) for part in mac_ver()[0].split(".")) if version_info >= (3, 6) and macos_ver >= (10, 9): try: - from .plat_osx_pyobjc import send2trash + from .modern import send2trash except ImportError: # Try to fall back to ctypes version, although likely problematic still - from .plat_osx_ctypes import send2trash + from .legacy import send2trash else: # Just use the old version otherwise - from .plat_osx_ctypes import send2trash # noqa: F401 + from .legacy import send2trash # noqa: F401 diff --git a/send2trash/plat_osx_ctypes.py b/send2trash/mac/legacy.py similarity index 96% rename from send2trash/plat_osx_ctypes.py rename to send2trash/mac/legacy.py index 4b8c2b3..6bb7a8c 100644 --- a/send2trash/plat_osx_ctypes.py +++ b/send2trash/mac/legacy.py @@ -9,8 +9,8 @@ from __future__ import unicode_literals from ctypes import cdll, byref, Structure, c_char, c_char_p from ctypes.util import find_library -from .compat import binary_type -from .util import preprocess_paths +from ..compat import binary_type +from ..util import preprocess_paths Foundation = cdll.LoadLibrary(find_library("Foundation")) CoreServices = cdll.LoadLibrary(find_library("CoreServices")) diff --git a/send2trash/plat_osx_pyobjc.py b/send2trash/mac/modern.py similarity index 93% rename from send2trash/plat_osx_pyobjc.py rename to send2trash/mac/modern.py index 8694444..0699ce7 100644 --- a/send2trash/plat_osx_pyobjc.py +++ b/send2trash/mac/modern.py @@ -5,8 +5,8 @@ # http://www.hardcoded.net/licenses/bsd_license from Foundation import NSFileManager, NSURL -from .compat import text_type -from .util import preprocess_paths +from ..compat import text_type +from ..util import preprocess_paths def check_op_result(op_result): diff --git a/send2trash/IFileOperationProgressSink.py b/send2trash/win/IFileOperationProgressSink.py similarity index 100% rename from send2trash/IFileOperationProgressSink.py rename to send2trash/win/IFileOperationProgressSink.py diff --git a/send2trash/plat_win.py b/send2trash/win/__init__.py similarity index 79% rename from send2trash/plat_win.py rename to send2trash/win/__init__.py index 64ae85c..78278d5 100644 --- a/send2trash/plat_win.py +++ b/send2trash/win/__init__.py @@ -11,10 +11,10 @@ from platform import version if int(version().split(".", 1)[0]) >= 6: try: # Attempt to use pywin32 to use IFileOperation - from .plat_win_modern import send2trash + from .modern import send2trash except ImportError: # use SHFileOperation as fallback - from .plat_win_legacy import send2trash + from .legacy import send2trash else: # use SHFileOperation as fallback - from .plat_win_legacy import send2trash # noqa: F401 + from .legacy import send2trash # noqa: F401 diff --git a/send2trash/plat_win_legacy.py b/send2trash/win/legacy.py similarity index 68% rename from send2trash/plat_win_legacy.py rename to send2trash/win/legacy.py index 137c592..1ae5a0b 100644 --- a/send2trash/plat_win_legacy.py +++ b/send2trash/win/legacy.py @@ -6,8 +6,8 @@ from __future__ import unicode_literals import os.path as op -from .compat import text_type -from .util import preprocess_paths +from ..compat import text_type +from ..util import preprocess_paths from ctypes import ( windll, @@ -53,6 +53,44 @@ FOF_ALLOWUNDO = 64 FOF_NOERRORUI = 1024 +def convert_sh_file_opt_result(result): + # map overlapping values from SHFileOpterationW to approximate standard windows errors + # ref https://docs.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-shfileoperationw#return-value + # ref https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499- + results = { + 0x71: 0x50, # DE_SAMEFILE -> ERROR_FILE_EXISTS + 0x72: 0x57, # DE_MANYSRC1DEST -> ERROR_INVALID_PARAMETER + 0x73: 0x57, # DE_DIFFDIR -> ERROR_INVALID_PARAMETER + 0x74: 0x57, # DE_ROOTDIR -> ERROR_INVALID_PARAMETER + 0x75: 0x4C7, # DE_OPCANCELLED -> ERROR_CANCELLED + 0x76: 0x57, # DE_DESTSUBTREE -> ERROR_INVALID_PARAMETER + 0x78: 0x05, # DE_ACCESSDENIEDSRC -> ERROR_ACCESS_DENIED + 0x79: 0x6F, # DE_PATHTOODEEP -> ERROR_BUFFER_OVERFLOW + 0x7A: 0x57, # DE_MANYDEST -> ERROR_INVALID_PARAMETER + 0x7C: 0xA1, # DE_INVALIDFILES -> ERROR_BAD_PATHNAME + 0x7D: 0x57, # DE_DESTSAMETREE -> ERROR_INVALID_PARAMETER + 0x7E: 0xB7, # DE_FLDDESTISFILE -> ERROR_ALREADY_EXISTS + 0x80: 0xB7, # DE_FILEDESTISFLD -> ERROR_ALREADY_EXISTS + 0x81: 0x6F, # DE_FILENAMETOOLONG -> ERROR_BUFFER_OVERFLOW + 0x82: 0x13, # DE_DEST_IS_CDROM -> ERROR_WRITE_PROTECT + 0x83: 0x13, # DE_DEST_IS_DVD -> ERROR_WRITE_PROTECT + 0x84: 0x6F9, # DE_DEST_IS_CDRECORD -> ERROR_UNRECOGNIZED_MEDIA + 0x85: 0xDF, # DE_FILE_TOO_LARGE -> ERROR_FILE_TOO_LARGE + 0x86: 0x13, # DE_SRC_IS_CDROM -> ERROR_WRITE_PROTECT + 0x87: 0x13, # DE_SRC_IS_DVD -> ERROR_WRITE_PROTECT + 0x88: 0x6F9, # DE_SRC_IS_CDRECORD -> ERROR_UNRECOGNIZED_MEDIA + 0xB7: 0x6F, # DE_ERROR_MAX -> ERROR_BUFFER_OVERFLOW + 0x402: 0xA1, # UNKNOWN -> ERROR_BAD_PATHNAME + 0x10000: 0x1D, # ERRORONDEST -> ERROR_WRITE_FAULT + 0x10074: 0x57, # DE_ROOTDIR | ERRORONDEST -> ERROR_INVALID_PARAMETER + } + + if result in results.keys(): + return results[result] + else: + return result + + def prefix_and_path(path): r"""Guess the long-path prefix based on the kind of *path*. Local paths (C:\folder\file.ext) and UNC names (\\server\folder\file.ext) @@ -141,4 +179,5 @@ def send2trash(paths): fileop.lpszProgressTitle = None result = SHFileOperationW(byref(fileop)) if result: - raise WindowsError(result, FormatError(result), paths) + error = convert_sh_file_opt_result(result) + raise WindowsError(None, FormatError(error), paths, error) diff --git a/send2trash/plat_win_modern.py b/send2trash/win/modern.py similarity index 93% rename from send2trash/plat_win_modern.py rename to send2trash/win/modern.py index 4a794be..3cd8833 100644 --- a/send2trash/plat_win_modern.py +++ b/send2trash/win/modern.py @@ -6,8 +6,8 @@ from __future__ import unicode_literals import os.path as op -from .compat import text_type -from .util import preprocess_paths +from ..compat import text_type +from ..util import preprocess_paths from platform import version import pythoncom import pywintypes @@ -27,7 +27,10 @@ def send2trash(paths): pythoncom.CoInitialize() # create instance of file operation object fileop = pythoncom.CoCreateInstance( - shell.CLSID_FileOperation, None, pythoncom.CLSCTX_ALL, shell.IID_IFileOperation, + shell.CLSID_FileOperation, + None, + pythoncom.CLSCTX_ALL, + shell.IID_IFileOperation, ) # default flags to use flags = shellcon.FOF_NOCONFIRMATION | shellcon.FOF_NOERRORUI | shellcon.FOF_SILENT | shellcon.FOFX_EARLYFAILURE diff --git a/tests/test_plat_other.py b/tests/test_plat_other.py index bec2949..331e3ef 100644 --- a/tests/test_plat_other.py +++ b/tests/test_plat_other.py @@ -50,7 +50,9 @@ def testfiles(): files = list( map( lambda index: NamedTemporaryFile( - dir=op.expanduser("~"), prefix="send2trash_test{}".format(index), delete=False, + dir=op.expanduser("~"), + prefix="send2trash_test{}".format(index), + delete=False, ), range(10), ) @@ -129,7 +131,10 @@ class ExtVol: return st.st_dev def s_ismount(path): - if op.realpath(path) in (op.realpath(self.trash_topdir), op.realpath(self.trash_topdir_b),): + if op.realpath(path) in ( + op.realpath(self.trash_topdir), + op.realpath(self.trash_topdir_b), + ): return True return old_ismount(path) @@ -163,7 +168,17 @@ def test_trash_topdir(gen_ext_vol): s2t(gen_ext_vol[2]) assert op.exists(gen_ext_vol[2]) is False assert op.exists(op.join(trash_dir, str(os.getuid()), "files", gen_ext_vol[1])) is True - assert op.exists(op.join(trash_dir, str(os.getuid()), "info", gen_ext_vol[1] + INFO_SUFFIX,)) is True + assert ( + op.exists( + op.join( + trash_dir, + str(os.getuid()), + "info", + gen_ext_vol[1] + INFO_SUFFIX, + ) + ) + is True + ) # info relative path (if another test is added, with the same fileName/Path, # then it gets renamed etc.) cfg = ConfigParser() @@ -175,7 +190,15 @@ def test_trash_topdir_fallback(gen_ext_vol): s2t(gen_ext_vol[2]) assert op.exists(gen_ext_vol[2]) is False assert ( - op.exists(op.join(gen_ext_vol[0].trash_topdir, ".Trash-" + str(os.getuid()), "files", gen_ext_vol[1],)) is True + op.exists( + op.join( + gen_ext_vol[0].trash_topdir, + ".Trash-" + str(os.getuid()), + "files", + gen_ext_vol[1], + ) + ) + is True ) @@ -195,6 +218,14 @@ def test_trash_symlink(gen_ext_vol): s2t(op.join(sl_dir, gen_ext_vol[1])) assert op.exists(file_path) is False assert ( - op.exists(op.join(gen_ext_vol[0].trash_topdir, ".Trash-" + str(os.getuid()), "files", gen_ext_vol[1],)) is True + op.exists( + op.join( + gen_ext_vol[0].trash_topdir, + ".Trash-" + str(os.getuid()), + "files", + gen_ext_vol[1], + ) + ) + is True ) os.remove(sl_dir) diff --git a/tests/test_plat_win.py b/tests/test_plat_win.py index bd1ea3b..87ebb1a 100644 --- a/tests/test_plat_win.py +++ b/tests/test_plat_win.py @@ -9,8 +9,8 @@ from send2trash import send2trash as s2t # import the two versions as well as the "automatic" version if sys.platform == "win32": - from send2trash.plat_win_modern import send2trash as s2t_modern - from send2trash.plat_win_legacy import send2trash as s2t_legacy + from send2trash.win.modern import send2trash as s2t_modern + from send2trash.win.legacy import send2trash as s2t_legacy else: pytest.skip("Skipping windows-only tests", allow_module_level=True)