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

[rst] Add level option to rubric directive #12506

Merged
merged 6 commits into from
Jul 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
26 changes: 26 additions & 0 deletions sphinx/directives/patches.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,38 @@ def add_target(self, ret: list[Node]) -> None:
ret.insert(0, target)


class Rubric(SphinxDirective):
"""A patch of the docutils' :rst:dir:`rubric` directive,
which adds a level option to specify the heading level of the rubric.
"""

required_arguments = 1
optional_arguments = 0
final_argument_whitespace = True
option_spec = {
'class': directives.class_option,
'name': directives.unchanged,
'level': lambda c: directives.choice(c, ('1', '2', '3', '4', '5')),
}

def run(self) -> list[Node]:
set_classes(self.options)
rubric_text = self.arguments[0]
textnodes, messages = self.parse_inline(rubric_text, lineno=self.lineno)
rubric = nodes.rubric(rubric_text, '', *textnodes, **self.options)
self.add_name(rubric)
if 'level' in self.options:
rubric['level'] = int(self.options['level'])
return [rubric, *messages]


def setup(app: Sphinx) -> ExtensionMetadata:
directives.register_directive('figure', Figure)
directives.register_directive('meta', Meta)
directives.register_directive('csv-table', CSVTable)
directives.register_directive('code', Code)
directives.register_directive('math', MathDirective)
directives.register_directive('rubric', Rubric)

return {
'version': 'builtin',
Expand Down
14 changes: 14 additions & 0 deletions sphinx/writers/html5.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,20 @@ def depart_title(self, node: Element) -> None:

super().depart_title(node)

# overwritten
def visit_rubric(self, node: Element) -> None:
if level := node.get("level"):
self.body.append(self.starttag(node, f'h{level}', '', CLASS='rubric'))
else:
super().visit_rubric(node)

# overwritten
def depart_rubric(self, node: Element) -> None:
if level := node.get("level"):
self.body.append(f'</h{level}>\n')
else:
Comment on lines +531 to +533
Copy link
Member

Choose a reason for hiding this comment

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

Add an assert on the allowed level (to avoid people injecting invalid attributes). And make sure that it's also of type int.

The level should not be at most 6 since otherwise it's not a valid HTML tag.

Copy link
Member Author

Choose a reason for hiding this comment

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

a warning for unsupported level values has been added

Copy link
Member

Choose a reason for hiding this comment

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

Say node['level'] = 9, the warning would be emitted on visit_ with no opening tag, but the closing </h9> tag would still be added. This isn't possible using the directive but we should guard against it (and test appropriately).

super().depart_rubric(node)

# overwritten
def visit_literal_block(self, node: Element) -> None:
if node.rawsource != node.astext():
Expand Down
9 changes: 8 additions & 1 deletion sphinx/writers/latex.py
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,14 @@ def depart_seealso(self, node: Element) -> None:
def visit_rubric(self, node: Element) -> None:
if len(node) == 1 and node.astext() in ('Footnotes', _('Footnotes')):
raise nodes.SkipNode
self.body.append(r'\subsubsection*{')
tag = {
1: 'section',
2: 'subsection',
3: 'subsubsection',
4: 'paragraph',
5: 'subparagraph',
}.get(node.get('level', 0), 'subsubsection')
Copy link
Member

Choose a reason for hiding this comment

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

Add also an assert here because people are not meant to inject a level deeper than what should be allowed. And possibly add a warning to indicate that a level outside 0-5 (and not 0-6!) is not allowed.

Actually, if someone uses level 6, then it's fine in HTML but in LaTeX you would have a subsubsection which may bigger than a subpargraph. So for level 6, it should 'subparagraph' (I'm not sure if the document class has a subparagraph, maybe you should check all levels + non-levels in tests)

Copy link
Member Author

@chrisjsewell chrisjsewell Jul 14, 2024

Choose a reason for hiding this comment

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

This now uses the "correct" logic, using the self.sectionnames and self.top_sectionlevel variables.

a warning for unsupported level values has also been added

self.body.append(rf'\{tag}*{{')
self.context.append('}' + CR)
self.in_title = 1

Expand Down
4 changes: 4 additions & 0 deletions tests/roots/test-markup-rubric/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@ test-markup-rubric

.. rubric:: This is
a multiline rubric

.. rubric:: A rubric with a heading level
:level: 2
Copy link
Member

Choose a reason for hiding this comment

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

Check levels 1 to 6 and invalid levels as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

:class: my-class
8 changes: 8 additions & 0 deletions tests/test_builders/test_build_html_5_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,11 @@ def checker(nodes):
def test_html5_output(app, cached_etree_parse, fname, path, check):
app.build()
check_xpath(cached_etree_parse(app.outdir / fname), fname, path, check)


@pytest.mark.sphinx('html', testroot='markup-rubric')
def test_html5_rubric(app):
app.build()
content = (app.outdir / 'index.html').read_text(encoding='utf8')
assert '<p class="rubric">This is a rubric</p>' in content
assert '<h2 class="my-class rubric">A rubric with a heading level</h2>' in content
8 changes: 8 additions & 0 deletions tests/test_builders/test_build_latex.py
Original file line number Diff line number Diff line change
Expand Up @@ -1759,3 +1759,11 @@ def test_one_parameter_per_line(app, status, warning):
assert ('\\pysiglinewithargsret{\\sphinxbfcode{\\sphinxupquote{hello}}}' in result)

assert ('\\pysigwithonelineperarg{\\sphinxbfcode{\\sphinxupquote{foo}}}' in result)


@pytest.mark.sphinx('latex', testroot='markup-rubric')
def test_latex_rubric(app):
app.build()
content = (app.outdir / 'test.tex').read_text(encoding='utf8')
assert r'\subsubsection*{This is a rubric}' in content
assert r'\subsection*{A rubric with a heading level}' in content
1 change: 1 addition & 0 deletions tests/test_builders/test_build_texinfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def test_texinfo_rubric(app, status, warning):
output = (app.outdir / 'python.texi').read_text(encoding='utf8')
assert '@heading This is a rubric' in output
assert '@heading This is a multiline rubric' in output
assert '@heading A rubric with a heading level' in output


@pytest.mark.sphinx('texinfo', testroot='markup-citation')
Expand Down