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

Fix #10594: HTML: field term colons are doubled if using Docutils 0.18+ #10595

Merged
merged 4 commits into from Jun 25, 2022

Conversation

jfbu
Copy link
Contributor

@jfbu jfbu commented Jun 25, 2022

Relates

The Docutils commit responsible for the change at 0.18 is rev8734

Nota bene:

  • our test suite did not detect the added <span class="colon">:</span> at Docutils 0.18. As I am not competent enough to add a test conditional on Docutils version I did not (remember I am only LaTeX guru here).
  • I guess the docinfo class for <dl> is not involved at Sphinx level

@jfbu jfbu added this to the 5.0.3 milestone Jun 25, 2022
sphinx/themes/basic/static/basic.css_t Outdated Show resolved Hide resolved
CHANGES Outdated Show resolved Hide resolved
@AA-Turner
Copy link
Member

Thanks @jfbu! I suggested a couple of changes.

Testing HTML output is a much bigger challenge in general, as it pretty much needs to be a visual comparison -- layered CSS files etc. can affect output with identical HTML content.

A

jfbu and others added 2 commits June 25, 2022 14:22
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@jfbu
Copy link
Contributor Author

jfbu commented Jun 25, 2022

Thanks @jfbu! I suggested a couple of changes.

ok thanks, applied.

I see now that you had been defining and using docutils_version_info elsewhere in basic.css_t already. And that we require Docutils >= 0.14 which added docutils.__version_info__.

About the change to CHANGES, I had noticed you used HTML Theme however I found this terminology a bit unclear.
Your suggestion applied anyhow.

Testing HTML output is a much bigger challenge in general, as it pretty much needs to be a visual comparison -- layered CSS files etc. can affect output with identical HTML content.

Yes, understood, however I was surprised that we (I initially wrote "you" by mistake ;-) ) did not have a check which would have signaled html output (I mean the html files, not how they are rendered, and not even the CSS files) got modified by the upstream Docutils 0.18 changes.

Visual comparison is imho simply not feasible and will always let things pass through. I apply it from time to time on our own docs sphinx.pdf but very superficially, trying more to detect changes in LaTeX compilation log behavior.

@AA-Turner
Copy link
Member

About the change to CHANGES, I had noticed you used HTML Theme however I found this terminology a bit unclear. Your suggestion applied anyhow.

I just use it as it seems to be the precedent for CSS changes etc.

Yes, understood, however I was surprised that we (I initially wrote "you" by mistake ;-) ) did not have a check which would have signaled html output (I mean the html files, not how they are rendered, and not even the CSS files) got modified by the upstream Docutils 0.18 changes.

Hmm, perhaps we can add such a thing. It is difficult to construct a short test file that hits all the various permutations of reST syntax, though.

A

@AA-Turner AA-Turner merged commit 4b48233 into sphinx-doc:5.0.x Jun 25, 2022
@jfbu jfbu deleted the 10594_colon_doctutils018 branch June 26, 2022 08:21
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants