Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
Jan 31, 2024

Conversation

SpaceshipOperations
Copy link
Contributor

Signed-off-by: Spaceship Operations [email protected]

- `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 <[email protected]>
Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@SpaceshipOperations
Copy link
Contributor Author

SpaceshipOperations commented Jan 25, 2024

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 <[email protected]>
@SpaceshipOperations
Copy link
Contributor Author

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
Copy link
Member

I'm ok with the idea.

Signed-off-by: Spaceship Operations <[email protected]>
This rids platform-specific classes of the need to reimplement
the former if they override the latter.

Signed-off-by: Spaceship Operations <[email protected]>
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 <[email protected]>
Ruff complains about strings with `:yield:` directives not ending
in a period. Added ignores for them in `pyproject.toml`.

Signed-off-by: Spaceship Operations <[email protected]>
Signed-off-by: Spaceship Operations <[email protected]>
Signed-off-by: Spaceship Operations <[email protected]>
@SpaceshipOperations
Copy link
Contributor Author

SpaceshipOperations commented Jan 25, 2024

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.

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise looks alright.

pyproject.toml Outdated Show resolved Hide resolved
Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaborbernat gaborbernat enabled auto-merge (squash) January 31, 2024 00:52
Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaborbernat gaborbernat disabled auto-merge January 31, 2024 00:59
@gaborbernat gaborbernat merged commit bc4d114 into tox-dev:main Jan 31, 2024
29 checks passed
alicejli referenced this pull request in googleapis/sdk-platform-java May 3, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [platformdirs](https://togithub.com/platformdirs/platformdirs) |
`==4.1.0` -> `==4.2.1` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/platformdirs/4.2.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/platformdirs/4.2.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/platformdirs/4.1.0/4.2.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/platformdirs/4.1.0/4.2.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>platformdirs/platformdirs (platformdirs)</summary>

###
[`v4.2.1`](https://togithub.com/platformdirs/platformdirs/releases/tag/4.2.1)

[Compare
Source](https://togithub.com/platformdirs/platformdirs/compare/4.2.0...4.2.1)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

- Switch to ruff for formatting and use codespell and docformatter by
[@&#8203;gaborbernat](https://togithub.com/gaborbernat) in
[https://github.com/platformdirs/platformdirs/pull/261](https://togithub.com/platformdirs/platformdirs/pull/261)
- Use hatch over tox by
[@&#8203;gaborbernat](https://togithub.com/gaborbernat) in
[https://github.com/platformdirs/platformdirs/pull/262](https://togithub.com/platformdirs/platformdirs/pull/262)
- chore: various minor fixes by
[@&#8203;deronnax](https://togithub.com/deronnax) in
[https://github.com/platformdirs/platformdirs/pull/263](https://togithub.com/platformdirs/platformdirs/pull/263)
- chore: update dead Microsoft's known folders documentation link by
[@&#8203;deronnax](https://togithub.com/deronnax) in
[https://github.com/platformdirs/platformdirs/pull/267](https://togithub.com/platformdirs/platformdirs/pull/267)
- Allow working without ctypes by
[@&#8203;youknowone](https://togithub.com/youknowone) in
[https://github.com/platformdirs/platformdirs/pull/275](https://togithub.com/platformdirs/platformdirs/pull/275)

#### New Contributors

- [@&#8203;deronnax](https://togithub.com/deronnax) made their first
contribution in
[https://github.com/platformdirs/platformdirs/pull/263](https://togithub.com/platformdirs/platformdirs/pull/263)
- [@&#8203;youknowone](https://togithub.com/youknowone) made their first
contribution in
[https://github.com/platformdirs/platformdirs/pull/275](https://togithub.com/platformdirs/platformdirs/pull/275)

**Full Changelog**:
tox-dev/platformdirs@4.2.0...4.2.1

###
[`v4.2.0`](https://togithub.com/platformdirs/platformdirs/releases/tag/4.2.0)

[Compare
Source](https://togithub.com/platformdirs/platformdirs/compare/4.1.0...4.2.0)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

- Fix 2 typos about XDG_DATA_DIR by
[@&#8203;Freed-Wu](https://togithub.com/Freed-Wu) in
[https://github.com/platformdirs/platformdirs/pull/256](https://togithub.com/platformdirs/platformdirs/pull/256)
- Add convenience methods to `PlatformDirsAPI` that allow iterating over
both user and site dirs/paths. by
[@&#8203;SpaceshipOperations](https://togithub.com/SpaceshipOperations)
in
[https://github.com/platformdirs/platformdirs/pull/258](https://togithub.com/platformdirs/platformdirs/pull/258)

#### New Contributors

-
[@&#8203;SpaceshipOperations](https://togithub.com/SpaceshipOperations)
made their first contribution in
[https://github.com/platformdirs/platformdirs/pull/258](https://togithub.com/platformdirs/platformdirs/pull/258)

**Full Changelog**:
tox-dev/platformdirs@4.1.0...4.2.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/googleapis/sdk-platform-java).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMDEuNCIsInVwZGF0ZWRJblZlciI6IjM3LjMxMy4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: Alice <[email protected]>
lqiu96 referenced this pull request in googleapis/sdk-platform-java May 22, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [platformdirs](https://togithub.com/platformdirs/platformdirs) |
`==4.1.0` -> `==4.2.1` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/platformdirs/4.2.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/platformdirs/4.2.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/platformdirs/4.1.0/4.2.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/platformdirs/4.1.0/4.2.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>platformdirs/platformdirs (platformdirs)</summary>

###
[`v4.2.1`](https://togithub.com/platformdirs/platformdirs/releases/tag/4.2.1)

[Compare
Source](https://togithub.com/platformdirs/platformdirs/compare/4.2.0...4.2.1)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

- Switch to ruff for formatting and use codespell and docformatter by
[@&#8203;gaborbernat](https://togithub.com/gaborbernat) in
[https://github.com/platformdirs/platformdirs/pull/261](https://togithub.com/platformdirs/platformdirs/pull/261)
- Use hatch over tox by
[@&#8203;gaborbernat](https://togithub.com/gaborbernat) in
[https://github.com/platformdirs/platformdirs/pull/262](https://togithub.com/platformdirs/platformdirs/pull/262)
- chore: various minor fixes by
[@&#8203;deronnax](https://togithub.com/deronnax) in
[https://github.com/platformdirs/platformdirs/pull/263](https://togithub.com/platformdirs/platformdirs/pull/263)
- chore: update dead Microsoft's known folders documentation link by
[@&#8203;deronnax](https://togithub.com/deronnax) in
[https://github.com/platformdirs/platformdirs/pull/267](https://togithub.com/platformdirs/platformdirs/pull/267)
- Allow working without ctypes by
[@&#8203;youknowone](https://togithub.com/youknowone) in
[https://github.com/platformdirs/platformdirs/pull/275](https://togithub.com/platformdirs/platformdirs/pull/275)

#### New Contributors

- [@&#8203;deronnax](https://togithub.com/deronnax) made their first
contribution in
[https://github.com/platformdirs/platformdirs/pull/263](https://togithub.com/platformdirs/platformdirs/pull/263)
- [@&#8203;youknowone](https://togithub.com/youknowone) made their first
contribution in
[https://github.com/platformdirs/platformdirs/pull/275](https://togithub.com/platformdirs/platformdirs/pull/275)

**Full Changelog**:
tox-dev/platformdirs@4.2.0...4.2.1

###
[`v4.2.0`](https://togithub.com/platformdirs/platformdirs/releases/tag/4.2.0)

[Compare
Source](https://togithub.com/platformdirs/platformdirs/compare/4.1.0...4.2.0)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

- Fix 2 typos about XDG_DATA_DIR by
[@&#8203;Freed-Wu](https://togithub.com/Freed-Wu) in
[https://github.com/platformdirs/platformdirs/pull/256](https://togithub.com/platformdirs/platformdirs/pull/256)
- Add convenience methods to `PlatformDirsAPI` that allow iterating over
both user and site dirs/paths. by
[@&#8203;SpaceshipOperations](https://togithub.com/SpaceshipOperations)
in
[https://github.com/platformdirs/platformdirs/pull/258](https://togithub.com/platformdirs/platformdirs/pull/258)

#### New Contributors

-
[@&#8203;SpaceshipOperations](https://togithub.com/SpaceshipOperations)
made their first contribution in
[https://github.com/platformdirs/platformdirs/pull/258](https://togithub.com/platformdirs/platformdirs/pull/258)

**Full Changelog**:
tox-dev/platformdirs@4.1.0...4.2.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/googleapis/sdk-platform-java).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMDEuNCIsInVwZGF0ZWRJblZlciI6IjM3LjMxMy4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: Alice <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants