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

Please test with Docutils 0.19b1 #113

Closed
AA-Turner opened this issue Jun 26, 2022 · 13 comments
Closed

Please test with Docutils 0.19b1 #113

AA-Turner opened this issue Jun 26, 2022 · 13 comments

Comments

@AA-Turner
Copy link

Hi,

We've just released Docutils 0.19b1, and intend to release 0.19 final on 05/07/2022 (a week on Tuesday). Please would you test with the pre-release, and raise any issues to me either on this issue or on the Docutils tracker?

Thanks,
Adam

@2bndy5
Copy link
Collaborator

2bndy5 commented Jun 26, 2022

We're pinned to whatever version sphinx is pinned to. Currently that excludes any 0.19 versions. So, we have no reason to accommodate your request (unless Sphinx decides to update their current docutils requirements).

@2bndy5 2bndy5 closed this as completed Jun 26, 2022
@AA-Turner
Copy link
Author

Hi Brendan,

Sphinx uses Docutils' HTML writers, and we've made changes in the HTML writers in DU 19. Sphinx is also likely to declare support for DU 19 in Sphinx 5.1 (all being well) -- if the theme breaks because of changes, we'd like to know as early as possible so we can mitigate in Docutils.

Installing Docutils 0.19 will work with Sphinx 5.0.2, as we (Sphinx) test with Docutils master.

Even if a quick visual comparison of DU 18.1 vs DU 19b1, I would appreciate it if possible.

A

@2bndy5
Copy link
Collaborator

2bndy5 commented Jun 26, 2022

Ah, ok. Well, we're a small dev group with other projects going, so I can't promise any feedback soon. This theme does some heavy monkey-patching of sphinx internals, so I'm not sure our feedback will be unblemished by this.

@2bndy5 2bndy5 reopened this Jun 26, 2022
@AA-Turner
Copy link
Author

No worries at all, completely understand!

As an aside, if there are changes you'd like to upstream to Sphinx I would be happy to look at these on Sphinx's repo.

A

@2bndy5
Copy link
Collaborator

2bndy5 commented Jun 26, 2022

if there are changes you'd like to upstream to Sphinx I would be happy to look at these on Sphinx's repo.

Be careful what you wish for.

  1. There's still some unresolved issues from @jbms.
  2. Also @jbms PRs are getting stale.
  3. Sphinx v6.0 will break our theme unless we monkey-patch literal blocks with its current capability. I've voiced my concern at Why deprecate the html_codeblock_linenos_style option? sphinx-doc/sphinx#10265

@AA-Turner
Copy link
Author

I don't know C++, so I'm not sure how much help I'd be on those issues (if I don't need to know C++ to review them though I am happy to, @jbms -- could you advise here please?).

I've commented on sphinx-doc/sphinx#10265, I see the issue there -- hopefully we can reach a happy medium but if not reverting the removal is entirely an option.

I'll try and find time for the other issues, at the moment that is limited to weekends pretty much though, sorry.

A

@2bndy5
Copy link
Collaborator

2bndy5 commented Jun 27, 2022

I just did a quick peek of our docs built with docutils 0.19b1, and I see no new errors that we aren't already aware of.

@2bndy5
Copy link
Collaborator

2bndy5 commented Jun 27, 2022

I do get these warnings:

WARNING: extlinks: Sphinx-6.0 will require a caption string to contain exactly one '%s' and all other '%' need to be escaped as '%%'.
WARNING: extlinks: Sphinx-6.0 will require a caption string to contain exactly one '%s' and all other '%' need to be escaped as '%%'.
WARNING: extlinks: Sphinx-6.0 will require a caption string to contain exactly one '%s' and all other '%' need to be escaped as '%%'.

but I don't think that's related to docutils.
Our docs' conf.py shouldn't be triggering these warnings though, so its kinda confusing.

extlinks = {
"duref": (
"http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#%s",
"",
),
"durole": ("http://docutils.sourceforge.net/docs/ref/rst/roles.html#%s", ""),
"dudir": ("http://docutils.sourceforge.net/docs/ref/rst/directives.html#%s", ""),
}

And I don't see the extlinks API used in our code base.


All use of extlinks extension is copied from the RST primer page in sphinx docs. So, if we need to update that, then it wouldn't be hard.

@jbms
Copy link
Owner

jbms commented Jun 27, 2022

Note: I already fixed those extlinks warnings in #99.

@2bndy5
Copy link
Collaborator

2bndy5 commented Jun 27, 2022

I fixed the warnings by changing the config.py to

extlinks = {
    "duref": (
        "http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#%s",
        "%s reference",
    ),
    "durole": ("http://docutils.sourceforge.net/docs/ref/rst/roles.html#%s", "%s role"),
    "dudir": (
        "http://docutils.sourceforge.net/docs/ref/rst/directives.html#%s",
        "%s directive",
    ),
}

Apparently, the warning is triggered for empty caption strings.

@2bndy5
Copy link
Collaborator

2bndy5 commented Jun 27, 2022

I don't see a cause for concern with docutils 0.19b1 (surprisingly). I'm more concerned with changes to sphinx itself.

Please re-open if there is a newer 0.19 beta to test (or start a new thread).

@2bndy5 2bndy5 closed this as completed Jun 27, 2022
@jbms
Copy link
Owner

jbms commented Jun 27, 2022

@AA-Turner Thanks for your offer to help.

As @2bndy5 mentioned, there are a bunch of C++-related Sphinx PRs that have been sitting for a few months now. I have pinged @jakobandersen a few times regarding them, but he indicated that unfortunately he doesn't have much time to review Sphinx PRs these days. I think the C/C++ domains in Sphinx could benefit from an additional maintainer, but indeed the code is rather complicated. I think my PRs are in fairly good shape already --- they have unit tests --- but I imagine there might be some things Jakob would do differently.

There are a number features in this theme that could be good candidates to upstream into Sphinx itself:

@2bndy5
Copy link
Collaborator

2bndy5 commented Jun 27, 2022

I haven't been able to ping Jake for breathe contributions either - seems to be MIA for now.

If you'd like to open thread(s) for upstreaming the domain/obj_desc customizations, feel free. This theme is still technically in beta, so everything we monkey patch feels rather brittle until equivalent additions are made to Sphinx. Right now, the monkey patching has only broken the breathe extension (that we know of).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants