Skip to content

Add XDG Support to confy #119

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "confy"
version = "1.0.0"
version = "2.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

I’m not sure we have a change-log file, but writing somewhere what made change the major version can help peoples migrate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

100% once we work out the API I am going to add into here: https://github.com/rust-cli/confy/?tab=readme-ov-file#breaking-changes

authors = ["Katharina Fey <[email protected]>"]
description = "Boilerplate-free configuration management"
license = "MIT/X11 OR Apache-2.0"
Expand All @@ -11,12 +11,12 @@ edition = "2024"

[dependencies]
ron = { version = "0.10.1", optional = true }
directories = "6"
etcetera = "0.10.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a link to https://docs.rs/etcetera/latest/etcetera/#native-strategy (or the discussion we had here) to let peoples know how to configure confy? At least some documentation (or example/test) around it to show can be useful.

serde = "^1.0"
serde_yaml = { version = "0.9", optional = true }
thiserror = "2.0"
basic-toml = { version = "0.1.10", optional = true }
toml = { version = "0.8", optional = true }
toml = { version = "0.9", optional = true }

[features]
default = ["toml_conf"]
Expand Down
5 changes: 2 additions & 3 deletions examples/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn main() -> Result<(), confy::ConfyError> {
name: "Test".to_string(),
..cfg
};
confy::store("confy_simple_app",None, &cfg)?;
confy::store("confy_simple_app", None, &cfg)?;
println!("The updated toml file content is:");
let mut content = String::new();
std::fs::File::open(&file)
Expand All @@ -53,7 +53,6 @@ fn main() -> Result<(), confy::ConfyError> {
name: "Test".to_string(),
..cfg
};
std::fs::remove_dir_all(file.parent().unwrap())
.expect("Failed to remove directory");
std::fs::remove_dir_all(file.parent().unwrap()).expect("Failed to remove directory");
Ok(())
}
39 changes: 22 additions & 17 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@
//!
//! ### Tip
//! to add this crate to your project with the default, toml config do the following: `cargo add confy`, otherwise do something like: `cargo add confy --no-default-features --features yaml_conf`, for more info, see [cargo docs on features]
//!
//!
//! [cargo docs on features]: https://docs.rust-lang.org/cargo/reference/resolver.html#features
//!
//!
//! feature | file format | description
//! ------- | ----------- | -----------
//! **default**: `toml_conf` | [toml] | considered a reasonable default, uses the standard-compliant [`toml` crate]
Expand All @@ -94,8 +94,8 @@
mod utils;
use utils::*;

use directories::ProjectDirs;
use serde::{de::DeserializeOwned, Serialize};
use etcetera::{AppStrategy, AppStrategyArgs, app_strategy::choose_app_strategy};
use serde::{Serialize, de::DeserializeOwned};
use std::fs::{self, File, OpenOptions, Permissions};
use std::io::{ErrorKind::NotFound, Write};
use std::path::{Path, PathBuf};
Expand All @@ -109,8 +109,8 @@ use toml::{

#[cfg(feature = "basic_toml_conf")]
use basic_toml::{
from_str as toml_from_str, to_string as toml_to_string_pretty, Error as TomlDeErr,
Error as TomlSerErr,
Error as TomlDeErr, Error as TomlSerErr, from_str as toml_from_str,
to_string as toml_to_string_pretty,
};

#[cfg(not(any(
Expand Down Expand Up @@ -469,23 +469,26 @@ pub fn get_configuration_file_path<'a>(
config_name: impl Into<Option<&'a str>>,
) -> Result<PathBuf, ConfyError> {
let config_name = config_name.into().unwrap_or("default-config");
let project = ProjectDirs::from("rs", "", app_name).ok_or_else(|| {
ConfyError::BadConfigDirectory("could not determine home directory path".to_string())
let project = choose_app_strategy(AppStrategyArgs {
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I just understand now that you are "forcing" the app strategy? Not configurable then? (Still needs to document this.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I think you're right we should allow this to be configurable for GUI applications, I'll update!

top_level_domain: "rs".to_string(),
author: "".to_string(),
app_name: app_name.to_string(),
})
.map_err(|e| {
ConfyError::BadConfigDirectory(format!("could not determine home directory path: {e}"))
})?;

let config_dir_str = get_configuration_directory_str(&project)?;

let path = [config_dir_str, &format!("{config_name}.{EXTENSION}")]
let path = [config_dir_str, format!("{config_name}.{EXTENSION}")]
.iter()
.collect();

Ok(path)
}

fn get_configuration_directory_str(project: &ProjectDirs) -> Result<&str, ConfyError> {
let path = project.config_dir();
path.to_str()
.ok_or_else(|| ConfyError::BadConfigDirectory(format!("{path:?} is not valid Unicode")))
fn get_configuration_directory_str(project: &impl AppStrategy) -> Result<String, ConfyError> {
Ok(project.config_dir().display().to_string())
Comment on lines +490 to +491
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fn get_configuration_directory_str(project: &impl AppStrategy) -> Result<String, ConfyError> {
Ok(project.config_dir().display().to_string())
fn get_configuration_directory_str(project: &impl AppStrategy) -> Result<PathBuf, ConfyError> {
Ok(project.config_dir())

I know it is a private function, but maybe it is better to use the PathBuf directly instead of relying on the String approximation?
Especially since you want to do a major version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I think the function is used only once… it was a nice helper but may be useless now?

}

#[cfg(test)]
Expand Down Expand Up @@ -601,10 +604,12 @@ mod tests {

store_path_perms(path, &config, permissions).expect("store_path_perms failed");

assert!(fs::metadata(path)
.expect("reading metadata failed")
.permissions()
.readonly());
assert!(
fs::metadata(path)
.expect("reading metadata failed")
.permissions()
.readonly()
);
})
}

Expand Down
Loading