Skip to content

Add functions to return standard directories #1822

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

Merged
merged 7 commits into from
Jan 11, 2024

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Jan 6, 2024

Add the following builtin functions:

  • cache_directory()
  • config_directory()
  • config_local_directory()
  • data_directory()
  • data_local_directory()
  • executable_directory()
  • home_directory()

These use the dirs crate to return XDG-compliant paths, or the system defaults.

Fixes: #1820

@laniakea64
Copy link
Contributor

Would it be better to use dirs::preference_dir for config_directory()?

@tgross35
Copy link
Contributor Author

tgross35 commented Jan 6, 2024

Would it be better to use dirs::preference_dir for config_directory()?

I think it would be better to keep names consistent with what they are on dirs, which could mean adding preference_directory. But the only difference between the two is on mac - /Users/Alice/Library/Application Support(config_dir) contains a directory with the application name and then whatever contents is needed, pretty in line with other platforms.

/Users/Alice/Library/Preferences (preference_dir) only contains com.*.*.plist files, which are a something like a binary JSON. I think less likely to be used than the config directory where you can just place files.

Copy link
Owner

@casey casey left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks so much for the docs and tests, a lot of PRs miss those. See review for comments.

@tgross35 tgross35 force-pushed the add-directory-functions branch from b591084 to 0e132e4 Compare January 6, 2024 22:55
tgross35 and others added 7 commits January 11, 2024 15:47
Add the following builtin functions:

- `cache_directory()`
- `config_directory()`
- `config_local_directory()`
- `data_directory()`
- `data_local_directory()`
- `executable_directory()`
- `home_directory()`

These use the `dirs` crate to return XDG-compliant paths, or the system
defaults.

Fixes: casey#1820
@casey casey force-pushed the add-directory-functions branch from ddfb64a to 5910eb9 Compare January 11, 2024 23:47
@tgross35
Copy link
Contributor Author

I see you changed this to return an error if the directory is not found (thanks for the updates!). Would it be better to make these accept either 0 or 1 argument, so a fallback can be provided if not available?

@tgross35
Copy link
Contributor Author

Or looks like it's ready to merge. If the above sounds reasonable, I can submit a followup PR :)

@casey
Copy link
Owner

casey commented Jan 11, 2024

Nice! I tweaked the readme a little, and changed the functions to return an error if the directory isn't found. Also, for some reason, on my mac the executable directory wasn't found, so I modified the test to handle this error.

@casey casey enabled auto-merge (squash) January 11, 2024 23:49
@casey casey merged commit c2af5a1 into casey:master Jan 11, 2024
@casey
Copy link
Owner

casey commented Jan 11, 2024

I see you changed this to return an error if the directory is not found (thanks for the updates!). Would it be better to make these accept either 0 or 1 argument, so a fallback can be provided if not available?

Whoops, I just noticed this comment. If people want it, I think it would be fine to modify these functions to accept a fallback which is returned if the directory isn't found. We could either wait and see if anyone complains, or if you feel like it, and do it in a follow up PR.

@tgross35
Copy link
Contributor Author

Sounds great. Thanks!

@tgross35 tgross35 deleted the add-directory-functions branch January 11, 2024 23:54
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.

Feature request: functions to get XDG base directories (home, config, etc)
3 participants