diff --git a/docs/page/actions.rst b/docs/page/actions.rst index 10a55031..f9e9e048 100644 --- a/docs/page/actions.rst +++ b/docs/page/actions.rst @@ -6,15 +6,15 @@ Actions Move ---- -.. autoclass:: Move(dest, [overwrite=False]) +.. autoclass:: Move(dest, [overwrite=False], [counter_separator=' ']) Copy ---- -.. autoclass:: Copy(dest, [overwrite=False]) +.. autoclass:: Copy(dest, [overwrite=False], [counter_separator=' ']) Rename ------ -.. autoclass:: Rename(dest, [overwrite=False]) +.. autoclass:: Rename(dest, [overwrite=False], [counter_separator=' ']) Trash ----- diff --git a/organize/actions/copy.py b/organize/actions/copy.py index 930ba8f5..27ac4939 100644 --- a/organize/actions/copy.py +++ b/organize/actions/copy.py @@ -24,6 +24,10 @@ class Copy(Action): Otherwise it will start enumerating files (append a counter to the filename) to resolve naming conflicts. [Default: False] + :param str counter_separator: + specifies the separator between filename and the appended counter. + Only relevant if **overwrite** is disabled. [Default: ``\' \'``] + Examples: - Copy all pdfs into `~/Desktop/somefolder/` and keep filenames @@ -55,7 +59,10 @@ class Copy(Action): overwrite: true - Copy into the folder `Invoices`. Keep the filename but do not - overwrite existing files (adds an index to the file) + overwrite existing files. To prevent overwriting files, an index is + added to the filename, so `somefile.jpg` becomes `somefile 2.jpg`. + The counter separator is `' '` by default, but can be changed using + the `counter_separator` property. .. code-block:: yaml :caption: config.yaml @@ -69,11 +76,13 @@ class Copy(Action): - Copy: dest: '~/Documents/Invoices/' overwrite: false + counter_separator: '_' """ - def __init__(self, dest: str, overwrite=False): + def __init__(self, dest: str, overwrite=False, counter_separator=' '): self.dest = dest self.overwrite = overwrite + self.counter_separator = counter_separator self.log = logging.getLogger(__name__) def run(self, attrs: dict, simulate: bool): @@ -92,7 +101,8 @@ def run(self, attrs: dict, simulate: bool): self.print('File already exists') Trash().run({'path': new_path}, simulate=simulate) else: - new_path = find_unused_filename(new_path) + new_path = find_unused_filename( + path=new_path, separator=self.counter_separator) self.print('Copy to "%s"' % new_path) if not simulate: diff --git a/organize/actions/move.py b/organize/actions/move.py index fd36fa79..590399e0 100644 --- a/organize/actions/move.py +++ b/organize/actions/move.py @@ -27,6 +27,10 @@ class Move(Action): Otherwise it will start enumerating files (append a counter to the filename) to resolve naming conflicts. [Default: False] + :param str counter_separator: + specifies the separator between filename and the appended counter. + Only relevant if **overwrite** is disabled. [Default: ``\' \'``] + Examples: - Move all pdfs and jpgs from the desktop into the folder "~/Desktop/media/". Filenames are not changed. @@ -61,7 +65,8 @@ class Move(Action): overwrite: true - Move pdfs into the folder `Invoices`. Keep the filename but do not - overwrite existing files (adds an index to the file) + overwrite existing files. To prevent overwriting files, an index is + added to the filename, so ``somefile.jpg`` becomes ``somefile 2.jpg``. .. code-block:: yaml :caption: config.yaml @@ -75,11 +80,13 @@ class Move(Action): - Copy: dest: '~/Documents/Invoices/' overwrite: false + counter_separator: '_' """ - def __init__(self, dest: str, overwrite=False): + def __init__(self, dest: str, overwrite=False, counter_separator=' '): self.dest = dest self.overwrite = overwrite + self.counter_separator = counter_separator self.log = logging.getLogger(__name__) def run(self, attrs: dict, simulate: bool): @@ -100,7 +107,8 @@ def run(self, attrs: dict, simulate: bool): self.print('File already exists') Trash().run({'path': new_path}, simulate=simulate) else: - new_path = find_unused_filename(new_path) + new_path = find_unused_filename( + path=new_path, separator=self.counter_separator) if new_path_samefile and new_path == path: self.print('Keep location') diff --git a/organize/actions/rename.py b/organize/actions/rename.py index cc43315d..e77e122e 100644 --- a/organize/actions/rename.py +++ b/organize/actions/rename.py @@ -21,6 +21,10 @@ class Rename(Action): Otherwise it will start enumerating files (append a counter to the filename) to resolve naming conflicts. [Default: False] + :param str counter_separator: + specifies the separator between filename and the appended counter. + Only relevant if **overwrite** is disabled. [Default: ``\' \'``] + Examples: - Convert all .PDF file extensions to lowercase (.pdf): @@ -47,12 +51,13 @@ class Rename(Action): - Rename: "{path.stem}.{extension.lower}" """ - def __init__(self, name: str, overwrite=False): + def __init__(self, name: str, overwrite=False, counter_separator=' '): if os.path.sep in name: ValueError('Rename only takes a filename as argument. To move ' 'files between folders use the Move action.') self.name = name self.overwrite = overwrite + self.counter_separator = counter_separator self.log = logging.getLogger(__name__) def run(self, attrs: dict, simulate: bool) -> Path: @@ -68,7 +73,8 @@ def run(self, attrs: dict, simulate: bool) -> Path: self.print('File already exists') Trash().run({'path': new_path}, simulate=simulate) else: - new_path = find_unused_filename(new_path) + new_path = find_unused_filename( + path=new_path, separator=self.counter_separator) # do nothing if the new name is equal to the old name and the file is # the same @@ -82,4 +88,5 @@ def run(self, attrs: dict, simulate: bool) -> Path: return new_path def __str__(self): - return 'Rename(name=%s, overwrite=%s)' % (self.name, self.overwrite) + return 'Rename(name=%s, overwrite=%s, sep=%s)' % ( + self.name, self.overwrite, self.counter_separator) diff --git a/organize/utils.py b/organize/utils.py index 2a15a4de..c0c901f4 100644 --- a/organize/utils.py +++ b/organize/utils.py @@ -76,22 +76,31 @@ def __str__(self): return '{%s}' % ', '.join('%r: %r' % (key, self[key]) for key in self) -def find_unused_filename(path: Path) -> Path: - """ - we assume path already exists. This function then adds a counter to the - filename until we find a unused filename. - """ +def increment_filename_version(path: Path, separator=' ') -> Path: stem = path.stem try: - splitstem = stem.split(' ') + # try to find any existing counter + splitstem = stem.split(separator) # raises ValueError on missing sep if len(splitstem) < 2: raise ValueError() - count = int(splitstem[-1]) - stem = ' '.join(splitstem[:-1]) + counter = int(splitstem[-1]) + stem = separator.join(splitstem[:-1]) except (ValueError, IndexError): - count = 1 + # not found, we start with 1 + counter = 1 + return path.with_name('{stem}{sep}{cnt}{suffix}'.format( + stem=stem, sep=separator, cnt=(counter + 1), suffix=path.suffix)) + + +def find_unused_filename(path: Path, separator=' ') -> Path: + """ + We assume the given path already exists. This function adds a counter to the + filename until we find a unused filename. + """ + # TODO: Check whether the assumption can be eliminated for cleaner code. + # TODO: Optimization: The counter only needs to be parsed once. + tmp = path while True: - count += 1 - tmp_path = path.with_name('%s %s%s' % (stem, count, path.suffix)) - if not tmp_path.exists(): - return tmp_path + tmp = increment_filename_version(tmp, separator=separator) + if not tmp.exists(): + return tmp diff --git a/tests/test_action_copy.py b/tests/test_action_copy.py index 62fce699..adf966c2 100644 --- a/tests/test_action_copy.py +++ b/tests/test_action_copy.py @@ -94,6 +94,25 @@ def test_already_exists_multiple(mock_exists, mock_samefile, mock_copy, mock_tra dst=os.path.join(USER_DIR, 'folder', 'test 4.py')) +def test_already_exists_multiple_with_separator(mock_exists, mock_samefile, + mock_copy, mock_trash, + mock_mkdir): + attrs = { + 'basedir': Path.home(), + 'path': Path.home() / 'test_2.py', + } + mock_exists.side_effect = [True, True, True, False] + mock_samefile.return_value = False + copy = Copy(dest='~/folder/', overwrite=False, counter_separator='_') + copy.run(attrs, False) + mock_mkdir.assert_called_with(exist_ok=True, parents=True) + mock_exists.assert_called_with() + mock_trash.assert_not_called() + mock_copy.assert_called_with( + src=os.path.join(USER_DIR, 'test_2.py'), + dst=os.path.join(USER_DIR, 'folder', 'test_5.py')) + + def test_makedirs(mock_parent, mock_copy, mock_trash): attrs = { 'basedir': Path.home(), diff --git a/tests/test_action_move.py b/tests/test_action_move.py index af7b88f0..f02adf68 100644 --- a/tests/test_action_move.py +++ b/tests/test_action_move.py @@ -97,6 +97,25 @@ def test_already_exists_multiple(mock_exists, mock_samefile, mock_move, mock_tra assert new_path is not None +def test_already_exists_multiple_separator(mock_exists, mock_samefile, + mock_move, mock_trash, mock_mkdir): + attrs = { + 'basedir': Path.home(), + 'path': Path.home() / 'test.py', + } + mock_exists.side_effect = [True, True, True, False] + mock_samefile.return_value = False + move = Move(dest='~/folder/', overwrite=False, counter_separator='_') + new_path = move.run(attrs, False) + mock_mkdir.assert_called_with(exist_ok=True, parents=True) + mock_exists.assert_called_with() + mock_trash.assert_not_called() + mock_move.assert_called_with( + src=os.path.join(USER_DIR, 'test.py'), + dst=os.path.join(USER_DIR, 'folder', 'test_4.py')) + assert new_path is not None + + def test_makedirs(mock_parent, mock_move, mock_trash): attrs = { 'basedir': Path.home(), diff --git a/tests/test_action_rename.py b/tests/test_action_rename.py index b9fb7cd7..5f86f31c 100644 --- a/tests/test_action_rename.py +++ b/tests/test_action_rename.py @@ -97,6 +97,21 @@ def test_already_exists_multiple(mock_exists, mock_samefile, mock_rename, mock_t assert new_path is not None +def test_already_exists_multiple_separator(mock_exists, mock_samefile, mock_rename, mock_trash): + attrs = { + 'basedir': Path.home(), + 'path': Path.home() / 'test.py', + } + mock_exists.side_effect = [True, True, True, False] + mock_samefile.return_value = False + rename = Rename(name='asd.txt', overwrite=False, counter_separator='-') + new_path = rename.run(attrs, False) + mock_exists.assert_called() + mock_trash.assert_not_called() + mock_rename.assert_called_with(Path('~/asd-4.txt').expanduser()) + assert new_path is not None + + def test_attrs(mock_exists, mock_samefile, mock_rename, mock_trash): attrs = { 'basedir': Path.home(), diff --git a/tests/test_utils.py b/tests/test_utils.py index d0ad49e2..c05dfe4e 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,4 +1,4 @@ -from organize.utils import Path, find_unused_filename, splitglob +from organize.utils import Path, find_unused_filename, splitglob, increment_filename_version def test_splitglob(): @@ -22,6 +22,12 @@ def test_unused_filename_basic(mock_exists): assert find_unused_filename(Path('somefile.jpg')) == Path('somefile 2.jpg') +def test_unused_filename_separator(mock_exists): + mock_exists.return_value = False + assert find_unused_filename( + Path('somefile.jpg'), separator='_') == Path('somefile_2.jpg') + + def test_unused_filename_multiple(mock_exists): mock_exists.side_effect = [True, True, False] assert find_unused_filename(Path('somefile.jpg')) == Path('somefile 4.jpg') @@ -37,3 +43,26 @@ def test_unused_filename_increase_digit(mock_exists): mock_exists.side_effect = [True, False] assert find_unused_filename( Path('7.gif')) == Path('7 3.gif') + + +def test_increment_filename_version(): + assert ( + increment_filename_version(Path.home() / 'f3' / 'test_123.7z') == + Path.home() / 'f3' / 'test_123 2.7z') + assert ( + increment_filename_version(Path.home() / 'f3' / 'test_123_2 10.7z') == + Path.home() / 'f3' / 'test_123_2 11.7z') + + +def test_increment_filename_version_separator(): + assert increment_filename_version( + Path('test_123.7z'), separator='_') == Path('test_124.7z') + assert increment_filename_version( + Path('test_123_2.7z'), separator='_') == Path('test_123_3.7z') + + +def test_increment_filename_version_no_separator(): + assert increment_filename_version( + Path('test.7z'), separator='') == Path('test2.7z') + assert increment_filename_version( + Path('test 10.7z'), separator='') == Path('test 102.7z')