netvl/shellexpand

add Traits?

vitiral opened this issue · 4 comments

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(): Calls shellexpand::full on the str.
  • "~/foo/bar".expand_tilde(): calls shellexpand::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 either full or tilde and a brief overview of what they do.

netvl commented

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 than traits:

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. use shellexpand::prelude::* or use shellexpand::traits::*, but it so much of complexity for such a simple library I don't think it is worth potential syntactic improvements

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