From b65ce6057a8fd63127c44da4ffbb0327e4498af6 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Tue, 30 Apr 2024 08:05:43 +0200 Subject: [PATCH] PICARD-2879: macOS: Extend all paths in filebrowser with /Volumes/ This ensures that selected starting directory gets resolved correctly, as the filebrowser only supports paths under /Volumes/, not directly under /. On saving drop the /Volumes/ prefix for paths on the root volume. --- picard/const/defaults.py | 5 +---- picard/ui/filebrowser.py | 10 +++++++++- picard/util/macos.py | 11 +++++++++++ test/test_util_macos.py | 26 +++++++++++++++++++++++++- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/picard/const/defaults.py b/picard/const/defaults.py index 8f58f04f3b..1f73c820aa 100644 --- a/picard/const/defaults.py +++ b/picard/const/defaults.py @@ -3,7 +3,7 @@ # Picard, the next-generation MusicBrainz tagger # # Copyright (C) 2007, 2014, 2016 Lukáš Lalinský -# Copyright (C) 2014, 2019-2022 Philipp Wolfer +# Copyright (C) 2014, 2019-2022, 2024 Philipp Wolfer # Copyright (C) 2014-2016, 2018-2021, 2024 Laurent Monin # Copyright (C) 2015 Ohm Patel # Copyright (C) 2016 Rahul Raturi @@ -77,9 +77,6 @@ DEFAULT_CURRENT_BROWSER_PATH = QStandardPaths.writableLocation(QStandardPaths.StandardLocation.HomeLocation) -if IS_MACOS: - from picard.util.macos import extend_root_volume_path - DEFAULT_CURRENT_BROWSER_PATH = extend_root_volume_path(DEFAULT_CURRENT_BROWSER_PATH) # Default query limit DEFAULT_QUERY_LIMIT = 50 diff --git a/picard/ui/filebrowser.py b/picard/ui/filebrowser.py index f9583c38a5..3643247754 100644 --- a/picard/ui/filebrowser.py +++ b/picard/ui/filebrowser.py @@ -4,7 +4,7 @@ # # Copyright (C) 2006-2008 Lukáš Lalinský # Copyright (C) 2008 Hendrik van Antwerpen -# Copyright (C) 2008-2009, 2019-2022 Philipp Wolfer +# Copyright (C) 2008-2009, 2019-2022, 2024 Philipp Wolfer # Copyright (C) 2011 Andrew Barnert # Copyright (C) 2012-2013 Michael Wiencek # Copyright (C) 2013 Wieland Hoffmann @@ -42,6 +42,10 @@ from picard.formats import supported_formats from picard.i18n import gettext as _ from picard.util import find_existing_path +from picard.util.macos import ( + extend_root_volume_path, + strip_root_volume_path, +) class FileBrowser(QtWidgets.QTreeView): @@ -176,6 +180,8 @@ def _restore_state(self): path = config.persist['current_browser_path'] scrolltype = QtWidgets.QAbstractItemView.ScrollHint.PositionAtCenter if path: + if IS_MACOS: + path = extend_root_volume_path(path) index = self.model().index(find_existing_path(path)) self.setCurrentIndex(index) self.expand(index) @@ -185,6 +191,8 @@ def _get_destination_from_path(self, path): destination = os.path.normpath(path) if not os.path.isdir(destination): destination = os.path.dirname(destination) + if IS_MACOS: + destination = strip_root_volume_path(destination) return destination def load_file_for_item(self, index): diff --git a/picard/util/macos.py b/picard/util/macos.py index af3b7eee02..41c51775d3 100644 --- a/picard/util/macos.py +++ b/picard/util/macos.py @@ -41,3 +41,14 @@ def extend_root_volume_path(path): path = path[1:] path = os.path.join(root_volume, path) return path + + +def strip_root_volume_path(path): + if not path.startswith("/Volumes/"): + return path + root_volume = _find_root_volume() + if root_volume: + norm_path = os.path.normpath(path) + if norm_path.startswith(root_volume): + path = os.path.join('/', norm_path[len(root_volume):]) + return path diff --git a/test/test_util_macos.py b/test/test_util_macos.py index 812e049702..f176cf3958 100644 --- a/test/test_util_macos.py +++ b/test/test_util_macos.py @@ -24,7 +24,10 @@ from test.picardtestcase import PicardTestCase from picard.const.sys import IS_MACOS -from picard.util.macos import extend_root_volume_path +from picard.util.macos import ( + extend_root_volume_path, + strip_root_volume_path, +) def fake_os_scandir(path): @@ -58,3 +61,24 @@ def test_path_donot_starts_with_volumes_failed(self): result = extend_root_volume_path(path) self.assertEqual(result, path) self.assertTrue(mock.called) + + +@unittest.skipUnless(IS_MACOS, "macOS test") +class UtilMacosStripRootVolumeTest(PicardTestCase): + + def test_path_starts_not_with_volumes(self): + path = '/Users/sandra' + result = strip_root_volume_path(path) + self.assertEqual(result, path) + + @patch('picard.util.macos._find_root_volume', lambda: '/Volumes/root_volume') + def test_path_starts_not_with_root_volume(self): + path = '/Volumes/other_volume/foo/bar' + result = strip_root_volume_path(path) + self.assertEqual(result, path) + + @patch('picard.util.macos._find_root_volume', lambda: '/Volumes/root_volume') + def test_path_starts_with_root_volume(self): + path = '/Volumes/root_volume/foo/bar' + result = strip_root_volume_path(path) + self.assertEqual(result, '/foo/bar')