Skip to content
This repository was archived by the owner on Apr 5, 2021. It is now read-only.

Conversation

TruputiGalvotas
Copy link

Hi,

there are some use cases (for example using clap and app_dirs in the same app) when you'd want to create your object which you could pass to app_dirs functions. This pull request implements that ability via trait.

I've create AppMeta trait and replaced all function declarations to accept any type that implements this trait. I've also added AppMeta implementations for AppInfo struct, so nothing really changes for the users I think.

Let me know what you think!

TruputiGalvotas and others added 2 commits April 11, 2017 18:53
This replaces usage of AppInfo struct with usage of AppMeta trait in all function parameters. Also Adds the AppMeta trait itself and implementations of methods to AppInfo
@TruputiGalvotas
Copy link
Author

I actually didn't check before doing this that there is #19 and now #23 which basically fixes it. I think, that my version doesn't brake existing library users, since the app info struct is left with the same name and in #23 it has a different name.. In my opinion, I would pull in this request (#22) and rework #23 in the following way:

struct AppInfoDynamic {
//
}
impl AppMeta for AppInfoDynamic {
//
}

This way, you would not brake existing users, because AppInfo would still be the same struct, and #19 would be solved by just adding a different struct which owns the strings..

Added OwnedAppInfo struct that owns it's strings
@TruputiGalvotas
Copy link
Author

Based on #23 I've added OwnedAppInfo struct, so if this is going to be merged it would require no additional work.

Renamed AppInfo to StaticAppInfo, added type declaration for AppInfo and marked as deprecated
@TruputiGalvotas
Copy link
Author

Thought that if AppInfo should be renamed to StaticAppInfo as suggested on #19 , implemented that and added type declaration for AppInfo, so it would not brake existing users.
Bumped version to 1.2.0, since I think such change is quite important for a version change.
Added deprecation declaration, so that existing users would see a warning that AppInfo is deprecated and should use StaticAppInfo instead.

@qmx
Copy link

qmx commented Jun 11, 2018

This is exactly what I need - we could potentially derive this trait impl from Cargo.toml info if this crate goes this way...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants