Skip to content

Commit 59c1244

Browse files
committed
Revert regression: escape=True also escapes path separators
1 parent 45d39fb commit 59c1244

File tree

2 files changed

+27
-16
lines changed

2 files changed

+27
-16
lines changed

nbgrader/coursedir.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,13 @@ def format_path(
310310
assignment_id: str = '.',
311311
escape: bool = False
312312
) -> str:
313+
"""
314+
Produce an absolute path to an assignment directory.
315+
316+
When escape=True, the non-passed elements of the path are regex-escaped, so the
317+
resulting string can be used as a pattern to match path components.
318+
"""
319+
313320
kwargs = dict(
314321
nbgrader_step=nbgrader_step,
315322
student_id=student_id,
@@ -323,7 +330,10 @@ def format_path(
323330

324331
path = base / self.directory_structure.format(**kwargs)
325332

326-
return str(path)
333+
if escape:
334+
return path.anchor + re.escape(os.path.sep).join(path.parts[1:])
335+
else:
336+
return str(path)
327337

328338
def find_assignments(self,
329339
nbgrader_step: str = "*",

nbgrader/tests/test_coursedir.py

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,25 @@ def test_coursedir_configurable(conf, course_dir):
2020
assert coursedir.root == course_dir
2121

2222

23-
def test_coursedir_format_path(conf):
23+
@pytest.mark.parametrize("root", [None, os.path.sep + "[special]~root"])
24+
def test_coursedir_format_path(conf, root):
25+
if root is not None:
26+
conf.CourseDirectory.root = root
2427
coursedir = CourseDirectory(config=conf)
2528

26-
expected = os.path.join(coursedir.root, "step", "student_id", "assignment_id")
27-
assert coursedir.format_path("step", "student_id", "assignment_id") == expected
29+
# The default includes the un-escaped root
30+
path = os.path.join(coursedir.root, "step", "student_id", "assignment1")
31+
assert coursedir.format_path("step", "student_id", "assignment1") == path
2832

33+
# The escape=True option escapes the root and path separators
2934
escaped = Path(re.escape(coursedir.root))
30-
expected = str(escaped / "step" / "student_id" / "assignment_id")
31-
assert coursedir.format_path("step", "student_id", "assignment_id", escape=True) == expected
35+
expected = escaped.anchor + re.escape(os.path.sep).join(
36+
(escaped / "step" / "student_id" / "(?P<assignment_id>.*)").parts[1:])
3237

38+
actual = coursedir.format_path("step", "student_id", "(?P<assignment_id>.*)", escape=True)
39+
assert actual == expected
3340

34-
def test_coursedir_format_path_with_specials(conf):
35-
conf.CourseDirectory.root = "/[test] root"
36-
coursedir = CourseDirectory(config=conf)
37-
38-
expected = os.path.join("/[test] root", "step", "student_id", "assignment_id")
39-
assert coursedir.format_path("step", "student_id", "assignment_id") == expected
40-
41-
escaped = Path(re.escape(coursedir.root))
42-
expected = str(escaped / "step" / "student_id" / "assignment_id")
43-
assert coursedir.format_path("step", "student_id", "assignment_id", escape=True) == expected
41+
# The escape=True option produces a regex pattern which can match paths
42+
match = re.match(actual, path)
43+
assert match is not None
44+
assert match.group("assignment_id") == "assignment1"

0 commit comments

Comments
 (0)