add counter_separator settings to copy, move, rename

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
 -----

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:

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')

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)

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

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(),

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(),

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(),

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')