Skip to content

Commit 8756f3a

Browse files
ptr727claude
andcommitted
Quote Each Path in Both Refusals, So a Space or Comma Cannot Read as a Separator
Both messages joined raw paths with commas, so a single path holding a comma read as two and one holding a space read ambiguously. That is worst exactly when the message matters, since it fires on a path the caller did not expect to be there. One `quoted` helper serves both, rather than a repr at each site, since the two messages drifting apart is the smaller version of the problem this PR is about. Two cases added, one per message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent cbb5684 commit 8756f3a

2 files changed

Lines changed: 31 additions & 7 deletions

File tree

scripts/prose_lint.py

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,15 @@ def repo_prefix(root: Path) -> str:
199199
return r.stdout.strip() if r.returncode == 0 else ''
200200

201201

202+
def quoted(paths) -> str:
203+
"""Paths as a sorted, quoted, comma-joined list for an error message.
204+
205+
Quoted because a path holding a space or a comma is indistinguishable from two paths once
206+
joined, which makes the message unreadable exactly when it names something unexpected.
207+
"""
208+
return ', '.join(repr(str(p)) for p in sorted(paths))
209+
210+
202211
def repo_root(path: Path) -> str:
203212
"""The repository top level containing `path`, or '' when git cannot say."""
204213
start = path if path.is_dir() else path.parent
@@ -1114,16 +1123,16 @@ def main(argv: list[str] | None = None) -> int:
11141123
# `discover` reads a non-file, non-directory argument as `.`, so a typo scanned the caller's directory while the rule set anchored on the missing path's parent.
11151124
absent = [p for p in scan_paths if not Path(p).exists()]
11161125
if absent:
1117-
print(f"error: requested path(s) do not exist: {', '.join(sorted(absent))}. Refusing "
1118-
'rather than falling back to the current directory, which would scan one tree and '
1119-
'choose the rule set from another.', file=sys.stderr)
1126+
# Quoted, since a path holding a space or a comma is unreadable in a bare comma-joined list.
1127+
print(f"error: requested path(s) do not exist: {quoted(absent)}. Refusing rather than "
1128+
'falling back to the current directory, which would scan one tree and choose the '
1129+
'rule set from another.', file=sys.stderr)
11201130
return 2
11211131
git_roots = {found for found in (repo_root(Path(p)) for p in scan_paths) if found}
11221132
if len(git_roots) > 1:
1123-
print('error: the requested paths span more than one repository (' +
1124-
', '.join(sorted(git_roots)) + '). Each declares its own workflow model, so no '
1125-
'single rule set is correct for all of them. Run the gate once per repository.',
1126-
file=sys.stderr)
1133+
print(f'error: the requested paths span more than one repository ({quoted(git_roots)}). '
1134+
'Each declares its own workflow model, so no single rule set is correct for all of '
1135+
'them. Run the gate once per repository.', file=sys.stderr)
11271136
return 2
11281137
# A file anchors on its own parent rather than on `.`, which is where the caller stands.
11291138
if git_roots:

scripts/test_prose_lint.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1868,6 +1868,21 @@ def test_a_path_that_does_not_exist_is_refused_rather_than_absorbed(self) -> Non
18681868
[str(self.tmp / 'no-such-dir'), '--check', 'home-path']))
18691869
self.assertIn('do not exist', self.err.getvalue())
18701870

1871+
def test_a_path_holding_a_space_or_comma_is_quoted_in_the_refusal(self) -> None:
1872+
"""Joined bare, one path with a comma in it reads as two paths and the message misleads."""
1873+
awkward = self.tmp / 'a dir, with comma'
1874+
with mock.patch.object(prose_lint, 'repo_root', return_value=''):
1875+
self.assertEqual(2, prose_lint.main([str(awkward), '--check', 'home-path']))
1876+
self.assertIn(repr(str(awkward)), self.err.getvalue())
1877+
1878+
def test_repository_roots_are_quoted_in_the_span_refusal(self) -> None:
1879+
spaced = self.tmp / 'root with space'
1880+
spaced.mkdir()
1881+
with mock.patch.object(prose_lint, 'repo_root', side_effect=lambda p: str(Path(p))):
1882+
self.assertEqual(2, prose_lint.main(
1883+
[str(self.release), str(spaced), '--check', 'home-path']))
1884+
self.assertIn(repr(str(spaced)), self.err.getvalue())
1885+
18711886
def test_an_existing_path_is_not_refused_by_that_check(self) -> None:
18721887
"""The guard must not reject the ordinary case it sits in front of."""
18731888
good = self.tmp / 'good.md'

0 commit comments

Comments
 (0)