Merge pull request #547 from bbinet/refactor-settings

Refactor how settings are handled

pelican/__init__.py

@@ -1,4 +1,3 @@
-import copy
 import os
 import re
 import sys
@@ -12,7 +11,7 @@ from pelican.generators import (ArticlesGenerator, PagesGenerator,
                                 StaticGenerator, PdfGenerator,
                                 LessCSSGenerator, SourceFileGenerator)
 from pelican.log import init
-from pelican.settings import read_settings, _DEFAULT_CONFIG
+from pelican.settings import read_settings
 from pelican.utils import (clean_output_dir, files_changed, file_changed,
                            NoFilesError)
 from pelican.writers import Writer
@@ -26,42 +25,21 @@ logger = logging.getLogger(__name__)
 
 
 class Pelican(object):
-    def __init__(self, settings=None, path=None, theme=None, output_path=None,
+    def __init__(self, settings):
-            markup=None, delete_outputdir=False, plugin_path=None):
+        """
-        """Read the settings, and performs some checks on the environment
+        Pelican initialisation, performs some checks on the environment before
-        before doing anything else.
+        doing anything else.
         """
-        if settings is None:
-            settings = copy.deepcopy(_DEFAULT_CONFIG)
-
-        self.path = path or settings['PATH']
-        if not self.path:
-            raise Exception('You need to specify a path containing the content'
-                    ' (see pelican --help for more information)')
-
-        if self.path.endswith('/'):
-            self.path = self.path[:-1]
 
         # define the default settings
         self.settings = settings
-
         self._handle_deprecation()
 
-        self.theme = theme or settings['THEME']
+        self.path = settings['PATH']
-        output_path = output_path or settings['OUTPUT_PATH']
+        self.theme = settings['THEME']
-        self.output_path = os.path.realpath(output_path)
+        self.output_path = settings['OUTPUT_PATH']
-        self.markup = markup or settings['MARKUP']
+        self.markup = settings['MARKUP']
-        self.delete_outputdir = delete_outputdir \
+        self.delete_outputdir = settings['DELETE_OUTPUT_DIRECTORY']
-                                    or settings['DELETE_OUTPUT_DIRECTORY']
-
-        # find the theme in pelican.theme if the given one does not exists
-        if not os.path.exists(self.theme):
-            theme_path = os.sep.join([os.path.dirname(
-                os.path.abspath(__file__)), "themes/%s" % self.theme])
-            if os.path.exists(theme_path):
-                self.theme = theme_path
-            else:
-                raise Exception("Impossible to find the theme %s" % theme)
 
         self.init_path()
         self.init_plugins()
@@ -259,11 +237,26 @@ def parse_arguments():
     return parser.parse_args()
 
 
-def get_instance(args):
+def get_config(args):
-    markup = [a.strip().lower() for a in args.markup.split(',')]\
+    config = {}
-              if args.markup else None
+    if args.path:
+        config['PATH'] = os.path.abspath(os.path.expanduser(args.path))
+    if args.output:
+        config['OUTPUT_PATH'] = \
+                os.path.abspath(os.path.expanduser(args.output))
+    if args.markup:
+        config['MARKUP'] = [a.strip().lower() for a in args.markup.split(',')]
+    if args.theme:
+        abstheme = os.path.abspath(os.path.expanduser(args.theme))
+        config['THEME'] = abstheme if os.path.exists(abstheme) else args.theme
+    if args.delete_outputdir is not None:
+        config['DELETE_OUTPUT_DIRECTORY'] = args.delete_outputdir
+    return config
 
-    settings = read_settings(args.settings)
+
+def get_instance(args):
+
+    settings = read_settings(args.settings, override=get_config(args))
 
     cls = settings.get('PELICAN_CLASS')
     if isinstance(cls, basestring):
@@ -271,15 +264,12 @@ def get_instance(args):
         module = __import__(module)
         cls = getattr(module, cls_name)
 
-    return cls(settings, args.path, args.theme, args.output, markup,
+    return cls(settings)
-               args.delete_outputdir)
 
 
 def main():
     args = parse_arguments()
     init(args.verbosity)
-    # Split the markup languages only if some have been given. Otherwise,
-    # populate the variable with None.
     pelican = get_instance(args)
 
     try:

pelican/settings.py

@@ -82,13 +82,24 @@ _DEFAULT_CONFIG = {'PATH': '.',
                    }
 
 
-def read_settings(filename=None):
+def read_settings(filename=None, override=None):
     if filename:
         local_settings = get_settings_from_file(filename)
+        # Make the paths relative to the settings file
+        for p in ['PATH', 'OUTPUT_PATH', 'THEME']:
+            if p in local_settings and local_settings[p] is not None \
+                    and not isabs(local_settings[p]):
+                absp = os.path.abspath(os.path.normpath(os.path.join(
+                            os.path.dirname(filename), local_settings[p])))
+                if p != 'THEME' or os.path.exists(p):
+                    local_settings[p] = absp
     else:
         local_settings = copy.deepcopy(_DEFAULT_CONFIG)
-    configured_settings = configure_settings(local_settings, None, filename)
+
-    return configured_settings
+    if override:
+        local_settings.update(override)
+
+    return configure_settings(local_settings)
 
 
 def get_settings_from_module(module=None, default_settings=_DEFAULT_CONFIG):
@@ -98,9 +109,8 @@ def get_settings_from_module(module=None, default_settings=_DEFAULT_CONFIG):
 
     context = copy.deepcopy(default_settings)
     if module is not None:
-         context.update(
+        context.update(
-             (k, v) for k, v in inspect.getmembers(module) if k.isupper()
+                (k, v) for k, v in inspect.getmembers(module) if k.isupper())
-         )
     return context
 
 
@@ -115,19 +125,23 @@ def get_settings_from_file(filename, default_settings=_DEFAULT_CONFIG):
     return get_settings_from_module(module, default_settings=default_settings)
 
 
-def configure_settings(settings, default_settings=None, filename=None):
+def configure_settings(settings):
-    """Provide optimizations, error checking, and warnings for loaded settings"""
+    """
-    if default_settings is None:
+    Provide optimizations, error checking, and warnings for loaded settings
-        default_settings = copy.deepcopy(_DEFAULT_CONFIG)
+    """
+    if not 'PATH' in settings or not os.path.isdir(settings['PATH']):
+        raise Exception('You need to specify a path containing the content'
+                ' (see pelican --help for more information)')
 
-    # Make the paths relative to the settings file
+    # find the theme in pelican.theme if the given one does not exists
-    if filename:
+    if not os.path.isdir(settings['THEME']):
-        for path in ['PATH', 'OUTPUT_PATH']:
+        theme_path = os.sep.join([os.path.dirname(
-            if path in settings:
+            os.path.abspath(__file__)), "themes/%s" % settings['THEME']])
-                if settings[path] is not None and not isabs(settings[path]):
+        if os.path.exists(theme_path):
-                    settings[path] = os.path.abspath(os.path.normpath(
+            settings['THEME'] = theme_path
-                        os.path.join(os.path.dirname(filename), settings[path]))
+        else:
-                    )
+            raise Exception("Impossible to find the theme %s"
+                    % settings['THEME'])
 
     # if locales is not a list, make it one
     locales = settings['LOCALE']

tests/test_pelican.py

@@ -55,7 +55,11 @@ class TestPelican(unittest.TestCase):
         with patch("pelican.contents.getenv") as mock_getenv:
             # force getenv('USER') to always return the same value
             mock_getenv.return_value = "Dummy Author"
-            pelican = Pelican(path=INPUT_PATH, output_path=self.temp_path)
+            settings = read_settings(filename=None, override={
+                'PATH': INPUT_PATH,
+                'OUTPUT_PATH': self.temp_path,
+                })
+            pelican = Pelican(settings=settings)
             pelican.run()
             diff = dircmp(
                     self.temp_path, os.sep.join((OUTPUT_PATH, "basic")))
@@ -63,8 +67,11 @@ class TestPelican(unittest.TestCase):
 
     def test_custom_generation_works(self):
         # the same thing with a specified set of settings should work
-        pelican = Pelican(path=INPUT_PATH, output_path=self.temp_path,
+        settings = read_settings(filename=SAMPLE_CONFIG, override={
-                            settings=read_settings(SAMPLE_CONFIG))
+            'PATH': INPUT_PATH,
+            'OUTPUT_PATH': self.temp_path,
+            })
+        pelican = Pelican(settings=settings)
         pelican.run()
         diff = dircmp(self.temp_path, os.sep.join((OUTPUT_PATH, "custom")))
         self.assertFilesEqual(diff)

tests/test_settings.py

@@ -56,16 +56,19 @@ class TestSettingsConfiguration(unittest.TestCase):
     def test_configure_settings(self):
         """Manipulations to settings should be applied correctly."""
 
-        # SITEURL should not have a trailing slash
+        settings = {
-        settings = {'SITEURL': 'http://blog.notmyidea.org/', 'LOCALE': ''}
+                'SITEURL': 'http://blog.notmyidea.org/',
+                'LOCALE': '',
+                'PATH': '.',
+                'THEME': DEFAULT_THEME,
+                }
         configure_settings(settings)
+        # SITEURL should not have a trailing slash
         self.assertEqual(settings['SITEURL'], 'http://blog.notmyidea.org')
 
         # FEED_DOMAIN, if undefined, should default to SITEURL
-        settings = {'SITEURL': 'http://blog.notmyidea.org', 'LOCALE': ''}
-        configure_settings(settings)
         self.assertEqual(settings['FEED_DOMAIN'], 'http://blog.notmyidea.org')
 
-        settings = {'FEED_DOMAIN': 'http://feeds.example.com', 'LOCALE': ''}
+        settings['FEED_DOMAIN'] = 'http://feeds.example.com'
         configure_settings(settings)
         self.assertEqual(settings['FEED_DOMAIN'], 'http://feeds.example.com')