app-dirs-rs
app-dirs-rs copied to clipboard
Make `AppInfo` own its strings.
Some of us want to make a library that uses app_dirs
to manage data but lets the user of the library specify the strings for the application using it. Forcing this to be a &'static str
instead of a String
is a pain and also seems to not really gain us anything.
Ah, I hadn't even thought of this use case! Great point. I'll fix this soon, or feel free to open a PR if you like - it should be a pretty simple change.
Wait, I remember why I did this now. You can't create a thread-safe, read-only const
/static
instance with String
, but you can with &'static str
. I really didn't want to require folks to pass around the AppInfo
instance or have to pull in lazy_static
as a dependency...
I wonder how we can solve this issue and yours? I think they are both valid.
It appears that using std::borrow::Cow
makes it possible, though it's not terribly ergonomic and I'm still not 100% sure how it interacts with threading. See https://github.com/icefoxen/app-dirs-rs/commit/52c69a945db05e7568c9c895d8d46d689d7815db The canonical next step seems like it would be "add a macro to make it a bit prettier" but it seems like overkill...
Ooh, I bet I could use a simple trait with two methods (for author and app name) that return String
, and provide the default struct as an implementation with &'static str
fields that return String
copies. So we can keep the ergonomic default implementation while allowing libs like yours to do different things :)
That would be wonderful. <3
Ok, trying to implement a PR for this and I've realized I don't understand exactly what you mean. Something like this?
trait AppInfoT {
fn name(&self) -> String;
fn author(&self) -> String;
}
impl AppInfoT for AppInfo {
fn name(&self) -> String {
self.name.to_string()
}
fn author(&self) -> String {
self.author.to_string()
}
}
More like this:
trait AppInfo {
fn name(&self) -> &str;
fn author(&self) -> &str;
}
struct StaticAppInfo {
name: &'static str,
author: &'static str,
}
struct DynamicAppInfo {
name: String,
author: String,
}
impl AppInfo for StaticAppInfo {
fn name(&self) -> &str {
self.name
}
fn author(&self) -> &str {
self.author
}
}
impl AppInfo for DynamicAppInfo {
fn name(&self) -> &str {
&self.name
}
fn author(&self) -> &str {
&self.author
}
}
Returning str
instead of String
avoids unnecessary cloning/allocation.
Finally got around to implementing this in https://github.com/icefoxen/app-dirs-rs/commit/71ba239f14ac62bda59ec3c03ac6f5beb50e7890 . If you like it I'll update the docs and make a PR. It breaks the API, which is annoying, and I don't entirely like the StaticAppInfo
name, but I messed around with other ways of doing the same thing in https://github.com/icefoxen/app-dirs-rs/commit/09a244038821451ce8757ee89ac7144a717361f4 and I'm not sure any of them are better.
Hello,
so I've made a change, before checking that there was an issue about this, please see #22 I've also added a comment about #22 vs #23 in there.