Add convenience methods to `PlatformDirsAPI` that allow iterating over both user and site dirs/paths. by SpaceshipOperations · Pull Request #258 · tox-dev/platformdirs

SpaceshipOperations · 2024-01-25T01:12:21Z

Signed-off-by: Spaceship Operations spaceshipoperations@gmail.com

- `iter_config_dirs()` - `iter_config_paths()` - `iter_data_dirs()` - `iter_data_paths()` - `iter_cache_dirs()` - `iter_cache_paths()` - `iter_runtime_dirs()` - `iter_runtime_paths()` Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

for more information, see https://pre-commit.ci

gaborbernat

Tests and changelog? Use case? No issue to discuss it first?

SpaceshipOperations · 2024-01-25T01:52:43Z

Use case

Making it easier for applications to find their configuration files
in multiple places, starting with the user directory and falling back to the system directory if not present.

To be honest the only important method to me is iter_config_paths. All of the other ones are just mirroring it "for completeness' sake". I can remove any ones you do not want.

No issue to discuss it first?

The methods are simple; it takes literally a couple minutes to write them and submit a PR, so it's easier to show them first and discuss them afterwards than to discuss them before. Of course, you are free to accept or reject them.

Also, I can rename the methods if you don't like their name, or do any other modifications.

changelog

I added an entry in CHANGES.rst. Is that correct?

Tests

Sure, I can write them if you say you will accept the PR.

Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

SpaceshipOperations · 2024-01-25T02:58:38Z

Also, the initial implementation I submitted is rudimentary.

If you are okay with the idea, I will also override the Unix variants of iter_config_{dirs,paths} so they would yield each entry from XDG_CONFIG_DIRS, which rids the application of the need to manually split that string (in the case of site_*_dir) and is more XDG-compliant than just returning the first one (like {user,site}_* do by default).

Ideally iter_* should yield all paths regardless of the value of multipath, because that switch only exists because the return value of {user,site}_*_{dir,path} is expected to be a single item, a problem that the iter_* methods do not suffer from.

Would it be more suitable to keep the Unix override to a second PR, so that we're doing one thing at a time?

gaborbernat · 2024-01-25T03:06:28Z

I'm ok with the idea.

Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

This rids platform-specific classes of the need to reimplement the former if they override the latter. Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

This involves minor restructuring in the `Unix` class. A couple private helpers which contain common code for obtaining a list of config/data dirs were added, and now all of the `{user,site,iter}_{config,data}_*` methods internally use them. Consequently the private helper `_with_multi_path` became unnecessary, and was therefore removed. Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

Ruff complains about strings with `:yield:` directives not ending in a period. Added ignores for them in `pyproject.toml`. Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

SpaceshipOperations · 2024-01-25T14:21:05Z

Great to know!

I implemented the Unix overrides. The commit contains a summary of the change:

This involves minor restructuring in the Unix class.

A couple private helpers which contain common code for obtaining a list of config/data dirs were added, and now all of the {user,site,iter}_{config,data}_* methods internally use them.

Consequently the private helper _with_multi_path became unnecessary, and was therefore removed.

Are you okay with this? It does make things easier.

I have yet to add tests, but the existing coverage tests are 100% passing, although tox ends up with a failing status due to Ruff emitting a few warnings.

One of them is due to Ruff not recognizing the :yield: directive I used in docstrings, which causes it to complain about the lack of a period at the end of the line.

The second one is about "useless assignment before return". For the time being, I reformatted the offending lines, though that resulted in a long line due to having to satisfy the pre-commit hook. Would you prefer if I reverted to the old more readable "assignment then return" and silenced Ruff's warnings about it?

I also added ignores in pyproject.toml for the period warning, though I'm not confident that this is the right thing to do here.

gaborbernat

Otherwise looks alright.

pyproject.toml

Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

gaborbernat

gaborbernat

SpaceshipOperations requested review from Julian, RonnyPfannschmidt, gaborbernat and ofek as code owners January 25, 2024 01:12

[pre-commit.ci] auto fixes from pre-commit.com hooks

328c35f

for more information, see https://pre-commit.ci

gaborbernat requested changes Jan 25, 2024

View reviewed changes

Updated changelog.

4247705

Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

SpaceshipOperations added 7 commits January 25, 2024 06:40

Add docstrings to iter_*_paths() methods

d315253

Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

Made iter_*_path() methods yield from iter_*_dirs()

e94896e

This rids platform-specific classes of the need to reimplement the former if they override the latter. Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

Reorder imports to satisfy pre-commit hook

7649420

Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

Ignore Ruff warnings about docstrings

b80ac6f

Ruff complains about strings with `:yield:` directives not ending in a period. Added ignores for them in `pyproject.toml`. Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

Reformatted to avoid linter warnings

c810e63

Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

Reformatted for pre-commit hook

16d87b3

Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

gaborbernat reviewed Jan 25, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

Add periods and remove ignore for linter rule

1990159

Signed-off-by: Spaceship Operations <spaceshipoperations@gmail.com>

SpaceshipOperations force-pushed the main branch from a802cbb to 1990159 Compare January 25, 2024 23:38

gaborbernat approved these changes Jan 31, 2024

View reviewed changes

Merge branch 'main' into main

bf15a67

gaborbernat enabled auto-merge (squash) January 31, 2024 00:52

gaborbernat approved these changes Jan 31, 2024

View reviewed changes

gaborbernat disabled auto-merge January 31, 2024 00:59

gaborbernat merged commit bc4d114 into tox-dev:main Jan 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add convenience methods to `PlatformDirsAPI` that allow iterating over both user and site dirs/paths.#258

Add convenience methods to `PlatformDirsAPI` that allow iterating over both user and site dirs/paths.#258
gaborbernat merged 12 commits intotox-dev:mainfrom
SpaceshipOperations:main

SpaceshipOperations commented Jan 25, 2024

Uh oh!

gaborbernat left a comment •

edited

Loading

Uh oh!

SpaceshipOperations commented Jan 25, 2024 •

edited

Loading

Uh oh!

SpaceshipOperations commented Jan 25, 2024

Uh oh!

gaborbernat commented Jan 25, 2024

Uh oh!

SpaceshipOperations commented Jan 25, 2024 •

edited

Loading

Uh oh!

gaborbernat left a comment

Uh oh!

Uh oh!

gaborbernat left a comment

Uh oh!

gaborbernat left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

SpaceshipOperations commented Jan 25, 2024

Uh oh!

gaborbernat left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SpaceshipOperations commented Jan 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SpaceshipOperations commented Jan 25, 2024

Uh oh!

gaborbernat commented Jan 25, 2024

Uh oh!

SpaceshipOperations commented Jan 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gaborbernat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gaborbernat left a comment

Choose a reason for hiding this comment

Uh oh!

gaborbernat left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gaborbernat left a comment •

edited

Loading

SpaceshipOperations commented Jan 25, 2024 •

edited

Loading

SpaceshipOperations commented Jan 25, 2024 •

edited

Loading