Refactor sphinx.theming

sphinx/theming.py

@@ -11,9 +11,9 @@
 
 import os
 import shutil
-import zipfile
 import tempfile
 from os import path
+from zipfile import ZipFile
 
 import pkg_resources
 from six import string_types, iteritems
@@ -29,7 +29,7 @@ logger = logging.getLogger(__name__)
 
 if False:
     # For type annotation
-    from typing import Any, Callable, Dict, List, Tuple  # NOQA
+    from typing import Any, Dict, Iterator, List, Tuple  # NOQA
 
 NODEFAULT = object()
 THEMECONF = 'theme.conf'
@@ -117,105 +117,115 @@ class _Theme(object):
             self.base.cleanup()
 
 
+def is_archived_theme(filename):
+    # type: (unicode) -> bool
+    try:
+        with ZipFile(filename) as f:  # type: ignore
+            return THEMECONF in f.namelist()
+    except:
+        return False
+
+
+def find_theme_entries(themedir):
+    # type: (unicode) -> Iterator[Tuple[unicode, unicode]]
+    for entry in os.listdir(themedir):
+        pathname = path.join(themedir, entry)
+        if path.isfile(pathname) and entry.lower().endswith('.zip'):
+            if is_archived_theme(pathname):
+                name = entry[:-4]
+                yield name, pathname
+            else:
+                logger.warning(_('file %r on theme path is not a valid '
+                                 'zipfile or contains no theme'), entry)
+        else:
+            if path.isfile(path.join(pathname, THEMECONF)):
+                yield entry, pathname
+
+
 class Theme(object):
     """
     Represents the theme chosen in the configuration.
     """
-    themes = {}     # type: Dict[unicode, Tuple[unicode, zipfile.ZipFile]]
+    themes = {}     # type: Dict[unicode, unicode]
-    themepath = []  # type: List[unicode]
 
     @classmethod
     def init_themes(cls, confdir, theme_path):
         # type: (unicode, unicode) -> None
         """Search all theme paths for available themes."""
-        cls.themepath = list(theme_path)
+        themepath = list(theme_path)
-        cls.themepath.append(path.join(package_dir, 'themes'))
+        themepath.append(path.join(package_dir, 'themes'))
 
-        for themedir in cls.themepath[::-1]:
+        # search themes from theme_paths
-            themedir = path.join(confdir, themedir)
+        for themedir in themepath:
+            themedir = path.abspath(path.join(confdir, themedir))
             if not path.isdir(themedir):
                 continue
-            for theme in os.listdir(themedir):
+            else:
-                if theme.lower().endswith('.zip'):
+                for name, theme in find_theme_entries(themedir):
-                    try:
+                    cls.themes[name] = theme
-                        zfile = zipfile.ZipFile(path.join(themedir, theme))  # type: ignore
-                        if THEMECONF not in zfile.namelist():
-                            continue
-                        tname = theme[:-4]
-                        tinfo = zfile
-                    except Exception:
-                        logger.warning('file %r on theme path is not a valid '
-                                       'zipfile or contains no theme', theme)
-                        continue
-                else:
-                    if not path.isfile(path.join(themedir, theme, THEMECONF)):
-                        continue
-                    tname = theme
-                    tinfo = None
-                cls.themes[tname] = (path.join(themedir, theme), tinfo)
 
     @classmethod
     def load_extra_theme(cls, name):
-        # type: (unicode) -> None
+        if name == 'alabaster':
-        themes = ['alabaster']
+            cls.load_alabaster()
+        elif name == 'sphinx_rtd_theme':
+            cls.load_sphinx_rtd_theme()
+        else:
+            pass
+
+    @classmethod
+    def load_alabaster(cls):
+        import alabaster
+        cls.themes['alabaster'] = path.join(alabaster.get_path(), 'alabaster')
+
+    @classmethod
+    def load_sphinx_rtd_theme(cls):
         try:
             import sphinx_rtd_theme
-            themes.append('sphinx_rtd_theme')
+            cls.themes['sphinx_rtd_theme'] = path.join(sphinx_rtd_theme.get_html_theme_path(),
+                                                       'sphinx_rtd_theme')
         except ImportError:
             pass
-        if name in themes:
-            if name == 'alabaster':
-                import alabaster
-                themedir = alabaster.get_path()
-                # alabaster theme also requires 'alabaster' extension, it will be loaded
-                # at sphinx.application module.
-            elif name == 'sphinx_rtd_theme':
-                themedir = sphinx_rtd_theme.get_html_theme_path()
-            else:
-                raise NotImplementedError('Programming Error')
 
-        else:
+    @classmethod
-            for themedir in load_theme_plugins():
+    def load_external_themes(cls, name):
-                if path.isfile(path.join(themedir, name, THEMECONF)):
+        for entry_point in pkg_resources.iter_entry_points('sphinx_themes'):
-                    break
+            target = entry_point.load()
+            if callable(target):
+                themedir = target()
+                if not isinstance(path, string_types):
+                    logger.warning(_('Theme extension %r does not response correctly.') %
+                                   entry_point.module_name)
             else:
-                # specified theme is not found
+                themedir = target
-                return
 
-        cls.themepath.append(themedir)
+            for entry, theme in find_theme_entries(themedir):
-        cls.themes[name] = (path.join(themedir, name), None)
+                if name == entry:
-        return
+                    cls.themes[entry] = theme
 
     @classmethod
     def create(cls, name):
         # type: (unicode) -> None
         if name not in cls.themes:
             cls.load_extra_theme(name)
