Merge branch '5hir0kur0-fix-1798-crashes-when-deleting-to-trash'

2 files changed, 61 insertions, 5 deletions

diff --git a/ranger/config/commands.py b/ranger/config/commands.py
index 90f509ab..484dc0ad 100755
--- a/ranger/config/commands.py
+++ b/ranger/config/commands.py
@@ -728,8 +728,11 @@ class trash(Command):
             return os.path.isdir(path) and not os.path.islink(path) and len(os.listdir(path)) > 0
 
         if self.rest(1):
-            files = shlex.split(self.rest(1))
-            many_files = (len(files) > 1 or is_directory_with_files(files[0]))
+            file_names = shlex.split(self.rest(1))
+            files = self.fm.get_filesystem_objects(file_names)
+            if files is None:
+                return
+            many_files = (len(files) > 1 or is_directory_with_files(files[0].path))
         else:
             cwd = self.fm.thisdir
             tfile = self.fm.thisfile
@@ -737,27 +740,42 @@ class trash(Command):
                 self.fm.notify("Error: no file selected for deletion!", bad=True)
                 return
 
+            files = self.fm.thistab.get_selection()
             # relative_path used for a user-friendly output in the confirmation.
-            files = [f.relative_path for f in self.fm.thistab.get_selection()]
+            file_names = [f.relative_path for f in files]
             many_files = (cwd.marked_items or is_directory_with_files(tfile.path))
 
         confirm = self.fm.settings.confirm_on_delete
         if confirm != 'never' and (confirm != 'multiple' or many_files):
             self.fm.ui.console.ask(
-                "Confirm deletion of: %s (y/N)" % ', '.join(files),
+                "Confirm deletion of: %s (y/N)" % ', '.join(file_names),
                 partial(self._question_callback, files),
                 ('n', 'N', 'y', 'Y'),
             )
         else:
             # no need for a confirmation, just delete
-            self.fm.execute_file(files, label='trash')
+            self._trash_files_catch_arg_list_error(files)
 
     def tab(self, tabnum):
         return self._tab_directory_content()
 
     def _question_callback(self, files, answer):
         if answer.lower() == 'y':
+            self._trash_files_catch_arg_list_error(files)
+
+    def _trash_files_catch_arg_list_error(self, files):
+        """
+        Executes the fm.execute_file method but catches the OSError ("Argument list too long")
+        that occurs when moving too many files to trash (and would otherwise crash ranger).
+        """
+        try:
             self.fm.execute_file(files, label='trash')
+        except OSError as err:
+            if err.errno == 7:
+                self.fm.notify("Error: Command too long (try passing less files at once)",
+                               bad=True)
+            else:
+                raise
 
 
 class jump_non(Command):
diff --git a/ranger/core/fm.py b/ranger/core/fm.py
index 52cd83d6..57bc9af4 100644
--- a/ranger/core/fm.py
+++ b/ranger/core/fm.py
@@ -329,6 +329,44 @@ class FM(Actions,  # pylint: disable=too-many-instance-attributes
             self.directories[path] = obj
             return obj
 
+    @staticmethod
+    def group_paths_by_dirname(paths):
+        """
+        Groups the paths into a dictionary with their dirnames as keys and a set of
+        basenames as entries.
+        """
+        groups = dict()
+        for path in paths:
+            abspath = os.path.abspath(os.path.expanduser(path))
+            dirname, basename = os.path.split(abspath)
+            groups.setdefault(dirname, set()).add(basename)
+        return groups
+
+    def get_filesystem_objects(self, paths):
+        """
+        Find FileSystemObjects corresponding to the paths if they are already in
+        memory and load those that are not.
+        """
+        result = []
+        # Grouping the files by dirname avoids the potentially quadratic running time of doing
+        # a linear search in the directory for each entry name that is supposed to be deleted.
+        groups = self.group_paths_by_dirname(paths)
+        for dirname, basenames in groups.items():
+            directory = self.fm.get_directory(dirname)
+            directory.load_content_if_outdated()
+            for entry in directory.files_all:
+                if entry.basename in basenames:
+                    result.append(entry)
+                    basenames.remove(entry.basename)
+            if basenames != set():
+                # Abort the operation with an error message if there are entries
+                # that weren't found.
+                names = ', '.join(basenames)
+                self.fm.notify('Error: No such file or directory: {0}'.format(
+                    names), bad=True)
+                return None
+        return result
+
     def garbage_collect(
             self, age,
             tabs=None):  # tabs=None is for COMPATibility pylint: disable=unused-argument