Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Py, use more desc_sig_* nodes #9672

Merged
merged 22 commits into from Oct 3, 2021
Merged

Conversation

@jakobandersen
Copy link
Contributor

@jakobandersen jakobandersen commented Sep 25, 2021

Feature or Bugfix

  • Feature
  • Bugfix?
  • Refactoring

Purpose

A continuation of #9023, changing many parts of the Python domain to use more of the addnodes.desc_sig_* nodes for markup. This especially includes the use of the space node to make sure spaces are not swallowed if themes use alternative CSS (#9599).

Detail

All of the changes are relatively simple, but it is not complete: the whole pendinx_xref handling is a bit more complicated than I dare to touch right now.

TODO and Context

  • Any semi-trivial places I missed?
  • Probably for another PR: the pending_xref`' handling. It seem like a name A.B.Cbecomes a single xref with the content being anodes.Tex``.
    I think it would be good to change it to something like a
    1. pending_xref with target A and content desc_sig_name with A
    2. . as punctuation
    3. pending_xref with target A.B and content desc_sig_name with B
    4. . as punctuation
    5. pending_xref with target A.B.C and content desc_sig_name with C
  • For another PR: write some documentation for how to style domain elements in HTML
  • For another PR: work around the <span class="pre"> wrapping in
    self.body.append('<span class="pre">%s</span>' % token)
    and
    self.body.append('<span class="pre">%s</span>' % token)

Relates

@jakobandersen
Copy link
Contributor Author

@jakobandersen jakobandersen commented Sep 25, 2021

While the use of nodes is undocumented this may still break some themes, so perhaps there should be an "incompatible changes" entry as well?

Copy link
Member

@tk0miya tk0miya left a comment

LGTM with nits.

Any semi-trivial places I missed?

It seems OK. Let's fix them if we'll find them later.

While the use of nodes is undocumented this may still break some themes, so perhaps there should be an "incompatible changes" entry as well?

I think side effects are nothing or very less. But I'm not sure nobody gives styles to these classes. So it would be better to mention it.

sphinx/domains/python.py Outdated Show resolved Hide resolved
sphinx/domains/python.py Show resolved Hide resolved
tests/test_domain_py.py Outdated Show resolved Hide resolved
@tk0miya tk0miya added this to the 4.3.0 milestone Sep 25, 2021
@jakobandersen jakobandersen force-pushed the py_nodes branch 2 times, most recently from 1e90014 to 2e408cd Sep 26, 2021
Copy link
Member

@tk0miya tk0miya left a comment

LGTM with nits. Please merge this after fixing the typo.

CHANGES Outdated Show resolved Hide resolved
tk0miya
tk0miya approved these changes Oct 2, 2021
Co-authored-by: Takeshi KOMIYA <i.tkomiya@gmail.com>
@jakobandersen jakobandersen merged commit ebea432 into sphinx-doc:4.x Oct 3, 2021
16 checks passed
@jakobandersen jakobandersen deleted the py_nodes branch Oct 3, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants