Make `AppInfo` own its strings. about app-dirs-rs HOT 9 OPEN

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.

from app-dirs-rs.

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.

from app-dirs-rs.

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 icefoxen@52c69a9 The canonical next step seems like it would be "add a macro to make it a bit prettier" but it seems like overkill...

from app-dirs-rs.

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 :)

from app-dirs-rs.

That would be wonderful. <3

from app-dirs-rs.

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()
    }
}

from app-dirs-rs.

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.

from app-dirs-rs.

Finally got around to implementing this in icefoxen@71ba239 . 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 icefoxen@09a2440 and I'm not sure any of them are better.

from app-dirs-rs.

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.

from app-dirs-rs.