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.

Sign up for GitHub

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

Use XDG dirs per spec #2135

Closed
wants to merge 8 commits into from
Closed

Use XDG dirs per spec #2135

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d9ba5f8
expose only select paths from loader
mmatous Apr 13, 2022
3c17e68
drop etcetera in favor of directories
mmatous Apr 13, 2022
78e756d
allow setting config location via HELIX_CONFIG
mmatous Apr 13, 2022
1d46f55
support setting paths from config file
mmatous Apr 13, 2022
ea4bae0
reorganize project directory structure
mmatous Apr 13, 2022
8f5813a
update docs
mmatous Apr 13, 2022
5683e80
Switch to updated etcetera
mmatous May 11, 2022
020182f
Fix docs/impl disparity
mmatous May 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion helix-loader/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ homepage = "https://helix-editor.com"
anyhow = "1"
serde = { version = "1.0", features = ["derive"] }
toml = "0.5"
etcetera = "0.3"
directories = "4"
Copy link
Member

Choose a reason for hiding this comment

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

❌ etcetera was specifically chosen because it uses XDG on macOS instead of system directories. For a command line tool it makes more sense to use XDG.

Copy link
Member

Choose a reason for hiding this comment

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

directories/dirs is also deprecated in favour of dirs-next (used by etcetera) https://www.reddit.com/r/rust/comments/pps4f0/directories_crate_version_400_broken_on_macos/

Copy link
Author

Choose a reason for hiding this comment

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

What do you mean by system directories? Going by directories' README and etcetera code, all paths used on macOS begin with $HOME/Library. Both crates behave the same way.

directories crate isn't deprecated. It has more recent release than the *-next forks or etcetera and it's the only cross-platform crate I found that bothered to incorporate XDG_STATE_HOME. The Reddit link says nothing about deprecation either, just random people recommending directories-next, one without explanation and other saying "might be good alternative". It may have been at the time but now it's *-next that's outdated.

I'm open to using whatever, but etcetera and directories-next aren't up-to-date and don't do what's required.

Copy link
Member

Choose a reason for hiding this comment

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

This is one of the defining features of etcetera: https://docs.rs/etcetera/latest/etcetera/base_strategy/fn.choose_base_strategy.html

Meanwhile it's not implemented in directories-next: xdg-rs/dirs#45

directories had a minor release last year but it's unmaintained. Most crates switched over to directories-next after original author stopped responding: https://www.reddit.com/r/rust/comments/gli4pc/directoriesnext_dirsnext_the_new_home_for/

Copy link
Author

@mmatous mmatous Apr 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Got it, I only looked at the Apple strategy. I created a PR lunacookies/etcetera#1 to support state. Etcetera dev seems fairly active so hopefully it'll be merged soon.

tree-sitter = "0.20"
libloading = "0.7"
once_cell = "1.9"
Expand Down
19 changes: 7 additions & 12 deletions helix-loader/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
pub mod grammar;

use etcetera::base_strategy::{choose_base_strategy, BaseStrategy};

pub static RUNTIME_DIR: once_cell::sync::Lazy<std::path::PathBuf> =
once_cell::sync::Lazy::new(runtime_dir);

fn project_dirs() -> directories::ProjectDirs {
directories::ProjectDirs::from("com", "helix-editor", "helix")
.expect("Unable to continue. User has no home")
}

pub fn runtime_dir() -> std::path::PathBuf {
if let Ok(dir) = std::env::var("HELIX_RUNTIME") {
return dir.into();
Expand All @@ -29,19 +32,11 @@ pub fn runtime_dir() -> std::path::PathBuf {
}

fn config_dir() -> std::path::PathBuf {
// TODO: allow env var override
let strategy = choose_base_strategy().expect("Unable to find the config directory!");
let mut path = strategy.config_dir();
path.push("helix");
path
project_dirs().config_dir().to_path_buf()
}

fn cache_dir() -> std::path::PathBuf {
// TODO: allow env var override
let strategy = choose_base_strategy().expect("Unable to find the config directory!");
let mut path = strategy.cache_dir();
path.push("helix");
path
project_dirs().cache_dir().to_path_buf()
}

pub fn config_file() -> std::path::PathBuf {
Expand Down