Skip to content

Commit d9baafa

Browse files
authored
Merge pull request #3673 from takluyver/win-dir-notrash
Don't trash non-empty directories on Windows
2 parents ebe0176 + 7ea7a8b commit d9baafa

File tree

2 files changed

+19
-7
lines changed

2 files changed

+19
-7
lines changed

notebook/services/contents/filemanager.py

+16-7
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,21 @@ def _check_trash(os_path):
517517
home_dev = os.stat(os.path.expanduser('~')).st_dev
518518
return file_dev == home_dev
519519

520+
def is_non_empty_dir(os_path):
521+
if os.path.isdir(os_path):
522+
# A directory containing only leftover checkpoints is
523+
# considered empty.
524+
cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None)
525+
if set(os.listdir(os_path)) - {cp_dir}:
526+
return True
527+
528+
return False
529+
520530
if self.delete_to_trash:
531+
if sys.platform == 'win32' and is_non_empty_dir(os_path):
532+
# send2trash can really delete files on Windows, so disallow
533+
# deleting non-empty files. See Github issue 3631.
534+
raise web.HTTPError(400, u'Directory %s not empty' % os_path)
521535
if _check_trash(os_path):
522536
self.log.debug("Sending %s to trash", os_path)
523537
# Looking at the code in send2trash, I don't think the errors it
@@ -530,14 +544,9 @@ def _check_trash(os_path):
530544
"to home directory", os_path)
531545

532546
if os.path.isdir(os_path):
533-
listing = os.listdir(os_path)
534547
# Don't permanently delete non-empty directories.
535-
# A directory containing only leftover checkpoints is
536-
# considered empty.
537-
cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None)
538-
for entry in listing:
539-
if entry != cp_dir:
540-
raise web.HTTPError(400, u'Directory %s not empty' % os_path)
548+
if is_non_empty_dir(os_path):
549+
raise web.HTTPError(400, u'Directory %s not empty' % os_path)
541550
self.log.debug("Removing directory %s", os_path)
542551
with self.perm_to_403():
543552
shutil.rmtree(os_path)

notebook/services/contents/tests/test_contents_api.py

+3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import json
88
import os
99
import shutil
10+
import sys
1011
from unicodedata import normalize
1112

1213
pjoin = os.path.join
@@ -523,6 +524,8 @@ def test_delete_dirs(self):
523524
self.assertEqual(listing, [])
524525

525526
def test_delete_non_empty_dir(self):
527+
if sys.platform == 'win32':
528+
self.skipTest("Disabled deleting non-empty dirs on Windows")
526529
# Test that non empty directory can be deleted
527530
self.api.delete(u'å b')
528531
# Check if directory has actually been deleted

0 commit comments

Comments
 (0)