tox-dev / platformdirs

A small Python module for determining appropriate platform-specific dirs, e.g. a "user data dir".
https://platformdirs.readthedocs.io
MIT License
593 stars 51 forks source link

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

Closed SpaceshipOperations closed 9 months ago

SpaceshipOperations commented 9 months ago

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

SpaceshipOperations commented 9 months ago

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.

SpaceshipOperations commented 9 months ago

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 commented 9 months ago

I'm ok with the idea.

SpaceshipOperations commented 9 months ago

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.