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 Sphinx 4.0 support for docsite #74798

Closed
wants to merge 2 commits into from

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Port of ansible-community/antsibull#273. Sphinx 4 no longer accepts instantiated lexer classes.

CC @abadger @acozine @relrod

ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
COMPONENT NAME

docs/docsite/_extensions/pygments_lexer.py

@ansibot ansibot added affects_2.12 bug This issue/PR relates to a bug. docs This issue/PR relates to or includes documentation. docs_only All changes are to files within the docs/docsite/ directory needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. small_patch support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels May 22, 2021
@ansibot
Copy link
Contributor

ansibot commented May 22, 2021

The test ansible-test sanity --test docs-build [explain] failed with 1 error:

docs/docsite/rst/index.rst:125:0: literal-block-lex-error: Could not lex literal_block as "ansible-output". Highlighting skipped.

click here for bot help

@ansibot ansibot removed the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. label May 22, 2021
@felixfontein
Copy link
Contributor Author

The actual error is: docs/docsite/rst/user_guide/intro_getting_started.rst:125: WARNING: Could not lex literal_block as "ansible-output". Highlighting skipped.

Interestingly, it goes away with AnsibleOutputLexer(startinline=True), but also with AnsibleOutputLexer(startinline=False) and AnsibleOutputLexer(). It only happens when only the class name is specified here. (This is both for Sphinx 2.1.2, which the build process currently uses, and the latest pre-4.0 version 3.5.4.) Also interestingly, when passing on startinline directly in AnsibleOutputLexer.__init__, or even explicitly writing it into options, and at the same time just writing the class name, it also produces this error.

@ansibot ansibot added the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. label May 22, 2021
@felixfontein
Copy link
Contributor Author

Well, I think I solved this. The issue is that this error should be there, but was not shown for whatever reason if the lexer was passed already instantiated. You can see on the actual output in https://docs.ansible.com/ansible/devel/user_guide/intro_getting_started.html#action-run-your-first-playbook (you need to scroll down a bit) that there's a formatting error. The reason is that the fields rescued and ignored were added to the status at some point, and the lexer does not know about them. 7ad0ea1 fixes that.

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 22, 2021
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label May 25, 2021
@acozine
Copy link
Contributor

acozine commented May 26, 2021

Tested locally. This gets rid of the Sphinx 4 error (which was: TypeError: 'AnsibleOutputLexer' object is not callable).

Note that Sphinx 4 requires Jinja2 <3.0, at least as of 4.0.1.

@webknjaz
Copy link
Member

FWIW I've absorbed this patch into the theme for now.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 31, 2021
@webknjaz
Copy link
Member

webknjaz commented Jun 1, 2021

FWIW I've absorbed this patch into the theme for now.

FYI if #74318 gets merged first, the current PR will become unnecessary.

@acozine
Copy link
Contributor

acozine commented Jun 8, 2021

Fixed as part of #74318.

@acozine acozine closed this Jun 8, 2021
@felixfontein felixfontein deleted the sphinx-4 branch June 8, 2021 19:41
@ansible ansible locked and limited conversation to collaborators Jul 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.12 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. docs_only All changes are to files within the docs/docsite/ directory docs This issue/PR relates to or includes documentation. small_patch stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants