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

Allow ..image:: directive on 404.rst #21

Merged
merged 6 commits into from
Jun 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 21 additions & 27 deletions notfound/extension.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import os
import docutils
import sphinx

from sphinx.builders.html import DirectoryHTMLBuilder
from sphinx.errors import ExtensionError

from .utils import replace_uris


class BaseURIError(ExtensionError):
"""Exception for malformed base URI."""
Expand Down Expand Up @@ -38,6 +38,7 @@ def html_collect_pages(app):
)]


# https://www.sphinx-doc.org/en/stable/extdev/appapi.html#event-html-page-context
def finalize_media(app, pagename, templatename, context, doctree):
"""
Point media files at our media server.
Expand Down Expand Up @@ -134,29 +135,7 @@ def toctree(*args, **kwargs):
if not toc:
return None

# https://github.com/sphinx-doc/sphinx/blob/2adeb68af1763be46359d5e808dae59d708661b1/sphinx/environment/adapters/toctree.py#L260-L266
for refnode in toc.traverse(docutils.nodes.reference):
refuri = refnode.attributes.get('refuri') # somepage.html (or ../sompage.html)

if isinstance(app.builder, DirectoryHTMLBuilder):
# When the builder is ``DirectoryHTMLBuilder``, refuri will be
# ``../somepage.html``. In that case, we want to remove the
# initial ``../`` to make valid links
if refuri.startswith('../'):
refuri = refuri.replace('../', '')

if app.config.notfound_no_urls_prefix:
refuri = '/{filename}'.format(
filename=refuri,
)
else:
refuri = '/{language}/{version}/{filename}'.format(
language=app.config.language or 'en',
version=os.environ.get('READTHEDOCS_VERSION', 'latest'),
filename=refuri,
)
refnode.replace_attr('refuri', refuri)

replace_uris(app, toc, docutils.nodes.reference, 'refuri')
return app.builder.render_partial(toc)['fragment']

# Apply our custom manipulation to 404.html page only
Expand All @@ -172,14 +151,28 @@ def toctree(*args, **kwargs):
context['toctree'] = toctree


def setup(app):
# https://www.sphinx-doc.org/en/stable/extdev/appapi.html#event-doctree-resolved
Copy link
Member

Choose a reason for hiding this comment

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

Any reason this can't be included in the docstring?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want to display it in the autoapi docs.

def doctree_resolved(app, doctree, docname):
"""
Entry point to register a Sphinx extension.
Generate and override URLs for ``.. image::`` Sphinx directive.

When ``.. image::`` is used in the ``404.rst`` file, this function will
override the URLs to point to the right place.

:param app: Sphinx Application
:type app: sphinx.application.Sphinx
:param doctree: doctree representing the document
:type doctree: docutils.nodes.document
:param docname: name of the document
:type docname: str
"""

if docname == app.config.notfound_pagename:
# Replace image ``uri`` to its absolute version
replace_uris(app, doctree, docutils.nodes.image, 'uri')


def setup(app):
default_context = {
'title': 'Page not found',
'body': '<h1>Page not found</h1>\n\nThanks for trying.',
Expand All @@ -197,6 +190,7 @@ def setup(app):

app.connect('html-collect-pages', html_collect_pages)
app.connect('html-page-context', finalize_media)
app.connect('doctree-resolved', doctree_resolved)

# Sphinx injects some javascript files using ``add_js_file``. The path for
# this file is rendered in the template using ``js_tag`` instead of
Expand Down
39 changes: 39 additions & 0 deletions notfound/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import os
from sphinx.builders.html import DirectoryHTMLBuilder


def replace_uris(app, doctree, nodetype, nodeattr):
"""
Replace ``nodetype`` URIs from ``doctree`` to the proper one.

:param app: Sphinx Application
:type app: sphinx.application.Sphinx
:param doctree: doctree representing the document
:type doctree: docutils.nodes.document
:param nodetype: type of node to replace URIs
:type nodetype: docutils.nodes.Node
:param nodeattr: node attribute to be replaced
:type nodeattr: str
"""
# https://github.com/sphinx-doc/sphinx/blob/2adeb68af1763be46359d5e808dae59d708661b1/sphinx/environment/adapters/toctree.py#L260-L266
for node in doctree.traverse(nodetype):
refuri = node.attributes.get(nodeattr) # somepage.html (or ../sompage.html)

if isinstance(app.builder, DirectoryHTMLBuilder):
# When the builder is ``DirectoryHTMLBuilder``, refuri will be
# ``../somepage.html``. In that case, we want to remove the
# initial ``../`` to make valid links
if refuri.startswith('../'):
refuri = refuri.replace('../', '')

if app.config.notfound_no_urls_prefix:
uri = '/{filename}'.format(
filename=refuri,
)
else:
uri = '/{language}/{version}/{filename}'.format(
language=app.config.language or 'en',
version=os.environ.get('READTHEDOCS_VERSION', 'latest'),
filename=refuri,
)
node.replace_attr(nodeattr, uri)
13 changes: 13 additions & 0 deletions tests/examples/404rst/404.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,16 @@ This file should be rendered instead of the default one.

Variables Sphinx substitution should be allowed here.
Example, version: |version|.

Including an image using ``.. image::`` directive,
Copy link
Member

Choose a reason for hiding this comment

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

I would just put both directives in the same sentence, like: using .. image:: or .. figure::

Copy link
Member Author

Choose a reason for hiding this comment

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

This is used for the tests to check that they render properly. Both directive are needed.

Copy link
Member

Choose a reason for hiding this comment

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

Thought this was an actual doc page p:

should also make the extension to fix the URIs.

.. image:: test.png
:alt: An image


Also, using ``.. figure::`` should work as well.

.. figure:: test.png

Description.
1 change: 1 addition & 0 deletions tests/examples/404rst/test.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
29 changes: 29 additions & 0 deletions tests/test_urls.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# -*- coding: utf-8 -*-

import os
import pytest
import sphinx
Expand Down Expand Up @@ -298,6 +300,33 @@ def test_custom_404_rst_source(app, status, warning):
assert chunk in content


@pytest.mark.sphinx(srcdir=rstsrcdir)
def test_image_on_404_rst_source(app, status, warning):
app.build()
path = app.outdir / '404.html'
assert path.exists() == True

content = open(path).read()

chunks = [
# .. image::
'<img alt="An image" src="/en/latest/test.png" />',
]

# .. figure::
if sphinx.version_info < (2, 0):
chunks.append(
'<div class="figure" id="id1">\n<img alt="/en/latest/test.png" src="/en/latest/test.png" />\n<p class="caption"><span class="caption-text">Description.</span></p>\n</div>'
)
else:
chunks.append(
u'<div class="figure align-center" id="id1">\n<img alt="/en/latest/test.png" src="/en/latest/test.png" />\n<p class="caption"><span class="caption-text">Description.</span><a class="headerlink" href="#id1" title="Permalink to this image">¶</a></p>\n</div>',
)

for chunk in chunks:
assert chunk in content


@pytest.mark.sphinx(
srcdir=srcdir,
buildername='dirhtml',
Expand Down