Skip to content

Conversation

@tk0miya
Copy link
Contributor

@tk0miya tk0miya commented Apr 11, 2020

On handling autodoc-process-signature event, it's not needed to
escape "*" characters. The escaping is mainly allowed for
highlighting feature of editors. So it's not necessary for
generating contents by autodoc.

In addition, since Sphinx-3.0, the escaping is not recommended by
default (see strip_signature_backslash)

refs: sphinx-doc/sphinx#7439

On handling autodoc-process-signature event, it's not needed to
escape "*" characters.  The escaping is mainly allowed for
highlighting feature of editors.  So it's not necessary for
generating contents by autodoc.

In addition, since Sphinx-3.0, the escaping is not recommended by
default (see strip_signature_backslash)

refs: sphinx-doc/sphinx#7439
@larsoner
Copy link
Collaborator

Changes LGTM but it would be nice to see the effect before and after. I'd like to see if we can squeak in #258 first, then merge this, if that seems reasonable to you @tk0miya

@tk0miya
Copy link
Contributor Author

tk0miya commented Apr 12, 2020

Changes LGTM but it would be nice to see the effect before and after.

Before Sphinx-3.0, backslashes are removed before processing. So this changes nothing.
https://github.com/sphinx-doc/sphinx/blob/2.x/sphinx/directives/__init__.py#L95-L97

Since Sphinx-3.0, backslashes are removed before processing only if strip_signature_backslash is enabled (optional). So backslashes will be shown without this change (or enable strip_signature_backslash on each project). This is one of incompatible changes in our latest release. Anyway, not to escape also works fine for both versions.
https://github.com/sphinx-doc/sphinx/blob/3.x/sphinx/directives/__init__.py#L95-L100

@larsoner larsoner closed this Apr 13, 2020
@larsoner larsoner reopened this Apr 13, 2020
@larsoner larsoner merged commit f884309 into numpy:master Apr 13, 2020
@larsoner
Copy link
Collaborator

Thanks @tk0miya

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants