Skip to content

Commit f37f2ed

Browse files
committed
Simplify solution titles and add order validation
- Simplified solution title to just 'Solution' when exercise_style='solution_follow_exercise' (instead of 'Solution to Exercise #.#' since solution follows exercise) - Clean up redundant code in post_transforms.py title building - Add node order tracking and validation system - Tracks exercise/solution node order as documents are read - Validates solutions follow their exercises when config is set - Reports helpful warnings with line numbers and labels - Warning messages prefixed with [sphinx-exercise] for clarity - Warnings include: - 'Solution X does not follow exercise Y' with line numbers - Cross-document reference detection - Only runs when exercise_style='solution_follow_exercise' - Add comprehensive tests for order validation - Update test assertions to check for simplified titles
1 parent 69d0345 commit f37f2ed

File tree

4 files changed

+249
-30
lines changed

4 files changed

+249
-30
lines changed

sphinx_exercise/__init__.py

Lines changed: 129 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,13 @@ def purge_exercises(app: Sphinx, env: BuildEnvironment, docname: str) -> None:
8787
for label in remove_labels:
8888
del env.sphinx_exercise_registry[label]
8989

90+
# Purge node order tracking for this document
91+
if (
92+
hasattr(env, "sphinx_exercise_node_order")
93+
and docname in env.sphinx_exercise_node_order
94+
):
95+
del env.sphinx_exercise_node_order[docname]
96+
9097

9198
def merge_exercises(
9299
app: Sphinx, env: BuildEnvironment, docnames: Set[str], other: BuildEnvironment
@@ -103,6 +110,16 @@ def merge_exercises(
103110
**other.sphinx_exercise_registry,
104111
}
105112

113+
# Merge node order tracking
114+
if not hasattr(env, "sphinx_exercise_node_order"):
115+
env.sphinx_exercise_node_order = {}
116+
117+
if hasattr(other, "sphinx_exercise_node_order"):
118+
env.sphinx_exercise_node_order = {
119+
**env.sphinx_exercise_node_order,
120+
**other.sphinx_exercise_node_order,
121+
}
122+
106123

107124
def init_numfig(app: Sphinx, config: Config) -> None:
108125
"""Initialize numfig"""
@@ -127,23 +144,133 @@ def copy_asset_files(app: Sphinx, exc: Union[bool, Exception]):
127144
copy_asset(path, str(Path(app.outdir).joinpath("_static").absolute()))
128145

129146

147+
def validate_exercise_solution_order(app: Sphinx, env: BuildEnvironment) -> None:
148+
"""
149+
Validate that solutions follow their referenced exercises when
150+
exercise_style='solution_follow_exercise' is set.
151+
"""
152+
# Only validate if the config option is set
153+
if app.config.exercise_style != "solution_follow_exercise":
154+
return
155+
156+
if not hasattr(env, "sphinx_exercise_node_order"):
157+
return
158+
159+
logger = logging.getLogger(__name__)
160+
161+
# Process each document
162+
for docname, nodes in env.sphinx_exercise_node_order.items():
163+
# Build a map of exercise labels to their positions and info
164+
exercise_info = {}
165+
for i, node_info in enumerate(nodes):
166+
if node_info["type"] == "exercise":
167+
exercise_info[node_info["label"]] = {
168+
"position": i,
169+
"line": node_info.get("line"),
170+
}
171+
172+
# Check each solution
173+
for i, node_info in enumerate(nodes):
174+
if node_info["type"] == "solution":
175+
target_label = node_info["target_label"]
176+
solution_label = node_info["label"]
177+
solution_line = node_info.get("line")
178+
179+
if not target_label:
180+
continue
181+
182+
# Check if target exercise exists in this document
183+
if target_label not in exercise_info:
184+
# Exercise is in a different document or doesn't exist
185+
docpath = env.doc2path(docname)
186+
path = str(Path(docpath).with_suffix(""))
187+
188+
# Build location string with line number if available
189+
location = f"{path}:{solution_line}" if solution_line else path
190+
191+
logger.warning(
192+
f"[sphinx-exercise] Solution '{solution_label}' references exercise '{target_label}' "
193+
f"which is not in the same document. When exercise_style='solution_follow_exercise', "
194+
f"solutions should appear in the same document as their exercises.",
195+
location=location,
196+
color="yellow",
197+
)
198+
continue
199+
200+
# Check if solution comes after exercise
201+
exercise_data = exercise_info[target_label]
202+
exercise_pos = exercise_data["position"]
203+
exercise_line = exercise_data.get("line")
204+
205+
if i <= exercise_pos:
206+
docpath = env.doc2path(docname)
207+
path = str(Path(docpath).with_suffix(""))
208+
209+
# Build more informative message with line numbers
210+
if solution_line and exercise_line:
211+
location = f"{path}:{solution_line}"
212+
msg = (
213+
f"[sphinx-exercise] Solution '{solution_label}' (line {solution_line}) does not follow "
214+
f"exercise '{target_label}' (line {exercise_line}). "
215+
f"When exercise_style='solution_follow_exercise', solutions should "
216+
f"appear after their referenced exercises."
217+
)
218+
elif solution_line:
219+
location = f"{path}:{solution_line}"
220+
msg = (
221+
f"[sphinx-exercise] Solution '{solution_label}' does not follow exercise '{target_label}'. "
222+
f"When exercise_style='solution_follow_exercise', solutions should "
223+
f"appear after their referenced exercises."
224+
)
225+
else:
226+
location = path
227+
msg = (
228+
f"[sphinx-exercise] Solution '{solution_label}' does not follow exercise '{target_label}'. "
229+
f"When exercise_style='solution_follow_exercise', solutions should "
230+
f"appear after their referenced exercises."
231+
)
232+
233+
logger.warning(msg, location=location, color="yellow")
234+
235+
130236
def doctree_read(app: Sphinx, document: Node) -> None:
131237
"""
132238
Read the doctree and apply updates to sphinx-exercise nodes
133239
"""
134240

135241
domain = cast(StandardDomain, app.env.get_domain("std"))
136242

243+
# Initialize node order tracking for this document
244+
if not hasattr(app.env, "sphinx_exercise_node_order"):
245+
app.env.sphinx_exercise_node_order = {}
246+
247+
docname = app.env.docname
248+
if docname not in app.env.sphinx_exercise_node_order:
249+
app.env.sphinx_exercise_node_order[docname] = []
250+
137251
# Traverse sphinx-exercise nodes
138252
for node in findall(document):
139253
if is_extension_node(node):
140254
name = node.get("names", [])[0]
141255
label = document.nameids[name]
142-
docname = app.env.docname
143256
section_name = node.attributes.get("title")
144257
domain.anonlabels[name] = docname, label
145258
domain.labels[name] = docname, label, section_name
146259

260+
# Track node order for validation
261+
node_type = node.get("type", "unknown")
262+
node_label = node.get("label", "")
263+
target_label = node.get("target_label", None) # Only for solution nodes
264+
265+
app.env.sphinx_exercise_node_order[docname].append(
266+
{
267+
"type": node_type,
268+
"label": node_label,
269+
"target_label": target_label,
270+
"line": node.line if hasattr(node, "line") else None,
271+
}
272+
)
273+
147274

148275
def setup(app: Sphinx) -> Dict[str, Any]:
149276
app.add_config_value("hide_solutions", False, "env")
@@ -153,6 +280,7 @@ def setup(app: Sphinx) -> Dict[str, Any]:
153280
app.connect("env-purge-doc", purge_exercises) # event order - 5 per file
154281
app.connect("doctree-read", doctree_read) # event order - 8
155282
app.connect("env-merge-info", merge_exercises) # event order - 9
283+
app.connect("env-updated", validate_exercise_solution_order) # event order - 10
156284
app.connect("build-finished", copy_asset_files) # event order - 16
157285

158286
app.add_node(

sphinx_exercise/post_transforms.py

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -141,39 +141,26 @@ def resolve_solution_title(app, node, exercise_node):
141141
exercise_title = exercise_node.children[0]
142142
if isinstance(title, solution_title):
143143
entry_title_text = node.get("title")
144-
updated_title_text = " " + exercise_title.children[0].astext()
145-
if isinstance(exercise_node, exercise_enumerable_node):
146-
node_number = get_node_number(app, exercise_node, "exercise")
147-
updated_title_text += f" {node_number}"
144+
148145
# New Title Node
149146
updated_title = docutil_nodes.title()
150147

151148
# Check if exercise_style is set to "solution_follow_exercise"
152149
if app.config.exercise_style == "solution_follow_exercise":
153-
# Don't create hyperlink - just add plain text and nodes
150+
# Simple title: just "Solution" without reference to exercise
154151
updated_title += docutil_nodes.Text(entry_title_text)
155-
updated_title += docutil_nodes.Text(updated_title_text)
156-
node["title"] = entry_title_text + updated_title_text
157-
158-
# Parse Custom Titles from Exercise
159-
if len(exercise_title.children) > 1:
160-
subtitle = exercise_title.children[1]
161-
if isinstance(subtitle, exercise_subtitle):
162-
updated_title += docutil_nodes.Text(" (")
163-
for child in subtitle.children:
164-
if isinstance(child, docutil_nodes.math):
165-
# Ensure mathjax is loaded for pages that only contain
166-
# references to nodes that contain math
167-
domain = app.env.get_domain("math")
168-
domain.data["has_equations"][app.env.docname] = True
169-
# Add child directly (could be text or math node)
170-
updated_title += child.deepcopy()
171-
updated_title += docutil_nodes.Text(")")
152+
node["title"] = entry_title_text
172153
else:
154+
# Build full title with exercise reference
155+
updated_title_text = " " + exercise_title.children[0].astext()
156+
if isinstance(exercise_node, exercise_enumerable_node):
157+
node_number = get_node_number(app, exercise_node, "exercise")
158+
updated_title_text += f" {node_number}"
159+
173160
# Create hyperlink (original behavior)
174161
wrap_reference = build_reference_node(app, exercise_node)
175162
wrap_reference += docutil_nodes.Text(updated_title_text)
176-
node["title"] = entry_title_text + updated_title_text
163+
177164
# Parse Custom Titles from Exercise
178165
if len(exercise_title.children) > 1:
179166
subtitle = exercise_title.children[1]
@@ -187,8 +174,11 @@ def resolve_solution_title(app, node, exercise_node):
187174
domain.data["has_equations"][app.env.docname] = True
188175
wrap_reference += child
189176
wrap_reference += docutil_nodes.Text(")")
177+
178+
# Build the title with entry text + hyperlinked reference
190179
updated_title += docutil_nodes.Text(entry_title_text)
191180
updated_title += wrap_reference
181+
node["title"] = entry_title_text + updated_title_text
192182

193183
updated_title.parent = title.parent
194184
node.children[0] = updated_title

tests/test_exercise_style.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,11 @@ def test_solution_no_link(app):
3535
len(links) == 0
3636
), "Solution title should not contain hyperlink when exercise_style='solution_follow_exercise'"
3737

38-
# Check that the title text still contains the exercise reference
38+
# Check that the title is just "Solution" without exercise reference
3939
title_text = title.get_text()
40-
assert "Exercise" in title_text
41-
assert "This is a title" in title_text
40+
assert (
41+
title_text.strip() == "Solution to"
42+
), "Solution title should be just 'Solution to' when exercise_style='solution_follow_exercise'"
4243

4344

4445
@pytest.mark.sphinx("html", testroot="mybook", confoverrides={"exercise_style": ""})
@@ -93,7 +94,8 @@ def test_solution_no_link_unenum(app):
9394
len(links) == 0
9495
), "Solution title should not contain hyperlink when exercise_style='solution_follow_exercise'"
9596

