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

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 jakobandersen added type:enhancement enhance or introduce a new feature domains:py labels Sep 25, 2021
@jakobandersen
Copy link
Contributor Author

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

CHANGES Outdated Show resolved Hide resolved
Co-authored-by: Takeshi KOMIYA <i.tkomiya@gmail.com>
@jakobandersen jakobandersen merged commit ebea432 into sphinx-doc:4.x Oct 3, 2021
@jakobandersen jakobandersen deleted the py_nodes branch October 3, 2021 08:16
@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.
Labels
domains:py type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants