From 1a43d47c338a3b2ca843c6c3d7d4b97c91c06bb6 Mon Sep 17 00:00:00 2001 From: Takeshi KOMIYA Date: Tue, 20 Mar 2018 20:43:44 +0900 Subject: [PATCH 1/2] Add a new keyword argument ``override`` to Application APIs --- CHANGES | 1 + sphinx/application.py | 131 ++++++++++++++++++++++++-------------- sphinx/registry.py | 90 +++++++++++++++----------- tests/test_application.py | 22 ------- 4 files changed, 137 insertions(+), 107 deletions(-) diff --git a/CHANGES b/CHANGES index 78d42cce1..b5d4e26d6 100644 --- a/CHANGES +++ b/CHANGES @@ -49,6 +49,7 @@ Features added * Add ``app.add_message_catalog()`` and ``sphinx.locale.get_translations()`` to support translation for 3rd party extensions * helper function ``warning()`` for HTML themes is added +* Add a new keyword argument ``override`` to Application APIs Bugs fixed ---------- diff --git a/sphinx/application.py b/sphinx/application.py index 8014460a3..5b6842d78 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -517,14 +517,17 @@ class Sphinx(object): # registering addon parts - def add_builder(self, builder): - # type: (Type[Builder]) -> None + def add_builder(self, builder, override=False): + # type: (Type[Builder], bool) -> None """Register a new builder. *builder* must be a class that inherits from :class:`~sphinx.builders.Builder`. + + .. versionchanged:: 1.8 + Add *override* keyword. """ - self.registry.add_builder(builder) + self.registry.add_builder(builder, override=override) # TODO(stephenfin): Describe 'types' parameter def add_config_value(self, name, default, rebuild, types=()): @@ -571,8 +574,8 @@ class Sphinx(object): logger.debug('[app] adding event: %r', name) self.events.add(name) - def set_translator(self, name, translator_class): - # type: (unicode, Type[nodes.NodeVisitor]) -> None + def set_translator(self, name, translator_class, override=False): + # type: (unicode, Type[nodes.NodeVisitor], bool) -> None """Register or override a Docutils translator class. This is used to register a custom output translator or to replace a @@ -580,11 +583,13 @@ class Sphinx(object): and define custom nodes for the translator (see :meth:`add_node`). .. versionadded:: 1.3 + .. versionchanged:: 1.8 + Add *override* keyword. """ - self.registry.add_translator(name, translator_class) + self.registry.add_translator(name, translator_class, override=override) - def add_node(self, node, **kwds): - # type: (nodes.Node, Any) -> None + def add_node(self, node, override=False, **kwds): + # type: (nodes.Node, bool, Any) -> None """Register a Docutils node class. This is necessary for Docutils internals. It may also be used in the @@ -615,7 +620,7 @@ class Sphinx(object): Added the support for keyword arguments giving visit functions. """ logger.debug('[app] adding node: %r', (node, kwds)) - if not kwds.pop('override', False) and docutils.is_node_registered(node): + if not override and docutils.is_node_registered(node): logger.warning(__('while setting up extension %s: node class %r is ' 'already registered, its visitors will be overridden'), self._setting_up_extension, node.__name__, @@ -623,8 +628,8 @@ class Sphinx(object): docutils.register_node(node) self.registry.add_translation_handlers(node, **kwds) - def add_enumerable_node(self, node, figtype, title_getter=None, **kwds): - # type: (nodes.Node, unicode, TitleGetter, Any) -> None + def add_enumerable_node(self, node, figtype, title_getter=None, override=False, **kwds): + # type: (nodes.Node, unicode, TitleGetter, bool, Any) -> None """Register a Docutils node class as a numfig target. Sphinx numbers the node automatically. And then the users can refer it @@ -648,8 +653,8 @@ class Sphinx(object): .. versionadded:: 1.4 """ - self.registry.add_enumerable_node(node, figtype, title_getter) - self.add_node(node, **kwds) + self.registry.add_enumerable_node(node, figtype, title_getter, override=override) + self.add_node(node, override=override, **kwds) @property def enumerable_nodes(self): @@ -659,8 +664,8 @@ class Sphinx(object): RemovedInSphinx30Warning) return self.registry.enumerable_nodes - def add_directive(self, name, obj, content=None, arguments=None, **options): - # type: (unicode, Any, bool, Tuple[int, int, bool], Any) -> None + def add_directive(self, name, obj, content=None, arguments=None, override=False, **options): # NOQA + # type: (unicode, Any, bool, Tuple[int, int, bool], bool, Any) -> None """Register a Docutils directive. *name* must be the prospective directive name. There are two possible @@ -698,10 +703,12 @@ class Sphinx(object): Docutils 0.5-style directive classes are now supported. .. deprecated:: 1.8 Docutils 0.4-style (function based) directives support is deprecated. + .. versionchanged:: 1.8 + Add *override* keyword. """ logger.debug('[app] adding directive: %r', (name, obj, content, arguments, options)) - if name in directives._directives: + if name in directives._directives and not override: logger.warning(__('while setting up extension %s: directive %r is ' 'already registered, it will be overridden'), self._setting_up_extension[-1], name, @@ -713,36 +720,41 @@ class Sphinx(object): else: directives.register_directive(name, obj) - def add_role(self, name, role): - # type: (unicode, Any) -> None + def add_role(self, name, role, override=False): + # type: (unicode, Any, bool) -> None """Register a Docutils role. *name* must be the role name that occurs in the source, *role* the role function. Refer to the `Docutils documentation `_ for more information. + + .. versionchanged:: 1.8 + Add *override* keyword. """ logger.debug('[app] adding role: %r', (name, role)) - if name in roles._roles: + if name in roles._roles and not override: logger.warning(__('while setting up extension %s: role %r is ' 'already registered, it will be overridden'), self._setting_up_extension[-1], name, type='app', subtype='add_role') roles.register_local_role(name, role) - def add_generic_role(self, name, nodeclass): - # type: (unicode, Any) -> None + def add_generic_role(self, name, nodeclass, override=False): + # type: (unicode, Any, bool) -> None """Register a generic Docutils role. Register a Docutils role that does nothing but wrap its contents in the node given by *nodeclass*. .. versionadded:: 0.6 + .. versionchanged:: 1.8 + Add *override* keyword. """ # Don't use ``roles.register_generic_role`` because it uses # ``register_canonical_role``. logger.debug('[app] adding generic role: %r', (name, nodeclass)) - if name in roles._roles: + if name in roles._roles and not override: logger.warning(__('while setting up extension %s: role %r is ' 'already registered, it will be overridden'), self._setting_up_extension[-1], name, @@ -750,16 +762,18 @@ class Sphinx(object): role = roles.GenericRole(name, nodeclass) roles.register_local_role(name, role) - def add_domain(self, domain): - # type: (Type[Domain]) -> None + def add_domain(self, domain, override=False): + # type: (Type[Domain], bool) -> None """Register a domain. Make the given *domain* (which must be a class; more precisely, a subclass of :class:`~sphinx.domains.Domain`) known to Sphinx. .. versionadded:: 1.0 + .. versionchanged:: 1.8 + Add *override* keyword. """ - self.registry.add_domain(domain) + self.registry.add_domain(domain, override=override) def override_domain(self, domain): # type: (Type[Domain]) -> None @@ -770,48 +784,57 @@ class Sphinx(object): of the existing one. .. versionadded:: 1.0 + .. deprecated:: 1.8 + Integrated to :meth:`add_domain`. """ - self.registry.override_domain(domain) + self.registry.add_domain(domain, override=True) - def add_directive_to_domain(self, domain, name, obj, - has_content=None, argument_spec=None, **option_spec): - # type: (unicode, unicode, Any, bool, Any, Any) -> None + def add_directive_to_domain(self, domain, name, obj, has_content=None, argument_spec=None, + override=False, **option_spec): + # type: (unicode, unicode, Any, bool, Any, bool, Any) -> None """Register a Docutils directive in a domain. Like :meth:`add_directive`, but the directive is added to the domain named *domain*. .. versionadded:: 1.0 + .. versionchanged:: 1.8 + Add *override* keyword. """ self.registry.add_directive_to_domain(domain, name, obj, - has_content, argument_spec, **option_spec) + has_content, argument_spec, override=override, + **option_spec) - def add_role_to_domain(self, domain, name, role): - # type: (unicode, unicode, Union[RoleFunction, XRefRole]) -> None + def add_role_to_domain(self, domain, name, role, override=False): + # type: (unicode, unicode, Union[RoleFunction, XRefRole], bool) -> None """Register a Docutils role in a domain. Like :meth:`add_role`, but the role is added to the domain named *domain*. .. versionadded:: 1.0 + .. versionchanged:: 1.8 + Add *override* keyword. """ - self.registry.add_role_to_domain(domain, name, role) + self.registry.add_role_to_domain(domain, name, role, override=override) - def add_index_to_domain(self, domain, index): - # type: (unicode, Type[Index]) -> None + def add_index_to_domain(self, domain, index, override=False): + # type: (unicode, Type[Index], bool) -> None """Register a custom index for a domain. Add a custom *index* class to the domain named *domain*. *index* must be a subclass of :class:`~sphinx.domains.Index`. .. versionadded:: 1.0 + .. versionchanged:: 1.8 + Add *override* keyword. """ self.registry.add_index_to_domain(domain, index) def add_object_type(self, directivename, rolename, indextemplate='', parse_node=None, ref_nodeclass=None, objname='', - doc_field_types=[]): - # type: (unicode, unicode, unicode, Callable, nodes.Node, unicode, List) -> None + doc_field_types=[], override=False): + # type: (unicode, unicode, unicode, Callable, nodes.Node, unicode, List, bool) -> None """Register a new object type. This method is a very convenient way to add a new :term:`object` type @@ -866,9 +889,13 @@ class Sphinx(object): This method is also available under the deprecated alias :meth:`add_description_unit`. + + .. versionchanged:: 1.8 + Add *override* keyword. """ self.registry.add_object_type(directivename, rolename, indextemplate, parse_node, - ref_nodeclass, objname, doc_field_types) + ref_nodeclass, objname, doc_field_types, + override=override) def add_description_unit(self, directivename, rolename, indextemplate='', parse_node=None, ref_nodeclass=None, objname='', @@ -886,8 +913,8 @@ class Sphinx(object): ref_nodeclass, objname, doc_field_types) def add_crossref_type(self, directivename, rolename, indextemplate='', - ref_nodeclass=None, objname=''): - # type: (unicode, unicode, unicode, nodes.Node, unicode) -> None + ref_nodeclass=None, objname='', override=False): + # type: (unicode, unicode, unicode, nodes.Node, unicode, bool) -> None """Register a new crossref object type. This method is very similar to :meth:`add_object_type` except that the @@ -913,9 +940,13 @@ class Sphinx(object): (Of course, the element following the ``topic`` directive needn't be a section.) + + .. versionchanged:: 1.8 + Add *override* keyword. """ self.registry.add_crossref_type(directivename, rolename, - indextemplate, ref_nodeclass, objname) + indextemplate, ref_nodeclass, objname, + override=override) def add_transform(self, transform): # type: (Type[Transform]) -> None @@ -1070,25 +1101,29 @@ class Sphinx(object): assert issubclass(cls, SearchLanguage) languages[cls.lang] = cls - def add_source_suffix(self, suffix, filetype): - # type: (unicode, unicode) -> None + def add_source_suffix(self, suffix, filetype, override=False): + # type: (unicode, unicode, bool) -> None """Register a suffix of source files. Same as :confval:`source_suffix`. The users can override this using the setting. - """ - self.registry.add_source_suffix(suffix, filetype) - def add_source_parser(self, *args): - # type: (Any) -> None + .. versionadded:: 1.8 + """ + self.registry.add_source_suffix(suffix, filetype, override=override) + + def add_source_parser(self, *args, **kwargs): + # type: (Any, Any) -> None """Register a parser class. .. versionadded:: 1.4 .. versionchanged:: 1.8 *suffix* argument is deprecated. It only accepts *parser* argument. Use :meth:`add_source_suffix` API to register suffix instead. + .. versionchanged:: 1.8 + Add *override* keyword. """ - self.registry.add_source_parser(*args) + self.registry.add_source_parser(*args, **kwargs) def add_env_collector(self, collector): # type: (Type[EnvironmentCollector]) -> None diff --git a/sphinx/registry.py b/sphinx/registry.py index 0d23e8368..ab4cfce70 100644 --- a/sphinx/registry.py +++ b/sphinx/registry.py @@ -113,12 +113,12 @@ class SphinxComponentRegistry(object): #: additional transforms; list of transforms self.transforms = [] # type: List[Type[Transform]] - def add_builder(self, builder): - # type: (Type[Builder]) -> None + def add_builder(self, builder, override=False): + # type: (Type[Builder], bool) -> None logger.debug('[app] adding builder: %r', builder) if not hasattr(builder, 'name'): raise ExtensionError(__('Builder class %s has no "name" attribute') % builder) - if builder.name in self.builders: + if builder.name in self.builders and not override: raise ExtensionError(__('Builder %r already exists (in module %s)') % (builder.name, self.builders[builder.name].__module__)) self.builders[builder.name] = builder @@ -145,10 +145,10 @@ class SphinxComponentRegistry(object): return self.builders[name](app) - def add_domain(self, domain): - # type: (Type[Domain]) -> None + def add_domain(self, domain, override=False): + # type: (Type[Domain], bool) -> None logger.debug('[app] adding domain: %r', domain) - if domain.name in self.domains: + if domain.name in self.domains and not override: raise ExtensionError(__('domain %s already registered') % domain.name) self.domains[domain.name] = domain @@ -172,48 +172,54 @@ class SphinxComponentRegistry(object): def override_domain(self, domain): # type: (Type[Domain]) -> None - logger.debug('[app] overriding domain: %r', domain) - if domain.name not in self.domains: - raise ExtensionError(__('domain %s not yet registered') % domain.name) - if not issubclass(domain, self.domains[domain.name]): - raise ExtensionError(__('new domain not a subclass of registered %s ' - 'domain') % domain.name) - self.domains[domain.name] = domain + warnings.warn('registry.override_domain() is deprecated. ' + 'Use app.add_domain(domain, override=True) instead.', + RemovedInSphinx30Warning) + self.add_domain(domain, override=True) - def add_directive_to_domain(self, domain, name, obj, - has_content=None, argument_spec=None, **option_spec): - # type: (unicode, unicode, Any, bool, Any, Any) -> None + def add_directive_to_domain(self, domain, name, obj, has_content=None, argument_spec=None, + override=False, **option_spec): + # type: (unicode, unicode, Any, bool, Any, bool, Any) -> None logger.debug('[app] adding directive to domain: %r', (domain, name, obj, has_content, argument_spec, option_spec)) if domain not in self.domains: raise ExtensionError(__('domain %s not yet registered') % domain) directives = self.domain_directives.setdefault(domain, {}) + if name in directives and not override: + raise ExtensionError(__('The %r directive is already registered to %d domain') % + (name, domain)) if not isclass(obj) or not issubclass(obj, Directive): directives[name] = directive_helper(obj, has_content, argument_spec, **option_spec) else: directives[name] = obj - def add_role_to_domain(self, domain, name, role): - # type: (unicode, unicode, Union[RoleFunction, XRefRole]) -> None + def add_role_to_domain(self, domain, name, role, override=False): + # type: (unicode, unicode, Union[RoleFunction, XRefRole], bool) -> None logger.debug('[app] adding role to domain: %r', (domain, name, role)) if domain not in self.domains: raise ExtensionError(__('domain %s not yet registered') % domain) roles = self.domain_roles.setdefault(domain, {}) + if name in roles and not override: + raise ExtensionError(__('The %r role is already registered to %d domain') % + (name, domain)) roles[name] = role - def add_index_to_domain(self, domain, index): - # type: (unicode, Type[Index]) -> None + def add_index_to_domain(self, domain, index, override=False): + # type: (unicode, Type[Index], bool) -> None logger.debug('[app] adding index to domain: %r', (domain, index)) if domain not in self.domains: raise ExtensionError(__('domain %s not yet registered') % domain) indices = self.domain_indices.setdefault(domain, []) + if index in indices and not override: + raise ExtensionError(__('The %r index is already registered to %d domain') % + (index.name, domain)) indices.append(index) def add_object_type(self, directivename, rolename, indextemplate='', parse_node=None, ref_nodeclass=None, objname='', - doc_field_types=[]): - # type: (unicode, unicode, unicode, Callable, nodes.Node, unicode, List) -> None + doc_field_types=[], override=False): + # type: (unicode, unicode, unicode, Callable, nodes.Node, unicode, List, bool) -> None logger.debug('[app] adding object type: %r', (directivename, rolename, indextemplate, parse_node, ref_nodeclass, objname, doc_field_types)) @@ -229,11 +235,14 @@ class SphinxComponentRegistry(object): self.add_role_to_domain('std', rolename, XRefRole(innernodeclass=ref_nodeclass)) object_types = self.domain_object_types.setdefault('std', {}) + if directivename in object_types and not override: + raise ExtensionError(__('The %r object_type is already registered') % + directivename) object_types[directivename] = ObjType(objname or directivename, rolename) def add_crossref_type(self, directivename, rolename, indextemplate='', - ref_nodeclass=None, objname=''): - # type: (unicode, unicode, unicode, nodes.Node, unicode) -> None + ref_nodeclass=None, objname='', override=False): + # type: (unicode, unicode, unicode, nodes.Node, unicode, bool) -> None logger.debug('[app] adding crossref type: %r', (directivename, rolename, indextemplate, ref_nodeclass, objname)) @@ -246,18 +255,21 @@ class SphinxComponentRegistry(object): self.add_role_to_domain('std', rolename, XRefRole(innernodeclass=ref_nodeclass)) object_types = self.domain_object_types.setdefault('std', {}) + if directivename in object_types and not override: + raise ExtensionError(__('The %r crossref_type is already registered') % + directivename) object_types[directivename] = ObjType(objname or directivename, rolename) - def add_source_suffix(self, suffix, filetype): - # type: (unicode, unicode) -> None + def add_source_suffix(self, suffix, filetype, override=False): + # type: (unicode, unicode, bool) -> None logger.debug('[app] adding source_suffix: %r, %r', suffix, filetype) - if suffix in self.source_suffix: + if suffix in self.source_suffix and not override: raise ExtensionError(__('source_parser for %r is already registered') % suffix) else: self.source_suffix[suffix] = filetype - def add_source_parser(self, *args): - # type: (Any) -> None + def add_source_parser(self, *args, **kwargs): + # type: (Any, bool) -> None logger.debug('[app] adding search source_parser: %r', args) if len(args) == 1: # new sytle arguments: (source_parser) @@ -281,7 +293,7 @@ class SphinxComponentRegistry(object): # create a map from filetype to parser for filetype in parser.supported: - if filetype in self.source_parsers: + if filetype in self.source_parsers and not kwargs.get('override'): raise ExtensionError(__('source_parser for %r is already registered') % filetype) else: @@ -312,10 +324,10 @@ class SphinxComponentRegistry(object): parser.set_application(app) return parser - def add_source_input(self, input_class): - # type: (Type[Input]) -> None + def add_source_input(self, input_class, override=False): + # type: (Type[Input], bool) -> None for filetype in input_class.supported: - if filetype in self.source_inputs: + if filetype in self.source_inputs and not override: raise ExtensionError(__('source_input for %r is already registered') % filetype) self.source_inputs[filetype] = input_class @@ -331,9 +343,11 @@ class SphinxComponentRegistry(object): except KeyError: raise SphinxError(__('source_input for %s not registered') % filetype) - def add_translator(self, name, translator): - # type: (unicode, Type[nodes.NodeVisitor]) -> None + def add_translator(self, name, translator, override=False): + # type: (unicode, Type[nodes.NodeVisitor], bool) -> None logger.debug('[app] Change of translator for the %s builder.' % name) + if name in self.translators and not override: + raise ExtensionError(__('Translatoro for %r already exists') % name) self.translators[name] = translator def add_translation_handlers(self, node, **kwargs): @@ -403,9 +417,11 @@ class SphinxComponentRegistry(object): logger.debug('[app] adding latex package: %r', name) self.latex_packages.append((name, options)) - def add_enumerable_node(self, node, figtype, title_getter=None): - # type: (nodes.Node, unicode, TitleGetter) -> None + def add_enumerable_node(self, node, figtype, title_getter=None, override=False): + # type: (nodes.Node, unicode, TitleGetter, bool) -> None logger.debug('[app] adding enumerable node: (%r, %r, %r)', node, figtype, title_getter) + if node in self.enumerable_nodes and not override: + raise ExtensionError(__('enumerable_node %r already registered') % node) self.enumerable_nodes[node] = (figtype, title_getter) def load_extension(self, app, extname): diff --git a/tests/test_application.py b/tests/test_application.py index babf95497..d520a4567 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -58,28 +58,6 @@ def test_extension_in_blacklist(app, status, warning): assert msg.startswith("WARNING: the extension 'sphinxjp.themecore' was") -def test_domain_override(app, status, warning): - class A(Domain): - name = 'foo' - - class B(A): - name = 'foo' - - class C(Domain): - name = 'foo' - - # No domain know named foo. - with pytest.raises(ExtensionError) as excinfo: - app.override_domain(A) - assert 'domain foo not yet registered' in str(excinfo.value) - - assert app.add_domain(A) is None - assert app.override_domain(B) is None - with pytest.raises(ExtensionError) as excinfo: - app.override_domain(C) - assert 'new domain not a subclass of registered foo domain' in str(excinfo.value) - - @pytest.mark.sphinx(testroot='add_source_parser') def test_add_source_parser(app, status, warning): assert set(app.config.source_suffix) == set(['.rst', '.md', '.test']) From 61828786a6036d2919bb43473f2484d8f42b5319 Mon Sep 17 00:00:00 2001 From: Takeshi KOMIYA Date: Thu, 29 Mar 2018 21:55:59 +0900 Subject: [PATCH 2/2] Deprecate app.override_domain() --- CHANGES | 1 + doc/extdev/index.rst | 5 +++++ sphinx/application.py | 3 +++ 3 files changed, 9 insertions(+) diff --git a/CHANGES b/CHANGES index 2c553e75e..2fa319435 100644 --- a/CHANGES +++ b/CHANGES @@ -32,6 +32,7 @@ Deprecated deprecated * ``sphinx.locale.l_()`` is deprecated * #2157: helper function ``warn()`` for HTML themes is deprecated +* ``app.override_domain()`` is deprecated For more details, see `deprecation APIs list `_ diff --git a/doc/extdev/index.rst b/doc/extdev/index.rst index a28d01890..e1ae13be1 100644 --- a/doc/extdev/index.rst +++ b/doc/extdev/index.rst @@ -113,6 +113,11 @@ The following is a list of deprecated interface. - (will be) Removed - Alternatives + * - ``sphinx.application.Sphinx.override_domain()`` + - 1.8 + - 3.0 + - :meth:`~sphinx.application.Sphinx.add_domain()` + * - ``warn()`` (template helper function) - 1.8 - 3.0 diff --git a/sphinx/application.py b/sphinx/application.py index 5b6842d78..14aa262ec 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -787,6 +787,9 @@ class Sphinx(object): .. deprecated:: 1.8 Integrated to :meth:`add_domain`. """ + warnings.warn('app.override_domain() is deprecated. ' + 'Use app.add_domain() with override option instead.', + RemovedInSphinx30Warning) self.registry.add_domain(domain, override=True) def add_directive_to_domain(self, domain, name, obj, has_content=None, argument_spec=None,