-            if name not in cls.themes:
+
-                if name == 'sphinx_rtd_theme':
+        if name not in cls.themes:
-                    raise ThemeError('sphinx_rtd_theme is no longer a hard dependency '
+            if name == 'sphinx_rtd_theme':
-                                     'since version 1.4.0. Please install it manually.'
+                raise ThemeError(_('sphinx_rtd_theme is no longer a hard dependency '
-                                     '(pip install sphinx_rtd_theme)')
+                                   'since version 1.4.0. Please install it manually.'
-                else:
+                                   '(pip install sphinx_rtd_theme)'))
-                    raise ThemeError('no theme named %r found '
+            else:
-                                     '(missing theme.conf?)' % name)
+                raise ThemeError(_('no theme named %r found '
+                                   '(missing theme.conf?)') % name)
 
         theme = _Theme()
         theme.name = name
 
-        # Do not warn yet -- to be compatible with old Sphinxes, people *have*
+        themedir = cls.themes[name]
-        # to use "default".
+        if path.isdir(themedir):
-        # if name == 'default' and warn:
-        #     warn("'default' html theme has been renamed to 'classic'. "
-        #          "Please change your html_theme setting either to "
-        #          "the new 'alabaster' default theme, or to 'classic' "
-        #          "to keep using the old default.")
-
-        tdir, tinfo = cls.themes[name]
-        if tinfo is None:
             # already a directory, do nothing
             theme.rootdir = None
-            theme.themedir = tdir
+            theme.themedir = themedir
             theme.themedir_created = False
         else:
             # extract the theme to a temp directory
@@ -224,14 +234,14 @@ class Theme(object):
             theme.themedir_created = True
             ensuredir(theme.themedir)
 
-            for name in tinfo.namelist():
+            with ZipFile(themedir) as archive:  # type: ignore
-                if name.endswith('/'):
+                for name in archive.namelist():
-                    continue
+                    if name.endswith('/'):
-                dirname = path.dirname(name)
+                        continue
-                if not path.isdir(path.join(theme.themedir, dirname)):
+                    filename = path.join(theme.themedir, name)
-                    os.makedirs(path.join(theme.themedir, dirname))
+                    ensuredir(path.dirname(filename))
-                with open(path.join(theme.themedir, name), 'wb') as fp:
+                    with open(path.join(filename), 'wb') as fp:
-                    fp.write(tinfo.read(name))
+                        fp.write(archive.read(name))
 
         theme.themeconf = configparser.RawConfigParser()
         theme.themeconf.read(path.join(theme.themedir, THEMECONF))  # type: ignore
@@ -241,9 +251,6 @@ class Theme(object):
         except configparser.NoOptionError:
             raise ThemeError('theme %r doesn\'t have "inherit" setting' % name)
 
-        # load inherited theme automatically #1794, #1884, #1885
-        cls.load_extra_theme(inherit)
-
         if inherit == 'none':
             theme.base = None
         elif inherit not in cls.themes:
@@ -253,26 +260,3 @@ class Theme(object):
             theme.base = cls.create(inherit)
 
         return theme
-
-
-def load_theme_plugins():
-    # type: () -> List[unicode]
-    """load plugins by using``sphinx_themes`` section in setuptools entry_points.
-    This API will return list of directory that contain some theme directory.
-    """
-    theme_paths = []  # type: List[unicode]
-
-    for plugin in pkg_resources.iter_entry_points('sphinx_themes'):
-        func_or_path = plugin.load()
-        try:
-            path = func_or_path()
-        except Exception:
-            path = func_or_path
-
-        if isinstance(path, string_types):
-            theme_paths.append(path)
-        else:
-            raise ThemeError('Plugin %r does not response correctly.' %
-                             plugin.module_name)
-
-    return theme_paths

tests/roots/test-double-inheriting-theme/conf.py

@@ -5,4 +5,5 @@ import sys, os
 templates_path = ['_templates']
 master_doc = 'index'
 html_theme = 'base_theme2'
+html_theme_path = ['base_themes_dir']
 exclude_patterns = ['_build']

tests/test_theming.py

@@ -31,8 +31,8 @@ def test_theme_api(app, status, warning):
         set(['basic', 'default', 'scrolls', 'agogo', 'sphinxdoc', 'haiku',
              'traditional', 'testtheme', 'ziptheme', 'epub', 'nature',
              'pyramid', 'bizstyle', 'classic', 'nonav'])
-    assert Theme.themes['testtheme'][1] is None
+    assert Theme.themes['testtheme'] == app.srcdir / 'testtheme'
-    assert isinstance(Theme.themes['ziptheme'][1], zipfile.ZipFile)
+    assert Theme.themes['ziptheme'] == app.srcdir / 'ziptheme.zip'
 
     # test Theme instance API
     theme = app.builder.theme
@@ -88,21 +88,13 @@ def test_js_source(app, status, warning):
 
 
 @pytest.mark.sphinx(testroot='double-inheriting-theme')
-def test_double_inheriting_theme(make_app, app_params):
+def test_double_inheriting_theme(app, status, warning):
-    from sphinx.theming import load_theme_plugins  # load original before patching
+    assert app.builder.theme.name == 'base_theme2'
-
+    app.build()  # => not raises TemplateNotFound
-    def load_themes():
-        roots = path(__file__).abspath().parent / 'roots'
-        yield roots / 'test-double-inheriting-theme' / 'base_themes_dir'
-        for t in load_theme_plugins():
-            yield t
-
-    with mock.patch('sphinx.theming.load_theme_plugins', side_effect=load_themes):
-        args, kwargs = app_params
-        make_app(*args, **kwargs)
 
 
 @pytest.mark.sphinx(testroot='theming',
                     confoverrides={'html_theme': 'child'})
 def test_nested_zipped_theme(app, status, warning):
+    assert app.builder.theme.name == 'child'
     app.build()  # => not raises TemplateNotFound