contents service review

- missed some s/notebook/file/ in docstrings
- manager doesn’t sort contents

IPython/html/services/contents/filemanager.py

@@ -19,10 +19,6 @@ from IPython.utils.py3compat import getcwd
 from IPython.utils import tz
 from IPython.html.utils import is_hidden, to_os_path
 
-def sort_key(item):
-    """Case-insensitive sorting."""
-    return item['name'].lower()
-
 
 class FileContentsManager(ContentsManager):
 
@@ -38,9 +34,9 @@ class FileContentsManager(ContentsManager):
             raise TraitError("%r is not a directory" % new)
 
     checkpoint_dir = Unicode('.ipynb_checkpoints', config=True,
-        help="""The directory name in which to keep notebook checkpoints
+        help="""The directory name in which to keep file checkpoints
 
-        This is a path relative to the notebook's own directory.
+        This is a path relative to the file's own directory.
 
         By default, it is .ipynb_checkpoints
         """
@@ -157,7 +153,7 @@ class FileContentsManager(ContentsManager):
         info = os.stat(os_path)
         last_modified = tz.utcfromtimestamp(info.st_mtime)
         created = tz.utcfromtimestamp(info.st_ctime)
-        # Create the notebook model.
+        # Create the base model.
         model = {}
         model['name'] = name
         model['path'] = path
@@ -189,13 +185,12 @@ class FileContentsManager(ContentsManager):
         model['type'] = 'directory'
         dir_path = u'{}/{}'.format(path, name)
         if content:
-            contents = []
+            model['content'] = contents = []
             for os_path in glob.glob(self._get_os_path('*', dir_path)):
                 name = os.path.basename(os_path)
                 if self.should_list(name) and not is_hidden(os_path, self.root_dir):
                     contents.append(self.get_model(name=name, path=dir_path, content=False))
 
-            model['content'] = sorted(contents, key=sort_key)
             model['format'] = 'json'
 
         return model
@@ -204,7 +199,7 @@ class FileContentsManager(ContentsManager):
         """Build a model for a file
 
         if content is requested, include the file contents.
-        Text files will be unicode, binary files will be base64-encoded.
+        UTF-8 text files will be unicode, binary files will be base64-encoded.
         """
         model = self._base_model(name, path)
         model['type'] = 'file'
@@ -251,8 +246,7 @@ class FileContentsManager(ContentsManager):
         name : str
             the name of the target
         path : str
-            the URL path that describes the relative path for
-            the notebook
+            the URL path that describes the relative path for the target
 
         Returns
         -------
@@ -275,6 +269,7 @@ class FileContentsManager(ContentsManager):
         return model
 
     def _save_notebook(self, os_path, model, name='', path=''):
+        """save a notebook file"""
         # Save the notebook file
         nb = current.to_notebook_json(model['content'])
 
@@ -287,6 +282,7 @@ class FileContentsManager(ContentsManager):
             current.write(nb, f, u'json')
 
     def _save_file(self, os_path, model, name='', path=''):
+        """save a non-notebook file"""
         fmt = model.get('format', None)
         if fmt not in {'text', 'base64'}:
             raise web.HTTPError(400, "Must specify format of file contents as 'text' or 'base64'")
@@ -303,6 +299,7 @@ class FileContentsManager(ContentsManager):
             f.write(bcontent)
 
     def _save_directory(self, os_path, model, name='', path=''):
+        """create a directory"""
         if not os.path.exists(os_path):
             os.mkdir(os_path)
         elif not os.path.isdir(os_path):
@@ -442,7 +439,7 @@ class FileContentsManager(ContentsManager):
         # only the one checkpoint ID:
         checkpoint_id = u"checkpoint"
         cp_path = self.get_checkpoint_path(checkpoint_id, name, path)
-        self.log.debug("creating checkpoint for notebook %s", name)
+        self.log.debug("creating checkpoint for %s", name)
         self._copy(src_path, cp_path)
 
         # return the checkpoint info

IPython/html/services/contents/handlers.py

@@ -15,6 +15,16 @@ from IPython.html.base.handlers import (IPythonHandler, json_errors,
                                     file_name_regex)
 
 
+def sort_key(model):
+    """key function for case-insensitive sort by name and type"""
+    iname = model['name'].lower()
+    type_key = {
+        'directory' : '0',
+        'notebook'  : '1',
+        'file'      : '2',
+    }.get(model['type'], '9')
+    return u'%s%s' % (type_key, iname)
+
 class ContentsHandler(IPythonHandler):
 
     SUPPORTED_METHODS = (u'GET', u'PUT', u'PATCH', u'POST', u'DELETE')
@@ -52,16 +62,9 @@ class ContentsHandler(IPythonHandler):
         path = path or ''
         model = self.contents_manager.get_model(name=name, path=path)
         if model['type'] == 'directory':
-            # resort listing to group directories at the top
-            dirs = []
-            files = []
-            for entry in model['content']:
-                if entry['type'] == 'directory':
-                    dirs.append(entry)
-                else:
-                    # do we also want to group notebooks separate from files?
-                    files.append(entry)
-            model['content'] = dirs + files
+            # group listing by type, then by name (case-insensitive)
+            # FIXME: front-ends shouldn't rely on this sorting
+            model['content'].sort(key=sort_key)
         self._finish_model(model, location=False)
 
     @web.authenticated
@@ -130,9 +133,9 @@ class ContentsHandler(IPythonHandler):
     @web.authenticated
     @json_errors
     def post(self, path='', name=None):
-        """Create a new notebook in the specified path.
+        """Create a new file or directory in the specified path.
 
-        POST creates new notebooks. The server always decides on the notebook name.
+        POST creates new files or directories. The server always decides on the name.
 
         POST /api/contents/path
           New untitled notebook in path. If content specified, upload a

IPython/html/services/contents/manager.py

@@ -18,7 +18,10 @@ class ContentsManager(LoggingConfigurable):
     def _notary_default(self):
         return sign.NotebookNotary(parent=self)
 
-    hide_globs = List(Unicode, [u'__pycache__'], config=True, help="""
+    hide_globs = List(Unicode, [
+            u'__pycache__', '*.pyc', '*.pyo',
+            '.DS_Store', '*.so', '*.dylib', '*~',
+        ], config=True, help="""
         Glob patterns to hide in file and directory listings.
     """)
 
@@ -60,14 +63,14 @@ class ContentsManager(LoggingConfigurable):
         raise NotImplementedError
 
     def file_exists(self, name, path=''):
-        """Returns a True if the notebook exists. Else, returns False.
+        """Returns a True if the file exists. Else, returns False.
 
         Parameters
         ----------
         name : string
-            The name of the notebook you are checking.
+            The name of the file you are checking.
         path : string
-            The relative path to the notebook (with '/' as separator)
+            The relative path to the file's directory (with '/' as separator)
 
         Returns
         -------
@@ -87,38 +90,38 @@ class ContentsManager(LoggingConfigurable):
         raise NotImplementedError('must be implemented in a subclass')
 
     def get_model(self, name, path='', content=True):
-        """Get the notebook model with or without content."""
+        """Get the model of a file or directory with or without content."""
         raise NotImplementedError('must be implemented in a subclass')
 
     def save(self, model, name, path=''):
-        """Save the notebook and return the model with no content."""
+        """Save the file or directory and return the model with no content."""
         raise NotImplementedError('must be implemented in a subclass')
 
     def update(self, model, name, path=''):
-        """Update the notebook and return the model with no content."""
+        """Update the file or directory and return the model with no content."""
         raise NotImplementedError('must be implemented in a subclass')
 
     def delete(self, name, path=''):
-        """Delete notebook by name and path."""
+        """Delete file or directory by name and path."""
         raise NotImplementedError('must be implemented in a subclass')
 
     def create_checkpoint(self, name, path=''):
-        """Create a checkpoint of the current state of a notebook
+        """Create a checkpoint of the current state of a file
 
         Returns a checkpoint_id for the new checkpoint.
         """
         raise NotImplementedError("must be implemented in a subclass")
 
     def list_checkpoints(self, name, path=''):
-        """Return a list of checkpoints for a given notebook"""
+        """Return a list of checkpoints for a given file"""
         return []
 
     def restore_checkpoint(self, checkpoint_id, name, path=''):
-        """Restore a notebook from one of its checkpoints"""
+        """Restore a file from one of its checkpoints"""
         raise NotImplementedError("must be implemented in a subclass")
 
     def delete_checkpoint(self, checkpoint_id, name, path=''):
-        """delete a checkpoint for a notebook"""
+        """delete a checkpoint for a file"""
         raise NotImplementedError("must be implemented in a subclass")
 
     def info_string(self):
@@ -139,7 +142,7 @@ class ContentsManager(LoggingConfigurable):
         filename : unicode
             The name of a file, including extension
         path : unicode
-            The URL path of the notebooks directory
+            The URL path of the target's directory
 
         Returns
         -------
@@ -156,7 +159,7 @@ class ContentsManager(LoggingConfigurable):
         return name
 
     def create_file(self, model=None, path='', ext='.ipynb'):
-        """Create a new notebook and return its model with no content."""
+        """Create a new file or directory and return its model with no content."""
         path = path.strip('/')
         if model is None:
             model = {}