[pp:rename] improve renaming files 'to' a format (#5846, #6044)

docs/configuration.rst

@@ -6094,12 +6094,6 @@ Description
 
     When no value is given, `extractor.*.filename`_ is used.
 
-    Note:
-    With default settings, the potential download to `extractor.*.filename`_
-    still happens, even when using this post processor.
-    Disabling `file downloads <extractor.*.download_>`__
-    when using this option is recommended.
-
 
 rename.skip
 -----------

docs/options.md

@@ -149,8 +149,7 @@
     --rename FORMAT             Rename previously downloaded files from FORMAT
                                 to the current filename format
     --rename-to FORMAT          Rename previously downloaded files from the
-                                current filename format to FORMAT (disables
+                                current filename format to FORMAT
-                                downloads)
     --ugoira FMT                Convert Pixiv Ugoira to FMT using FFmpeg.
                                 Supported formats are 'webm', 'mp4', 'gif',
                                 'vp8', 'vp9', 'vp9-lossless', 'copy'.

gallery_dl/job.py

@@ -322,6 +322,12 @@ class DownloadJob(Job):
             for callback in hooks["prepare-after"]:
                 callback(pathfmt)
 
+            if pathfmt.exists():
+                if archive and self._archive_write_skip:
+                    archive.add(kwdict)
+                self.handle_skip()
+                return
+
         if self.sleep:
             self.extractor.sleep(self.sleep(), "download")
 
@@ -474,10 +480,11 @@ class DownloadJob(Job):
 
     def handle_skip(self):
         pathfmt = self.pathfmt
-        self.out.skip(pathfmt.path)
         if "skip" in self.hooks:
             for callback in self.hooks["skip"]:
                 callback(pathfmt)
+        self.out.skip(pathfmt.path)
+
         if self._skipexc:
             if not self._skipftr or self._skipftr(pathfmt.kwdict):
                 self._skipcnt += 1

gallery_dl/option.py

@@ -78,7 +78,6 @@ class RenameAction(argparse.Action):
     """Configure rename post processors"""
     def __call__(self, parser, namespace, value, option_string=None):
         if self.const:
-            namespace.options.append(((), "download", False))
             namespace.postprocessors.append({
                 "name": "rename",
                 "to"  : value,
@@ -687,7 +686,7 @@ def build_parser():
         "--rename-to",
         dest="postprocessors", metavar="FORMAT", action=RenameAction, const=1,
         help=("Rename previously downloaded files from the current filename "
-              "format to FORMAT (disables downloads)"),
+              "format to FORMAT"),
     )
     postprocessor.add_argument(
         "--ugoira",

gallery_dl/postprocessor/rename.py

@@ -26,29 +26,53 @@ class RenamePP(PostProcessor):
             self._old = self._apply_format(old)
             self._new = (self._apply_format(new) if new else
                          self._apply_pathfmt)
+            job.register_hooks({
+                "prepare": self.rename_from,
+            }, options)
+
         elif new:
             self._old = self._apply_pathfmt
             self._new = self._apply_format(new)
+            job.register_hooks({
+                "skip"         : self.rename_to_skip,
+                "prepare-after": self.rename_to_pafter,
+            }, options)
+
         else:
             raise ValueError("Option 'from' or 'to' is required")
 
-        job.register_hooks({"prepare": self.run}, options)
+    def rename_from(self, pathfmt):
-
+        name_old = self._old(pathfmt)
-    def run(self, pathfmt):
+        path_old = pathfmt.realdirectory + name_old
-        old = self._old(pathfmt)
-        path_old = pathfmt.realdirectory + old
 
         if os.path.exists(path_old):
-            new = self._new(pathfmt)
+            name_new = self._new(pathfmt)
-            path_new = pathfmt.realdirectory + new
+            path_new = pathfmt.realdirectory + name_new
+            self._rename(path_old, name_old, path_new, name_new)
 
-            if self.skip and os.path.exists(path_new):
+    def rename_to_skip(self, pathfmt):
-                return self.log.warning(
+        name_old = self._old(pathfmt)
-                    "Not renaming '%s' to '%s' since another file with the "
+        path_old = pathfmt.realdirectory + name_old
-                    "same name exists", old, new)
 
-            self.log.info("'%s' -> '%s'", old, new)
+        if os.path.exists(path_old):
-            os.replace(path_old, path_new)
+            pathfmt.filename = name_new = self._new(pathfmt)
+            pathfmt.path = pathfmt.directory + name_new
+            pathfmt.realpath = path_new = pathfmt.realdirectory + name_new
+            self._rename(path_old, name_old, path_new, name_new)
+
+    def rename_to_pafter(self, pathfmt):
+        pathfmt.filename = name_new = self._new(pathfmt)
+        pathfmt.path = pathfmt.directory + name_new
+        pathfmt.realpath = pathfmt.realdirectory + name_new
+
+    def _rename(self, path_old, name_old, path_new, name_new):
+        if self.skip and os.path.exists(path_new):
+            return self.log.warning(
+                "Not renaming '%s' to '%s' since another file with the "
+                "same name exists", name_old, name_new)
+
+        self.log.info("'%s' -> '%s'", name_old, name_new)
+        os.replace(path_old, path_new)
 
     def _apply_pathfmt(self, pathfmt):
         return pathfmt.build_filename(pathfmt.kwdict)

test/test_postprocessor.py

@@ -767,7 +767,7 @@ class RenameTest(BasePostprocessorTest):
         self._create({"to": "{id}.{extension}"}, {"id": 12345})
         path = self._prepare("file.ext")
 
-        self._trigger()
+        self._trigger(("skip",))
 
         self.assertEqual(os.listdir(path), ["12345.ext"])