From 456e65b1f3c4f53229d8fe431889d60010ffb259 Mon Sep 17 00:00:00 2001
From: Min RK <benjaminrk@gmail.com>
Date: Mon, 10 Nov 2014 13:12:21 -0800
Subject: [PATCH] adjustments to filename increment

- start with no number (Untitled0 -> Untitled.ipynb)
- copy of copy increments instead of adding another `-Copy` (copy Foo-Copy1.ipynb gives Foo-Copy2.ipynb, not Foo-Copy1-Copy1.ipynb)
- copy file to new folder starts with the original filename, instead of unconditional `-Copy0`
---
 IPython/html/services/contents/manager.py     | 21 ++++++++----
 .../contents/tests/test_contents_api.py       | 34 ++++++++++++-------
 .../services/contents/tests/test_manager.py   | 24 +++++++------
 3 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/IPython/html/services/contents/manager.py b/IPython/html/services/contents/manager.py
index 359cbd151..8274eddca 100644
--- a/IPython/html/services/contents/manager.py
+++ b/IPython/html/services/contents/manager.py
@@ -7,6 +7,7 @@ from fnmatch import fnmatch
 import itertools
 import json
 import os
+import re
 
 from tornado.web import HTTPError
 
@@ -15,6 +16,7 @@ from IPython.nbformat import sign, validate, ValidationError
 from IPython.nbformat.v4 import new_notebook
 from IPython.utils.traitlets import Instance, Unicode, List
 
+copy_pat = re.compile(r'\-Copy\d*\.')
 
 class ContentsManager(LoggingConfigurable):
     """Base class for serving files and directories.
@@ -188,7 +190,7 @@ class ContentsManager(LoggingConfigurable):
         """
         return path
 
