From 15e9d8fd8ddd12496215fcf89a5b441e5f31409d Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 28 Feb 2022 12:14:57 -0500 Subject: [PATCH] MkDocs: allow None on extra_css/extra_javascript Currently we are failing the build if we find None values, but MkDocs allows them. --- readthedocs/doc_builder/backends/mkdocs.py | 48 +++++++++---------- .../rtd_tests/tests/test_doc_builder.py | 46 +++++++++++++++++- 2 files changed, 69 insertions(+), 25 deletions(-) diff --git a/readthedocs/doc_builder/backends/mkdocs.py b/readthedocs/doc_builder/backends/mkdocs.py index 3298d3f80dc..4a5f1e3c7f3 100644 --- a/readthedocs/doc_builder/backends/mkdocs.py +++ b/readthedocs/doc_builder/backends/mkdocs.py @@ -4,9 +4,9 @@ .. _MkDocs: http://www.mkdocs.org/ """ -import structlog import os +import structlog import yaml from django.conf import settings from django.template import loader as template_loader @@ -149,35 +149,35 @@ def append_conf(self): # Set mkdocs config values static_url = get_absolute_static_url() + extra_assets = { + 'extra_javascript': [ + 'readthedocs-data.js', + f'{static_url}core/js/readthedocs-doc-embed.js', + f'{static_url}javascript/readthedocs-analytics.js', + ], + 'extra_css': [ + f'{static_url}css/badge_only.css', + f'{static_url}css/readthedocs-doc-embed.css', + ], + } - for config in ('extra_css', 'extra_javascript'): - user_value = user_config.get(config, []) - if not isinstance(user_value, list): + for config, extras in extra_assets.items(): + value = user_config.get(config, []) + if value is None: + value = [] + if not isinstance(value, list): raise MkDocsYAMLParseError( MkDocsYAMLParseError.INVALID_EXTRA_CONFIG.format( config=config, ), ) - - extra_javascript_list = [ - 'readthedocs-data.js', - '%score/js/readthedocs-doc-embed.js' % static_url, - '%sjavascript/readthedocs-analytics.js' % static_url, - ] - extra_css_list = [ - '%scss/badge_only.css' % static_url, - '%scss/readthedocs-doc-embed.css' % static_url, - ] - - # Only add static file if the files are not already in the list - user_config.setdefault('extra_javascript', []).extend( - [js for js in extra_javascript_list if js not in user_config.get( - 'extra_javascript')] - ) - user_config.setdefault('extra_css', []).extend( - [css for css in extra_css_list if css not in user_config.get( - 'extra_css')] - ) + # Add the static file only if isn't already in the list. + value.extend([ + extra + for extra in extras + if extra not in value + ]) + user_config[config] = value # The docs path is relative to the location # of the mkdocs configuration file. diff --git a/readthedocs/rtd_tests/tests/test_doc_builder.py b/readthedocs/rtd_tests/tests/test_doc_builder.py index 5d6a160e5e1..8f298c2ffef 100644 --- a/readthedocs/rtd_tests/tests/test_doc_builder.py +++ b/readthedocs/rtd_tests/tests/test_doc_builder.py @@ -566,7 +566,7 @@ def test_append_conf_existing_yaml_on_root_with_invalid_setting(self, checkout_p yaml_contents = [ {'docs_dir': ['docs']}, {'extra_css': 'a string here'}, - {'extra_javascript': None}, + {'extra_javascript': ''}, ] for content in yaml_contents: yaml.safe_dump( @@ -576,6 +576,50 @@ def test_append_conf_existing_yaml_on_root_with_invalid_setting(self, checkout_p with self.assertRaises(MkDocsYAMLParseError): self.searchbuilder.append_conf() + @patch('readthedocs.doc_builder.base.BaseBuilder.run') + @patch('readthedocs.projects.models.Project.checkout_path') + def test_append_conf_and_none_values(self, checkout_path, run): + tmpdir = tempfile.mkdtemp() + os.mkdir(os.path.join(tmpdir, 'docs')) + yaml_file = os.path.join(tmpdir, 'mkdocs.yml') + checkout_path.return_value = tmpdir + + python_env = Virtualenv( + version=self.version, + build_env=self.build_env, + config=None, + ) + builder = MkdocsHTML( + build_env=self.build_env, + python_env=python_env, + ) + + yaml.safe_dump( + { + 'extra_css': None, + 'extra_javascript': None, + }, + open(yaml_file, 'w'), + ) + builder.append_conf() + config = yaml_load_safely(open(yaml_file)) + + self.assertEqual( + config['extra_css'], + [ + 'http://readthedocs.org/static/css/badge_only.css', + 'http://readthedocs.org/static/css/readthedocs-doc-embed.css', + ], + ) + self.assertEqual( + config['extra_javascript'], + [ + 'readthedocs-data.js', + 'http://readthedocs.org/static/core/js/readthedocs-doc-embed.js', + 'http://readthedocs.org/static/javascript/readthedocs-analytics.js', + ], + ) + @patch('readthedocs.doc_builder.base.BaseBuilder.run') @patch('readthedocs.projects.models.Project.checkout_path') def test_dont_override_theme(self, checkout_path, run):