Separate traits into separate string-traits crate?

[daboross]

Hi,

Wondering if you'd consider making or accepting a PR which separates out StringExt into a separate crate.

This would be useful for possibly having generic operations of the std String, this crate's InlineableString, and other crate's always-inline and/or sometimes-inline string types.

The trait being in this crate is alright, but it's no where near as useful as it could be if it were implementation-independent.

[ArtemGr]

You have a particular alternative implementation of that trait in mind?

[daboross]

I'm just thinking it would be nice to let other library authors abstract over all of the different string-like types without caring what a user actually uses.

Types I would ideally want to implement this trait: std::string::String, inlinable_string::InlineableString, easy_strings::EZString, string_wrapper::StringWrapper, string::String, immut_string::ImmutString, string_intern::Symbol, string_cache::Atom, istring::IString, arrayvec::ArrayString, and smallstring::SmallString.

I don't use the majority of these crates, but I also don't know who does, and supporting all would be better than supporting only one (or none of them). Granted, some of these are very similar to inlinable_string with different design decisions (like a different size rather than 30 to inline), but that doesn't mean people won't use them over inlinable_string. There are also other use cases here besides just stack allocated strings, like cached/interned strings or auto reference counted strings.

Libraries which operate directly on owned strings and mutate them in place would benefit most from this. I'm singling this use case out because for most others, using AsRef<str> as the required bound would be much simpler.

[ArtemGr]

Sounds good!

I think I'll accept such a PR, unless @fitzgen has any objections.

We'll probably need a major version bump.

P.S. If it's a simple enough change of using the external trait instead of our own, I might be able to work on it here.

What do you think of leaving an option to use the internal trait?

How about using the internal trait by default (not sure if using inlineable_string and some other alternative string is the most usual use case)?

Should we somehow re-export the trait if we're using the external one? (I'm worried about the "dependency hell" in that case).

[daboross]

Cool!

My idea of the change would just be to pull the exact current trait out into a separate crate, then pub use it from within inlinable_string. I don't believe this would be a breaking change, since the public interface inlinable_string exposes would remain exactly the same.

As for having an internal trait as an option, I guess that could work? It seems like this would just mean more code duplication, though, if we do want to pull it out into a separate crate as well. Since it doesn't mean any API changes for inlinable_string, I imagine anyone could use it in the exact same manner, and there would be no need for an internal trait option.

If we want, we could even use #[doc(inline)] to keep the documentation for the trait in inlinable_string the same (not linked to the other crate).

I mostly just want to be able to depend on the trait without depending on inlinable_string- but if there's more reason to, we could separate them further?

[ArtemGr]

I don't believe this would be a breaking change, since the public interface inlinable_string exposes would remain exactly the same.

From https://stackoverflow.com/questions/41185023/what-exactly-is-considered-a-breaking-change-to-a-library-crate I think the following items might match:

"Renaming, moving or removing any public item in a module." (We're removing a public trait, since it will live in the outer crate now). P.S. Though the addendum in https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md#major-change-renamingmovingremoving-any-public-items says that pub use is a valid way to move stuff without introducing a breaking change.

"Altering the use of Cargo features on a crate." (Might need to use cargo features). https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md#minor-change-altering-the-use-of-cargo-features

"Implementing any non-fundamental trait on an existing type." (Again, the internal trait and the external trait are different traits, with different namespace paths. Rust won't duck type them to be the same. So we're effectively removing one trait and implementing another one. Yes, the code will still compile but I'd rather be safe than sorry).

If we want, we could even use #[doc(inline)] to keep the documentation for the trait in inlinable_string the same (not linked to the other crate).

Good to know.

[daboross]

"Renaming, moving or removing any public item in a module." (We're removing a public trait, since it will live in the outer crate).

I guess my thought was just that nothing public would be renamed or moved. It'd be exposed exactly under the same name, only the source location would have to change.

I think the 'adding or removing a feature' one is for adding a requirement on a dependency's feature, not really adding a new feature to Cargo.toml. It's a bit ambiguous, but how they talk about "a common dependency" makes it seem like this isn't about adding features to the crate itself?

If you think a major version change would be better, that's fully alright. I just didn't want to introduce one if not necessary.

I'll submit an initial PR in the next few days and we can possibly go from there?

[ArtemGr]

Sure.

[ArtemGr]

Closing as inactive.

[daboross]

Reasonable. I'm still planning on getting to this eventually but have had too many other things in real life at the moment.

[ArtemGr]

Yep. Focus on what's important. Feel free to reopen when needed.