Skip to content

Commit 4d4279f

Browse files
gaborbernatgaborbernat
authored
docs: fix the iterator order claim in api.rst (#533)
Follow-up to #524, which landed and shipped in 4.11.4 before this review finished. `docs/api.rst` now claims: > User directories come first, then site directories, and each distinct directory appears once. The first half is wrong when `use_site_for_root` is active and the process is root. `_iter_*_dirs` skips the user directory outright rather than yielding it first, so under `multipath` the first entry is a site directory that does not equal `user_*_dir`: ```pycon >>> dirs = Unix(appname="foo", multipath=True, use_site_for_root=True) # as root, XDG_CONFIG_DIRS=/xdg/a:/xdg/b >>> dirs.user_config_dir '/xdg/a/foo:/xdg/b/foo' >>> list(dirs.iter_config_dirs()) ['/xdg/a/foo', '/xdg/b/foo'] ``` `test_iter_dirs_as_root_with_multipath_skips_joined_user_dir` already pins that behaviour, so the code is right and only the sentence is wrong. "The most specific directory comes first" holds in every configuration and still tells callers what they need to merge in the right direction. Two cleanups in the same area while I was there. `test_use_site_for_root_bypasses_xdg_user_vars` kept an inline `monkeypatch.delenv("XDG_RUNTIME_DIR", ...)` after gaining the `_no_xdg_runtime_dir` fixture that does the same thing. And two test signatures stayed exploded across four lines only because a magic trailing comma survived the removal of their `mocker` and `monkeypatch` parameters; both fit on one line now. No behaviour change. `tox -e fix`, `-e type`, `-e docs` clean, full suite passes.
1 parent babf239 commit 4d4279f

4 files changed

Lines changed: 9 additions & 15 deletions

File tree

‎docs/api.rst‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,8 +277,7 @@ Windows. Executables here are available to all users.
277277

278278
These methods are available on :class:`~platformdirs.PlatformDirs` instances. They yield both user and site directories
279279
for a given type, enabling configuration merging and fallback patterns. See :ref:`howto:Merging config from multiple
280-
sources` for a practical example. User directories come first, then site directories, and each distinct directory
281-
appears once.
280+
sources` for a practical example. The most specific directory comes first, and each distinct directory appears once.
282281

283282
- :meth:`~platformdirs.api.PlatformDirsABC.iter_data_dirs` / :meth:`~platformdirs.api.PlatformDirsABC.iter_data_paths`
284283
- :meth:`~platformdirs.api.PlatformDirsABC.iter_config_dirs` /

‎docs/changelog/533.doc.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Correct the ordering note on the iterator methods. ``use_site_for_root`` drops the user directory entirely, so the
2+
iterators are documented as yielding the most specific directory first rather than always yielding the user one.

‎src/platformdirs/unix.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,8 @@ def site_cache_path(self) -> Path:
227227
return self._first_item_as_path_if_multipath(self.site_cache_dir)
228228

229229
def _iter_config_dirs(self) -> Iterator[str]:
230-
# Under multipath the user dir is an os.pathsep-joined string that equals no single site entry, so the
231-
# deduplication in iter_config_dirs cannot drop it. Skip it here instead.
230+
# Under multipath the user dir is an os.pathsep-joined string that no single site entry matches, so the
231+
# dedupe in iter_config_dirs cannot drop it. Skip it here instead.
232232
if not self._use_site:
233233
yield self.user_config_dir
234234
yield from self._site_config_dirs

‎tests/test_unix.py‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ def test_use_site_for_root_disabled_as_root(prop: str, expected: str) -> None:
485485
assert result != expected
486486

487487

488-
@pytest.mark.usefixtures("_as_root")
488+
@pytest.mark.usefixtures("_as_root", "_no_xdg_runtime_dir")
489489
@pytest.mark.parametrize(
490490
("xdg_var", "prop", "expected_site"),
491491
[
@@ -500,7 +500,6 @@ def test_use_site_for_root_bypasses_xdg_user_vars(
500500
monkeypatch: pytest.MonkeyPatch, xdg_var: str, prop: str, expected_site: str
501501
) -> None:
502502
monkeypatch.setenv(xdg_var, "/custom/xdg/path")
503-
monkeypatch.delenv("XDG_RUNTIME_DIR", raising=False)
504503
result = getattr(Unix(appname="foo", use_site_for_root=True), prop)
505504
assert result == expected_site
506505

@@ -539,20 +538,14 @@ def test_use_site_iter_dirs_no_duplicates(
539538

540539
@pytest.mark.usefixtures("_as_root", "_no_xdg_runtime_dir")
541540
@pytest.mark.parametrize(("func", "expected"), _SINGLE_SITE_ITER_CASES)
542-
def test_use_site_iter_dirs_no_duplicates_single_site_dir(
543-
func: Callable[[Unix], Iterator[str]],
544-
expected: str,
545-
) -> None:
541+
def test_use_site_iter_dirs_no_duplicates_single_site_dir(func: Callable[[Unix], Iterator[str]], expected: str) -> None:
546542
result = func(Unix(appname="foo", use_site_for_root=True))
547543
assert list(result) == [expected]
548544

549545

550546
@pytest.mark.usefixtures("_as_non_root", "_no_xdg_runtime_dir", "_writable_runtime_dir")
551547
@pytest.mark.parametrize(("func", "expected"), _SINGLE_SITE_ITER_CASES)
552-
def test_iter_dirs_as_non_root_keeps_user_dir(
553-
func: Callable[[Unix], Iterator[str]],
554-
expected: str,
555-
) -> None:
548+
def test_iter_dirs_as_non_root_keeps_user_dir(func: Callable[[Unix], Iterator[str]], expected: str) -> None:
556549
result = list(func(Unix(appname="foo", use_site_for_root=True)))
557550
assert len(result) == 2
558551
assert result[0] != expected
@@ -573,7 +566,7 @@ def test_iter_dirs_as_root_with_multipath_skips_joined_user_dir(
573566
func: Callable[[Unix], Iterator[str]],
574567
) -> None:
575568
monkeypatch.setenv(xdg_var, f"/xdg/a{os.pathsep}/xdg/b")
576-
# Under multipath the user dir is the joined string, which equals no single site entry for the dedupe to drop.
569+
# Under multipath the user dir is the joined string, which no single site entry matches.
577570
assert list(func(Unix(multipath=True, use_site_for_root=True))) == ["/xdg/a", "/xdg/b"]
578571

579572

0 commit comments

Comments
 (0)