96-
# Check that the title text still contains the exercise reference
97+
# Check that the title is just "Solution" without exercise reference
9798
title_text = title.get_text()
98-
assert "Exercise" in title_text
99-
assert "This is a title" in title_text
99+
assert (
100+
title_text.strip() == "Solution to"
101+
), "Solution title should be just 'Solution to' when exercise_style='solution_follow_exercise'"

tests/test_order_validation.py

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
"""Test exercise-solution order validation when exercise_style='solution_follow_exercise'"""
2+
from pathlib import Path
3+
import pytest
4+
5+
6+
@pytest.mark.sphinx(
7+
"html",
8+
testroot="mybook",
9+
confoverrides={"exercise_style": "solution_follow_exercise"},
10+
)
11+
def test_solution_before_exercise_warning(app, warning):
12+
"""Test that a warning is raised when solution appears before exercise"""
13+
# Create a temporary file with solution before exercise
14+
srcdir = Path(app.srcdir)
15+
test_file = srcdir / "test_wrong_order.rst"
16+
17+
test_content = """
18+
Wrong Order Test
19+
================
20+
21+
.. solution:: my-test-exercise
22+
:label: sol-wrong-order
23+
24+
This solution appears before the exercise!
25+
26+
.. exercise:: Test Exercise
27+
:label: my-test-exercise
28+
29+
This is the exercise that should come first.
30+
"""
31+
32+
test_file.write_text(test_content)
33+
34+
# Build and check for warnings
35+
app.build()
36+
37+
warnings_text = warning.getvalue()
38+
39+
# Should warn about solution not following exercise
40+
assert "does not follow" in warnings_text or "Solution" in warnings_text
41+
assert "sol-wrong-order" in warnings_text
42+
assert "my-test-exercise" in warnings_text
43+
44+
# Clean up
45+
test_file.unlink()
46+
47+
48+
@pytest.mark.sphinx(
49+
"html",
50+
testroot="mybook",
51+
confoverrides={"exercise_style": "solution_follow_exercise"},
52+
)
53+
def test_solution_different_document_warning(app, warning):
54+
"""Test that a warning is raised when solution and exercise are in different documents"""
55+
# The existing test files should have some cross-document references
56+
app.build()
57+
58+
# We expect the build to succeed but potentially with warnings
59+
# about cross-document references
60+
assert app.statuscode == 0 or app.statuscode is None
61+
62+
63+
@pytest.mark.sphinx(
64+
"html",
65+
testroot="mybook",
66+
confoverrides={"exercise_style": ""}, # Default - no validation
67+
)
68+
def test_no_validation_when_config_not_set(app, warning):
69+
"""Test that validation doesn't run when exercise_style is not set"""
70+
# Create a file with solution before exercise
71+
srcdir = Path(app.srcdir)
72+
test_file = srcdir / "test_no_validation.rst"
73+
74+
test_content = """
75+
No Validation Test
76+
==================
77+
78+
.. solution:: my-test-ex
79+
:label: sol-no-val
80+
81+
Solution before exercise - but should not warn
82+
83+
.. exercise:: Test
84+
:label: my-test-ex
85+
86+
Exercise content
87+
"""
88+
89+
test_file.write_text(test_content)
90+
91+
app.build()
92+
93+
warnings_text = warning.getvalue()
94+
95+
# Should NOT warn about order when config is not set
96+
assert "appears before" not in warnings_text
97+
98+
# Clean up
99+
test_file.unlink()

0 commit comments

Comments
 (0)