From ed15a0711650d8bac1fb9df6b770cea58a3f4938 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Mon, 28 Aug 2017 23:19:45 -0400 Subject: [PATCH 1/8] Do not allow copying a folder into itself --- lib/tree-view.coffee | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/tree-view.coffee b/lib/tree-view.coffee index 87045bdf..c1958a72 100644 --- a/lib/tree-view.coffee +++ b/lib/tree-view.coffee @@ -706,7 +706,7 @@ class TreeView window.localStorage.removeItem('tree-view:cutPath') window.localStorage['tree-view:copyPath'] = JSON.stringify(selectedPaths) - # Public: Copy the path of the selected entry element. + # Public: Cut the path of the selected entry element. # Save the path in localStorage, so that cutting from 2 different # instances of atom works as intended # @@ -744,6 +744,10 @@ class TreeView basePath = path.dirname(basePath) if selectedEntry.classList.contains('file') newPath = path.join(basePath, path.basename(initialPath)) + if basePath.startsWith(initialPath) # For example, trying to copy test/a/ into test/a/b/ + atom.notifications.addWarning('Cannot copy a folder into itself') + continue + if copiedPaths # append a number to the file if an item with the same name exists fileCounter = 0 From 6f613f304a2d59ccd5cf00b29aabda738a9fbef5 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Mon, 28 Aug 2017 23:24:51 -0400 Subject: [PATCH 2/8] Do not allow moving a folder into itself --- lib/tree-view.coffee | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/tree-view.coffee b/lib/tree-view.coffee index c1958a72..e9773453 100644 --- a/lib/tree-view.coffee +++ b/lib/tree-view.coffee @@ -745,7 +745,7 @@ class TreeView newPath = path.join(basePath, path.basename(initialPath)) if basePath.startsWith(initialPath) # For example, trying to copy test/a/ into test/a/b/ - atom.notifications.addWarning('Cannot copy a folder into itself') + atom.notifications.addWarning('Cannot paste a folder into itself') continue if copiedPaths @@ -772,9 +772,8 @@ class TreeView fs.writeFileSync(newPath, fs.readFileSync(initialPath)) @emitter.emit 'entry-copied', {initialPath, newPath} else if cutPaths - # Only move the target if the cut target doesn't exist and if the newPath - # is not within the initial path - unless fs.existsSync(newPath) or newPath.startsWith(initialPath) + # Only move the target if the cut target doesn't exist + unless fs.existsSync(newPath) try @emitter.emit 'will-move-entry', {initialPath, newPath} fs.moveSync(initialPath, newPath) @@ -865,8 +864,12 @@ class TreeView if initialPath is newDirectoryPath return + if newDirectoryPath.startsWith(initialPath) + atom.notifications.addWarning('Cannot move a folder into itself') + return + entryName = path.basename(initialPath) - newPath = "#{newDirectoryPath}/#{entryName}".replace(/\s+$/, '') + newPath = path.join(newDirectoryPath, entryName).replace(/\s+$/, '') try @emitter.emit 'will-move-entry', {initialPath, newPath} From b12d049f0cc25d88c7628ce417e78ff38a0e1200 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Mon, 4 Sep 2017 13:59:01 +0200 Subject: [PATCH 3/8] Specs --- spec/tree-view-package-spec.coffee | 78 +++++++++++++++++------------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/spec/tree-view-package-spec.coffee b/spec/tree-view-package-spec.coffee index e19181e0..56a5e68b 100644 --- a/spec/tree-view-package-spec.coffee +++ b/spec/tree-view-package-spec.coffee @@ -1510,41 +1510,35 @@ describe "TreeView", -> LocalStorage.clear() describe "when attempting to paste a directory into itself", -> - describe "when copied", -> - beforeEach -> - LocalStorage['tree-view:copyPath'] = JSON.stringify([dirPath]) - - it "makes a copy inside itself", -> - newPath = path.join(dirPath, path.basename(dirPath)) - dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) - expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() - expect(fs.existsSync(newPath)).toBeTruthy() - - it "dispatches an event to the tree-view", -> - newPath = path.join(dirPath, path.basename(dirPath)) - callback = jasmine.createSpy("onEntryCopied") - treeView.onEntryCopied(callback) - - dirView.click() - atom.commands.dispatch(treeView.element, "tree-view:paste") - expect(callback).toHaveBeenCalledWith(initialPath: dirPath, newPath: newPath) - - it 'does not keep copying recursively', -> - LocalStorage['tree-view:copyPath'] = JSON.stringify([dirPath]) - dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) - - newPath = path.join(dirPath, path.basename(dirPath)) - expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() - expect(fs.existsSync(newPath)).toBeTruthy() - expect(fs.existsSync(path.join(newPath, path.basename(dirPath)))).toBeFalsy() + beforeEach -> + atom.notifications.clear() - describe "when cut", -> - it "does nothing", -> - LocalStorage['tree-view:cutPath'] = JSON.stringify([dirPath]) - dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) + operations = + "copy": "copied" + "cut": "cut" - expect(fs.existsSync(dirPath)).toBeTruthy() - expect(fs.existsSync(path.join(dirPath, path.basename(dirPath)))).toBeFalsy() + for operation, text of operations + describe "when #{text}", -> + it "shows a warning notification and does not paste", -> + LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) + newPath = path.join(dirPath, path.basename(dirPath)) + dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) + expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() + expect(fs.existsSync(newPath)).toBe false + expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot paste a folder into itself' + atom.notifications.clear() + + nestedPath = path.join(dirPath, 'nested') + fs.makeTreeSync(nestedPath) + + LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) + newPath = path.join(nestedPath, path.basename(dirPath)) + dirView.expand() + nestedView = dirView.querySelector('.directory') + nestedView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) + expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() + expect(fs.existsSync(newPath)).toBe false + expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot paste a folder into itself' describe "when pasting entries which don't exist anymore", -> it "skips the entry which doesn't exist", -> @@ -3717,6 +3711,24 @@ describe "TreeView", -> expect(editors[0].getPath()).toBe thetaFilePath.replace('gamma', 'alpha') expect(editors[1].getPath()).toBe thetaFilePath2 + it "shows a warning notification and does not move the directory if it would result in recursive copying", -> + # Dragging alphaDir onto etaDir, which is a child of alphaDir's + alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha') + alphaDir.expand() + + etaDir = alphaDir.entries.children[0] + etaDir.expand() + + [dragStartEvent, dragEnterEvent, dropEvent] = + eventHelpers.buildInternalDragEvents(alphaDir, etaDir.querySelector('.header'), etaDir) + treeView.onDragStart(dragStartEvent) + treeView.onDrop(dropEvent) + expect(etaDir.children.length).toBe 2 + etaDir.expand() + expect(etaDir.querySelector('.entries').children.length).toBe 0 + + expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot move a folder into itself' + describe "when dragging a file from the OS onto a DirectoryView's header", -> it "should move the file to the hovered directory", -> # Dragging delta.txt from OS file explorer onto alphaDir From 969f6b199d96ffd888a2042d02dd3e98cdf2ff24 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Fri, 8 Sep 2017 20:02:17 +0200 Subject: [PATCH 4/8] Fix some edge cases when moving across siblings --- lib/tree-view.coffee | 9 ++++++--- spec/tree-view-package-spec.coffee | 30 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/lib/tree-view.coffee b/lib/tree-view.coffee index e9773453..0dcfb407 100644 --- a/lib/tree-view.coffee +++ b/lib/tree-view.coffee @@ -742,9 +742,11 @@ class TreeView if selectedEntry and initialPath and fs.existsSync(initialPath) basePath = selectedEntry.getPath() basePath = path.dirname(basePath) if selectedEntry.classList.contains('file') + basePath += path.sep # Helps out with the recursive copying check below newPath = path.join(basePath, path.basename(initialPath)) - if basePath.startsWith(initialPath) # For example, trying to copy test/a/ into test/a/b/ + # Do not allow copying test/a/ into test/a/b/ + if initialPathIsDirectory and basePath.startsWith(initialPath + path.sep) atom.notifications.addWarning('Cannot paste a folder into itself') continue @@ -761,7 +763,7 @@ class TreeView newPath = "#{filePath}#{fileCounter}#{extension}" fileCounter += 1 - if fs.isDirectorySync(initialPath) + if initialPathIsDirectory # use fs.copy to copy directories since read/write will fail for directories catchAndShowFileErrors => fs.copySync(initialPath, newPath) @@ -864,7 +866,8 @@ class TreeView if initialPath is newDirectoryPath return - if newDirectoryPath.startsWith(initialPath) + console.log newDirectoryPath + if fs.isDirectorySync(initialPath) and newDirectoryPath.startsWith(initialPath + path.sep) atom.notifications.addWarning('Cannot move a folder into itself') return diff --git a/spec/tree-view-package-spec.coffee b/spec/tree-view-package-spec.coffee index 56a5e68b..2f3ddf11 100644 --- a/spec/tree-view-package-spec.coffee +++ b/spec/tree-view-package-spec.coffee @@ -1520,6 +1520,7 @@ describe "TreeView", -> for operation, text of operations describe "when #{text}", -> it "shows a warning notification and does not paste", -> + # /dir-1/ -> /dir-1/ LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) newPath = path.join(dirPath, path.basename(dirPath)) dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) @@ -1531,6 +1532,7 @@ describe "TreeView", -> nestedPath = path.join(dirPath, 'nested') fs.makeTreeSync(nestedPath) + # /dir-1/ -> /dir-1/nested LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) newPath = path.join(nestedPath, path.basename(dirPath)) dirView.expand() @@ -1539,6 +1541,16 @@ describe "TreeView", -> expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() expect(fs.existsSync(newPath)).toBe false expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot paste a folder into itself' + atom.notifications.clear() + dirView.collapse() + + # /dir-1/ -> /dir-2/ + LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) + newPath = path.join(dirPath2, path.basename(dirPath)) + dirView2.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) + expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() + expect(fs.existsSync(newPath)).toBe true + expect(atom.notifications.getNotifications()[0]).toBeUndefined() describe "when pasting entries which don't exist anymore", -> it "skips the entry which doesn't exist", -> @@ -3557,6 +3569,8 @@ describe "TreeView", -> thetaDirPath = path.join(gammaDirPath, "theta") thetaFilePath = path.join(thetaDirPath, "theta.txt") + alpha2DirPath = path.join(rootDirPath, "alpha2") + fs.writeFileSync(alphaFilePath, "doesn't matter") fs.writeFileSync(zetaFilePath, "doesn't matter") @@ -3570,6 +3584,8 @@ describe "TreeView", -> fs.makeTreeSync(thetaDirPath) fs.writeFileSync(thetaFilePath, "doesn't matter") + fs.makeTreeSync(alpha2DirPath) + atom.project.setPaths([rootDirPath]) describe "when dragging a FileView onto a DirectoryView's header", -> @@ -3728,6 +3744,20 @@ describe "TreeView", -> expect(etaDir.querySelector('.entries').children.length).toBe 0 expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot move a folder into itself' + atom.notifications.clear() + alphaDir.collapse() + + # Dragging alpha onto alpha2, which is a sibling of alpha's + alpha2Dir = findDirectoryContainingText(treeView.roots[0], 'alpha2') + [dragStartEvent, dragEnterEvent, dropEvent] = + eventHelpers.buildInternalDragEvents(alphaDir, etaDir.querySelector('.header'), alpha2Dir) + treeView.onDragStart(dragStartEvent) + treeView.onDrop(dropEvent) + expect(alpha2Dir.children.length).toBe 2 + alpha2Dir.expand() + expect(alpha2Dir.querySelector('.entries').children.length).toBe 1 + + expect(atom.notifications.getNotifications()[0]).toBeUndefined() describe "when dragging a file from the OS onto a DirectoryView's header", -> it "should move the file to the hovered directory", -> From 955321540c18c42a05c4110126adc6311530bc61 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Fri, 8 Sep 2017 23:42:47 +0200 Subject: [PATCH 5/8] Handle symlinks --- lib/tree-view.coffee | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/tree-view.coffee b/lib/tree-view.coffee index 0dcfb407..4c85d774 100644 --- a/lib/tree-view.coffee +++ b/lib/tree-view.coffee @@ -742,13 +742,16 @@ class TreeView if selectedEntry and initialPath and fs.existsSync(initialPath) basePath = selectedEntry.getPath() basePath = path.dirname(basePath) if selectedEntry.classList.contains('file') - basePath += path.sep # Helps out with the recursive copying check below newPath = path.join(basePath, path.basename(initialPath)) # Do not allow copying test/a/ into test/a/b/ - if initialPathIsDirectory and basePath.startsWith(initialPath + path.sep) - atom.notifications.addWarning('Cannot paste a folder into itself') - continue + # Note: A trailing path.sep is added to prevent false positives, such as test/a -> test/ab + realBasePath = fs.realpathSync(basePath) + path.sep + realInitialPath = fs.realpathSync(initialPath) + path.sep + if initialPathIsDirectory and realBasePath.startsWith(realInitialPath) + unless fs.isSymbolicLinkSync(initialPath) + atom.notifications.addWarning('Cannot paste a folder into itself') + continue if copiedPaths # append a number to the file if an item with the same name exists @@ -866,10 +869,12 @@ class TreeView if initialPath is newDirectoryPath return - console.log newDirectoryPath - if fs.isDirectorySync(initialPath) and newDirectoryPath.startsWith(initialPath + path.sep) - atom.notifications.addWarning('Cannot move a folder into itself') - return + realNewDirectoryPath = fs.realpathSync(newDirectoryPath) + path.sep + realInitialPath = fs.realpathSync(initialPath) + path.sep + if fs.isDirectorySync(initialPath) and realNewDirectoryPath.startsWith(realInitialPath) + unless fs.isSymbolicLinkSync(initialPath) + atom.notifications.addWarning('Cannot move a folder into itself') + return entryName = path.basename(initialPath) newPath = path.join(newDirectoryPath, entryName).replace(/\s+$/, '') From 9315040560b2bff18aa1b8b654e0d919cd2192d6 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sat, 9 Sep 2017 01:09:17 +0200 Subject: [PATCH 6/8] Add symlink specs These specs can probably be cleaned up a bit --- spec/tree-view-package-spec.coffee | 153 ++++++++++++++++++++--------- 1 file changed, 108 insertions(+), 45 deletions(-) diff --git a/spec/tree-view-package-spec.coffee b/spec/tree-view-package-spec.coffee index 2f3ddf11..46571a51 100644 --- a/spec/tree-view-package-spec.coffee +++ b/spec/tree-view-package-spec.coffee @@ -1508,49 +1508,69 @@ describe "TreeView", -> beforeEach -> LocalStorage.clear() + atom.notifications.clear() - describe "when attempting to paste a directory into itself", -> - beforeEach -> - atom.notifications.clear() - - operations = - "copy": "copied" - "cut": "cut" - - for operation, text of operations - describe "when #{text}", -> - it "shows a warning notification and does not paste", -> - # /dir-1/ -> /dir-1/ - LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) - newPath = path.join(dirPath, path.basename(dirPath)) - dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) - expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() - expect(fs.existsSync(newPath)).toBe false - expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot paste a folder into itself' - atom.notifications.clear() - - nestedPath = path.join(dirPath, 'nested') - fs.makeTreeSync(nestedPath) - - # /dir-1/ -> /dir-1/nested - LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) - newPath = path.join(nestedPath, path.basename(dirPath)) - dirView.expand() - nestedView = dirView.querySelector('.directory') - nestedView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) - expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() - expect(fs.existsSync(newPath)).toBe false - expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot paste a folder into itself' - atom.notifications.clear() - dirView.collapse() - - # /dir-1/ -> /dir-2/ - LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) - newPath = path.join(dirPath2, path.basename(dirPath)) - dirView2.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) - expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() - expect(fs.existsSync(newPath)).toBe true - expect(atom.notifications.getNotifications()[0]).toBeUndefined() + for operation in ['copy', 'cut'] + describe "when attempting to #{operation} and paste a directory into itself", -> + it "shows a warning notification and does not paste", -> + # /dir-1/ -> /dir-1/ + LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) + newPath = path.join(dirPath, path.basename(dirPath)) + dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) + expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() + expect(fs.existsSync(newPath)).toBe false + expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot paste a folder into itself' + + describe "when attempting to #{operation} and paste a directory into a nested child directory", -> + it "shows a warning notification and does not paste", -> + nestedPath = path.join(dirPath, 'nested') + fs.makeTreeSync(nestedPath) + + # /dir-1/ -> /dir-1/nested/ + LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) + newPath = path.join(nestedPath, path.basename(dirPath)) + dirView.reload() + nestedView = dirView.querySelector('.directory') + nestedView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) + expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() + expect(fs.existsSync(newPath)).toBe false + expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot paste a folder into itself' + + describe "when attempting to #{operation} and paste a directory into a sibling directory that starts with the same letter", -> + it "allows the paste to occur", -> + # /dir-1/ -> /dir-2/ + LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) + newPath = path.join(dirPath2, path.basename(dirPath)) + dirView2.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) + expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() + expect(fs.existsSync(newPath)).toBe true + expect(atom.notifications.getNotifications()[0]).toBeUndefined() + + describe "when attempting to #{operation} and paste a directory into a symlink of itself", -> + it "shows a warning notification and does not paste", -> + fs.symlinkSync(dirPath, path.join(rootDirPath, 'symdir'), 'junction') + + # /dir-1/ -> symlink of /dir-1/ + LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([dirPath]) + newPath = path.join(dirPath, path.basename(dirPath)) + symlinkView = root1.querySelector('.directory') + symlinkView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) + expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() + expect(fs.existsSync(newPath)).toBe false + expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot paste a folder into itself' + + describe "when attempting to #{operation} and paste a symlink into its target directory", -> + it "allows the paste to occur", -> + symlinkedPath = path.join(rootDirPath, 'symdir') + fs.symlinkSync(dirPath, symlinkedPath, 'junction') + + # symlink of /dir-1/ -> /dir-1/ + LocalStorage["tree-view:#{operation}Path"] = JSON.stringify([symlinkedPath]) + newPath = path.join(dirPath, path.basename(symlinkedPath)) + dirView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1})) + expect(-> atom.commands.dispatch(treeView.element, "tree-view:paste")).not.toThrow() + expect(fs.existsSync(newPath)).toBe true + expect(atom.notifications.getNotifications()[0]).toBeUndefined() describe "when pasting entries which don't exist anymore", -> it "skips the entry which doesn't exist", -> @@ -3571,6 +3591,8 @@ describe "TreeView", -> alpha2DirPath = path.join(rootDirPath, "alpha2") + symlinkToAlphaDirPath = path.join(rootDirPath, "symalpha") + fs.writeFileSync(alphaFilePath, "doesn't matter") fs.writeFileSync(zetaFilePath, "doesn't matter") @@ -3586,7 +3608,10 @@ describe "TreeView", -> fs.makeTreeSync(alpha2DirPath) + fs.symlinkSync(alphaDirPath, symlinkToAlphaDirPath, 'junction') + atom.project.setPaths([rootDirPath]) + atom.notifications.clear() describe "when dragging a FileView onto a DirectoryView's header", -> it "should add the selected class to the DirectoryView", -> @@ -3744,13 +3769,33 @@ describe "TreeView", -> expect(etaDir.querySelector('.entries').children.length).toBe 0 expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot move a folder into itself' - atom.notifications.clear() - alphaDir.collapse() + it "shows a warning notification and does not move the directory if it would result in recursive copying (symlink)", -> + # Dragging alphaDir onto symalpha, which is a symlink to alphaDir + alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha') + alphaDir.expand() + + symlinkDir = treeView.roots[0].entries.children[3] + symlinkDir.expand() + + [dragStartEvent, dragEnterEvent, dropEvent] = + eventHelpers.buildInternalDragEvents(alphaDir, symlinkDir.querySelector('.header'), symlinkDir) + treeView.onDragStart(dragStartEvent) + treeView.onDrop(dropEvent) + expect(symlinkDir.children.length).toBe 2 + symlinkDir.expand() + expect(symlinkDir.querySelector('.entries').children.length).toBe 2 + + expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot move a folder into itself' + + it "moves successfully when dragging a directory onto a sibling directory that starts with the same letter", -> # Dragging alpha onto alpha2, which is a sibling of alpha's + alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha') + alphaDir.expand() + alpha2Dir = findDirectoryContainingText(treeView.roots[0], 'alpha2') [dragStartEvent, dragEnterEvent, dropEvent] = - eventHelpers.buildInternalDragEvents(alphaDir, etaDir.querySelector('.header'), alpha2Dir) + eventHelpers.buildInternalDragEvents(alphaDir, alpha2Dir.querySelector('.header'), alpha2Dir) treeView.onDragStart(dragStartEvent) treeView.onDrop(dropEvent) expect(alpha2Dir.children.length).toBe 2 @@ -3759,6 +3804,24 @@ describe "TreeView", -> expect(atom.notifications.getNotifications()[0]).toBeUndefined() + it "moves successfully when dragging a symlink into its target directory", -> + # Dragging alphaDir onto symalpha, which is a symlink to alphaDir + alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha') + alphaDir.expand() + + symlinkDir = treeView.roots[0].entries.children[3] + symlinkDir.expand() + + [dragStartEvent, dragEnterEvent, dropEvent] = + eventHelpers.buildInternalDragEvents(symlinkDir, alphaDir.querySelector('.header'), alphaDir) + treeView.onDragStart(dragStartEvent) + treeView.onDrop(dropEvent) + expect(alphaDir.children.length).toBe 2 + alphaDir.reload() + expect(alphaDir.querySelector('.entries').children.length).toBe 3 + + expect(atom.notifications.getNotifications()[0]).toBeUndefined() + describe "when dragging a file from the OS onto a DirectoryView's header", -> it "should move the file to the hovered directory", -> # Dragging delta.txt from OS file explorer onto alphaDir From 6c51822c082facda26ecbb8e2feaf4ee4022f2d2 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Thu, 22 Feb 2018 23:56:42 -0500 Subject: [PATCH 7/8] Get specs passing --- spec/tree-view-package-spec.coffee | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/spec/tree-view-package-spec.coffee b/spec/tree-view-package-spec.coffee index 5c925447..318bf18f 100644 --- a/spec/tree-view-package-spec.coffee +++ b/spec/tree-view-package-spec.coffee @@ -64,7 +64,7 @@ describe "TreeView", -> expect(root1.directory.watchSubscription).toBeTruthy() afterEach -> - # temp.cleanup() + temp.cleanup() if treeViewOpenPromise = atom.packages.getActivePackage('tree-view')?.mainModule.treeViewOpenPromise waitsForPromise -> treeViewOpenPromise @@ -3659,7 +3659,7 @@ describe "TreeView", -> entries: new Map()) describe "Dragging and dropping files", -> - [alphaDirPath, betaFilePath, etaDirPath, gammaDirPath, deltaFilePath, epsilonFilePath, thetaFilePath] = [] + [alphaDirPath, alphaFilePath, betaFilePath, etaDirPath, gammaDirPath, deltaFilePath, epsilonFilePath, thetaFilePath] = [] beforeEach -> rootDirPath = fs.absolute(temp.mkdirSync('tree-view')) @@ -3887,7 +3887,7 @@ describe "TreeView", -> describe "when dropping a DirectoryView and FileViews onto a DirectoryView's header", -> it "should move the files and directory to the hovered directory", -> # Dragging alpha.txt and alphaDir into thetaDir - alphaFile = treeView.roots[0].entries.children[2] + alphaFile = Array.from(treeView.roots[0].entries.children).find (element) -> element.getPath() is alphaFilePath alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha') alphaDir.expand() @@ -3979,7 +3979,7 @@ describe "TreeView", -> expect(editors[0].getPath()).toBe thetaFilePath.replace('gamma', 'alpha') expect(editors[1].getPath()).toBe thetaFilePath2 - it "shows a warning notification and does not move the directory if it would result in recursive copying", -> + it "does not move the directory if it would result in recursive copying", -> # Dragging alphaDir onto etaDir, which is a child of alphaDir's alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha') alphaDir.expand() @@ -3995,8 +3995,6 @@ describe "TreeView", -> etaDir.expand() expect(etaDir.querySelector('.entries').children.length).toBe 0 - expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot move a folder into itself' - it "shows a warning notification and does not move the directory if it would result in recursive copying (symlink)", -> # Dragging alphaDir onto symalpha, which is a symlink to alphaDir alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha') From 99be6c7db7863c5aec856643d23957e52964cc54 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Fri, 23 Feb 2018 10:17:53 -0500 Subject: [PATCH 8/8] Add back warning notification --- lib/tree-view.coffee | 6 ++++-- spec/tree-view-package-spec.coffee | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/tree-view.coffee b/lib/tree-view.coffee index b88fabf5..d7af1408 100644 --- a/lib/tree-view.coffee +++ b/lib/tree-view.coffee @@ -1134,11 +1134,13 @@ class TreeView return if initialPaths.includes(newDirectoryPath) entry.classList.remove('drag-over', 'selected') - parentSelected = entry.parentNode.closest('.entry.selected') - return if parentSelected # iterate backwards so files in a dir are moved before the dir itself for initialPath in initialPaths by -1 + # Note: this is necessary on Windows to circumvent node-pathwatcher + # holding a lock on expanded folders and preventing them from + # being moved or deleted + # TODO: This can be removed when tree-view is switched to @atom/watcher @entryForPath(initialPath)?.collapse?() @moveEntry(initialPath, newDirectoryPath) else diff --git a/spec/tree-view-package-spec.coffee b/spec/tree-view-package-spec.coffee index 318bf18f..ce1dce36 100644 --- a/spec/tree-view-package-spec.coffee +++ b/spec/tree-view-package-spec.coffee @@ -3979,7 +3979,7 @@ describe "TreeView", -> expect(editors[0].getPath()).toBe thetaFilePath.replace('gamma', 'alpha') expect(editors[1].getPath()).toBe thetaFilePath2 - it "does not move the directory if it would result in recursive copying", -> + it "shows a warning notification and does not move the directory if it would result in recursive copying", -> # Dragging alphaDir onto etaDir, which is a child of alphaDir's alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha') alphaDir.expand() @@ -3995,6 +3995,8 @@ describe "TreeView", -> etaDir.expand() expect(etaDir.querySelector('.entries').children.length).toBe 0 + expect(atom.notifications.getNotifications()[0].getMessage()).toContain 'Cannot move a folder into itself' + it "shows a warning notification and does not move the directory if it would result in recursive copying (symlink)", -> # Dragging alphaDir onto symalpha, which is a symlink to alphaDir alphaDir = findDirectoryContainingText(treeView.roots[0], 'alpha')