-    def increment_filename(self, filename, path=''):
+    def increment_filename(self, filename, path='', insert=''):
         """Increment a filename until it is unique.
 
         Parameters
@@ -206,8 +208,12 @@ class ContentsManager(LoggingConfigurable):
         path = path.strip('/')
         basename, ext = os.path.splitext(filename)
         for i in itertools.count():
-            name = u'{basename}{i}{ext}'.format(basename=basename, i=i,
-                                                ext=ext)
+            if i:
+                insert_i = '{}{}'.format(insert, i)
+            else:
+                insert_i = ''
+            name = u'{basename}{insert}{ext}'.format(basename=basename,
+                insert=insert_i, ext=ext)
             if not self.exists(u'{}/{}'.format(path, name)):
                 break
         return name
@@ -244,8 +250,10 @@ class ContentsManager(LoggingConfigurable):
         else:
             model.setdefault('type', 'file')
         
+        insert = ''
         if model['type'] == 'directory':
             untitled = self.untitled_directory
+            insert = ' '
         elif model['type'] == 'notebook':
             untitled = self.untitled_notebook
             ext = '.ipynb'
@@ -254,7 +262,7 @@ class ContentsManager(LoggingConfigurable):
         else:
             raise HTTPError(400, "Unexpected model type: %r" % model['type'])
         
-        name = self.increment_filename(untitled + ext, path)
+        name = self.increment_filename(untitled + ext, path, insert=insert)
         path = u'{0}/{1}'.format(path, name)
         return self.new(model, path)
     
@@ -309,9 +317,8 @@ class ContentsManager(LoggingConfigurable):
         if not to_path:
             to_path = from_dir
         if self.dir_exists(to_path):
-            base, ext = os.path.splitext(from_name)
-            copy_name = u'{0}-Copy{1}'.format(base, ext)
-            to_name = self.increment_filename(copy_name, to_path)
+            name = copy_pat.sub(u'.', from_name)
+            to_name = self.increment_filename(name, to_path, insert='-Copy')
             to_path = u'{0}/{1}'.format(to_path, to_name)
         
         model = self.save(model, to_path)
diff --git a/IPython/html/services/contents/tests/test_contents_api.py b/IPython/html/services/contents/tests/test_contents_api.py
index 80ab66351..db9311a25 100644
--- a/IPython/html/services/contents/tests/test_contents_api.py
+++ b/IPython/html/services/contents/tests/test_contents_api.py
@@ -275,7 +275,7 @@ class APITest(NotebookTestBase):
 
     def test_create_untitled(self):
         resp = self.api.create_untitled(path=u'å b')
-        self._check_created(resp, u'å b/Untitled0.ipynb')
+        self._check_created(resp, u'å b/Untitled.ipynb')
 
         # Second time
         resp = self.api.create_untitled(path=u'å b')
@@ -283,13 +283,13 @@ class APITest(NotebookTestBase):
 
         # And two directories down
         resp = self.api.create_untitled(path='foo/bar')
-        self._check_created(resp, 'foo/bar/Untitled0.ipynb')
+        self._check_created(resp, 'foo/bar/Untitled.ipynb')
 
     def test_create_untitled_txt(self):
         resp = self.api.create_untitled(path='foo/bar', ext='.txt')
-        self._check_created(resp, 'foo/bar/untitled0.txt', type='file')
+        self._check_created(resp, 'foo/bar/untitled.txt', type='file')
 
-        resp = self.api.read(path='foo/bar/untitled0.txt')
+        resp = self.api.read(path='foo/bar/untitled.txt')
         model = resp.json()
         self.assertEqual(model['type'], 'file')
         self.assertEqual(model['format'], 'text')
@@ -304,15 +304,15 @@ class APITest(NotebookTestBase):
 
     def test_mkdir_untitled(self):
         resp = self.api.mkdir_untitled(path=u'å b')
-        self._check_created(resp, u'å b/Untitled Folder0', type='directory')
+        self._check_created(resp, u'å b/Untitled Folder', type='directory')
 
         # Second time
         resp = self.api.mkdir_untitled(path=u'å b')
-        self._check_created(resp, u'å b/Untitled Folder1', type='directory')
+        self._check_created(resp, u'å b/Untitled Folder 1', type='directory')
 
         # And two directories down
         resp = self.api.mkdir_untitled(path='foo/bar')
-        self._check_created(resp, 'foo/bar/Untitled Folder0', type='directory')
+        self._check_created(resp, 'foo/bar/Untitled Folder', type='directory')
 
     def test_mkdir(self):
         path = u'å b/New ∂ir'
@@ -374,15 +374,25 @@ class APITest(NotebookTestBase):
         self.assertEqual(data['content']['nbformat'], 4)
 
     def test_copy(self):
-        resp = self.api.copy(u'å b/ç d.ipynb', u'unicodé')
-        self._check_created(resp, u'unicodé/ç d-Copy0.ipynb')
+        resp = self.api.copy(u'å b/ç d.ipynb', u'å b')
+        self._check_created(resp, u'å b/ç d-Copy1.ipynb')
         
         resp = self.api.copy(u'å b/ç d.ipynb', u'å b')
-        self._check_created(resp, u'å b/ç d-Copy0.ipynb')
-
+        self._check_created(resp, u'å b/ç d-Copy2.ipynb')
+    
+    def test_copy_copy(self):
+        resp = self.api.copy(u'å b/ç d.ipynb', u'å b')
+        self._check_created(resp, u'å b/ç d-Copy1.ipynb')
+        
+        resp = self.api.copy(u'å b/ç d-Copy1.ipynb', u'å b')
+        self._check_created(resp, u'å b/ç d-Copy2.ipynb')
+    
     def test_copy_path(self):
         resp = self.api.copy(u'foo/a.ipynb', u'å b')
-        self._check_created(resp, u'å b/a-Copy0.ipynb')
+        self._check_created(resp, u'å b/a.ipynb')
+        
+        resp = self.api.copy(u'foo/a.ipynb', u'å b')
+        self._check_created(resp, u'å b/a-Copy1.ipynb')
 
     def test_copy_put_400(self):
         with assert_http_error(400):
diff --git a/IPython/html/services/contents/tests/test_manager.py b/IPython/html/services/contents/tests/test_manager.py
index 11a75e068..b51ac9049 100644
--- a/IPython/html/services/contents/tests/test_manager.py
+++ b/IPython/html/services/contents/tests/test_manager.py
@@ -121,8 +121,8 @@ class TestContentsManager(TestCase):
         self.assertIn('path', model)
         self.assertIn('type', model)
         self.assertEqual(model['type'], 'notebook')
-        self.assertEqual(model['name'], 'Untitled0.ipynb')
-        self.assertEqual(model['path'], 'Untitled0.ipynb')
+        self.assertEqual(model['name'], 'Untitled.ipynb')
+        self.assertEqual(model['path'], 'Untitled.ipynb')
 
         # Test in sub-directory
         model = cm.new_untitled(type='directory')
@@ -131,8 +131,8 @@ class TestContentsManager(TestCase):
         self.assertIn('path', model)
         self.assertIn('type', model)
         self.assertEqual(model['type'], 'directory')
-        self.assertEqual(model['name'], 'Untitled Folder0')
-        self.assertEqual(model['path'], 'Untitled Folder0')
+        self.assertEqual(model['name'], 'Untitled Folder')
+        self.assertEqual(model['path'], 'Untitled Folder')
         sub_dir = model['path']
         
         model = cm.new_untitled(path=sub_dir)
@@ -141,8 +141,8 @@ class TestContentsManager(TestCase):
         self.assertIn('path', model)
         self.assertIn('type', model)
         self.assertEqual(model['type'], 'file')
-        self.assertEqual(model['name'], 'untitled0')
-        self.assertEqual(model['path'], '%s/untitled0' % sub_dir)
+        self.assertEqual(model['name'], 'untitled')
+        self.assertEqual(model['path'], '%s/untitled' % sub_dir)
 
     def test_get(self):
         cm = self.contents_manager
@@ -168,7 +168,7 @@ class TestContentsManager(TestCase):
         self.assertIn('name', model2)
         self.assertIn('path', model2)
         self.assertIn('content', model2)
-        self.assertEqual(model2['name'], 'Untitled0.ipynb')
+        self.assertEqual(model2['name'], 'Untitled.ipynb')
         self.assertEqual(model2['path'], '{0}/{1}'.format(sub_dir.strip('/'), name))
     
     @dec.skip_win32
@@ -274,8 +274,8 @@ class TestContentsManager(TestCase):
         assert isinstance(model, dict)
         self.assertIn('name', model)
         self.assertIn('path', model)
-        self.assertEqual(model['name'], 'Untitled0.ipynb')
-        self.assertEqual(model['path'], 'foo/Untitled0.ipynb')
+        self.assertEqual(model['name'], 'Untitled.ipynb')
+        self.assertEqual(model['path'], 'foo/Untitled.ipynb')
 
     def test_delete(self):
         cm = self.contents_manager
@@ -298,12 +298,16 @@ class TestContentsManager(TestCase):
 
         # copy with unspecified name
         copy = cm.copy(path)
-        self.assertEqual(copy['name'], orig['name'].replace('.ipynb', '-Copy0.ipynb'))
+        self.assertEqual(copy['name'], orig['name'].replace('.ipynb', '-Copy1.ipynb'))
 
         # copy with specified name
         copy2 = cm.copy(path, u'å b/copy 2.ipynb')
         self.assertEqual(copy2['name'], u'copy 2.ipynb')
         self.assertEqual(copy2['path'], u'å b/copy 2.ipynb')
+        # copy with specified path
+        copy2 = cm.copy(path, u'/')
+        self.assertEqual(copy2['name'], name)
+        self.assertEqual(copy2['path'], name)
 
     def test_trust_notebook(self):
         cm = self.contents_manager