-
Notifications
You must be signed in to change notification settings - Fork 12
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
add Traits? #2
Comments
I don't think that adding traits is a good idea, since it is not really an idiomatic approach. Libraries like this one usually provide top-level functions, which are actually easier to use shellexpand::StringExpandExt;
"a".expand_full(); vs use shellexpand;
shellexpand::full("a"); IDEs will also suggest all of the function names in the second case; they also can usually suggest added trait methods in the first case, but it is less reliable and requires the user to import the trait explicitly. The latter can probably be mitigated by having some special module for glob imports, e.g. As for the thing about documentation, yes, I can see what you mean. I wrote documentation using the "bottom-up" approach, with high-level functions described as emergent from more general functions, but I think that indeed mentioning the most common ones in the beginning would be a good thing. I'll fix the docs as soon as I'm able to. Thanks! |
Sorry I forgot to take back the stuff about the traits - on further thought I think you are absolutely right! The doc changes would be great. Thanks again for the awesome lib! |
Hi. As discussed in #17 I am taking over maintenance of this crate. The new repository is here: https://gitlab.com/ijackson/rust-shellexpand I think that an extension trait, as proposed, would be a nice addition. I haven't filed an issue in my new repo about this, but I would welcome an MR. |
I personally took back my thoughts on a trait, but feel free to implement/accept if you still think it's valuable :D |
Hey, I love the library!
I was thinking adding a trait which is implemented for
str
would be useful. The end result would be:"~/foo/bar".expand()
: Callsshellexpand::full
on the str."~/foo/bar".expand_tilde()
: callsshellexpand::tilde
on the str.I fell like these are the two most commonly needed methods and would improve the eronomics of this library.
I feel the docs could also be improved to suggest these two functions + trait at the very beginning. Right now
shellexpand::full
isn't mentioned until the 4th paragraph when most users of this library will be looking for eitherfull
ortilde
and a brief overview of what they do.The text was updated successfully, but these errors were encountered: