-
Notifications
You must be signed in to change notification settings - Fork 45
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
Allow using trait objects with fill
& wrap
.
#219
Conversation
Hey @Cryptjar, As you note, About changes to |
98c0ffb
to
fb7b3c4
Compare
Now, since you mentioned in #215 that you pushed your rewrite & removal of the Now, instead of altering the Due to the generic impl, I also changed the primary impl of The example from my initial PR comment still works unchanged. As a side note: this new approach might sound a bit similar to #217, but instead this PR does not introduce a general (blanked) impl for every |
fb7b3c4
to
4b272cc
Compare
Hey @Cryptjar,
Thanks for the rewrite, this sounds great!
This also sounds very nice 👍 However, the changes you propose here overlap with changes proposed by @Koxiaet in #227. I believe you're both basically doing the same thing with blanket implementations for It seems strange to me — as I wrote on #227, this is similar to implementing the Perhaps we can go a step back:
I'm not 100% sure which "issue" you're referring to here... Is it #178? It's still open since I'm hoping to hear back from the author — as far as I know, it's now possible to embed an |
Well, what I meant is related to #178 since it showed the symptoms of that issue that I'm referring to. By which I meant, since there is now a trait However, if one is writing a library crate (such as it appears to me, to be the case in #178), one might want to store arbitrary user-provided wrapping parameters (aka some The problem (or issue) is precisely that as of now, once you got a trait-object (e.g.
Interestingly, since
(those are strictly speaking not blanket implementations but generic impls on the generic reference type) Rust does have the auto-deref functionality (especially in the context of the That is nice and cozy, but it does not help with a trait-object: if you have On the other hand, it does not seem that @Koxiaet cares much about trait objects in his PR, but instead his generic implementation seems to have to do with his |
Thanks, I see now!
Yeah, I agree and I will like to merge the changes there so we can remove Basically, #277 adds a impl From<usize> for Options<'static, HyphenSplitter> {
fn from(width: usize) -> Self {
Options::new(width)
}
} and changes pub fn fill<'a, S: WordSplitter, T: Into<Options<'a, S>>>(text: &str, options: T) -> String {
// ...
} On the other hand, this PR adds a impl<T: ?Sized + WrapOptions> WrapOptions for &T {
// ...
} Can this be combined with the above? This compiles, but I haven't checked if it does the right thing... impl<'a, T: ?Sized + Into<Options<'a>>> From<&T> for Options<'a> {
fn from(other: &T) -> Self {
other.into()
}
} |
This is a really good explanation, thanks! I'm learning a lot here 😄 |
I've merged #227, so the |
In order to support turning a `Options<T>` into a trait object like `Options<dyn WordSplitter>`, the requirement of `Options` for a sized `WordSplitter` has been lifted. Some unit tests and an example have been added.
4b272cc
to
a733f92
Compare
Sure. The point now is that without the However, for the library use-case I mentioned earlier, it still seems interesting to have some trait-object support such as an This now third approach is what I used to call the 'outer boxing' (i.e. |
Excellent, thanks for the extensive explanations! They'll be super helpful when I link to the PR in the release notes. |
In #219, the type parameter for `Options` was relaxed to `?Sized`, which means that `Options<T>` can be a dynamically-sized type by using `dyn WordSplitter` as the type parameter. This allows code to freely assign both boxed and unboxed `WordSplitter`s to the same variable: ```rust let mut dynamic_opt: Box<Options<dyn WordSplitter>>; dynamic_opt = Box::new(Options::with_splitter(20, NoHyphenation)); dynamic_opt = Box::new(Options::with_splitter(20, Box::new(NoHyphenation))); ``` In both cases, dynamic dispatch would be used at runtime. This was called “proper outer boxing” in #219 and #215. By only boxing the word splitter (so-called “inner boxing”), the outer layer of indirection can be removed: ```rust let mut dynamic_opt: Options<Box<dyn WordSplitter>>; dynamic_opt = Options::with_splitter(20, Box::new(NoHyphenation)); dynamic_opt = Options::with_splitter(20, Box::new(HyphenSplitter)); ``` This also used dynamic dispatch at runtime. Static dispatching was also possible by using a fixed type. Trying to change the word splitter type is a compile time error: ```rust let mut static_opt: Options<NoHyphenation>; static_opt = Options::with_splitter(10, NoHyphenation); static_opt = Options::with_splitter(20, HyphenSplitter); // <- error! ``` In order to add a trait for the `WrapAlgorithm` enum (see #325), we’re now removing the `?Sized` bound on the `WordSplitter` type parameter. This makes the first block above a compile time error and you must now choose upfront between boxed and unboxed word splitters. If you choose a boxed word splitter (second example), then you get dynamic dispatch and can freely change the word splitter at runtime. If you choose an unboxed wordsplitter, you get static dispatch and cannot change the word splitter later. This change seems necessary since we will be adding more type parameters to the `Options` struct: one for the wrapping algorithm used and one for how we find words (splitting by space or by the full Unicode line breaking algorithm). Since both dynamic and static dispatch remains possible, this change should not cause big problems for Textwrap clients.
In #219, the type parameter for `Options` was relaxed to `?Sized`, which means that `Options<T>` can be a dynamically-sized type by using `dyn WordSplitter` as the type parameter. This allows code to freely assign both boxed and unboxed `WordSplitter`s to the same variable: ```rust let mut dynamic_opt: Box<Options<dyn WordSplitter>>; dynamic_opt = Box::new(Options::with_splitter(20, NoHyphenation)); dynamic_opt = Box::new(Options::with_splitter(20, Box::new(NoHyphenation))); ``` In both cases, dynamic dispatch would be used at runtime. This was called “proper outer boxing” in #219 and #215. By only boxing the word splitter (so-called “inner boxing”), the outer layer of indirection can be removed: ```rust let mut dynamic_opt: Options<Box<dyn WordSplitter>>; dynamic_opt = Options::with_splitter(20, Box::new(NoHyphenation)); dynamic_opt = Options::with_splitter(20, Box::new(HyphenSplitter)); ``` This also used dynamic dispatch at runtime. Static dispatching was also possible by using a fixed type. Trying to change the word splitter type is a compile time error: ```rust let mut static_opt: Options<NoHyphenation>; static_opt = Options::with_splitter(10, NoHyphenation); static_opt = Options::with_splitter(20, HyphenSplitter); // <- error! ``` In order to add a trait for the `WrapAlgorithm` enum (see #325), we’re now removing the `?Sized` bound on the `WordSplitter` type parameter. This makes the first block above a compile time error and you must now choose upfront between boxed and unboxed word splitters. If you choose a boxed word splitter (second example), then you get dynamic dispatch and can freely change the word splitter at runtime. If you choose an unboxed wordsplitter, you get static dispatch and cannot change the word splitter later. This change seems necessary since we will be adding more type parameters to the `Options` struct: one for the wrapping algorithm used and one for how we find words (splitting by space or by the full Unicode line breaking algorithm). Since both dynamic and static dispatch remains possible, this change should not cause big problems for Textwrap clients.
In #219, the type parameter for `Options` was relaxed to `?Sized`, which means that `Options<T>` can be a dynamically-sized type by using `dyn WordSplitter` as the type parameter. This allows code to freely assign both boxed and unboxed `WordSplitter`s to the same variable: ```rust let mut dynamic_opt: Box<Options<dyn WordSplitter>>; dynamic_opt = Box::new(Options::with_splitter(20, NoHyphenation)); dynamic_opt = Box::new(Options::with_splitter(20, Box::new(NoHyphenation))); ``` In both cases, dynamic dispatch would be used at runtime. This was called “proper outer boxing” in #219 and #215. By only boxing the word splitter (so-called “inner boxing”), the outer layer of indirection can be removed: ```rust let mut dynamic_opt: Options<Box<dyn WordSplitter>>; dynamic_opt = Options::with_splitter(20, Box::new(NoHyphenation)); dynamic_opt = Options::with_splitter(20, Box::new(HyphenSplitter)); ``` This also used dynamic dispatch at runtime. Static dispatching was also possible by using a fixed type. Trying to change the word splitter type is a compile time error: ```rust let mut static_opt: Options<NoHyphenation>; static_opt = Options::with_splitter(10, NoHyphenation); static_opt = Options::with_splitter(20, HyphenSplitter); // <- error! ``` In order to add a trait for the `WrapAlgorithm` enum (see #325), we’re now removing the `?Sized` bound on the `WordSplitter` type parameter. This makes the first block above a compile time error and you must now choose upfront between boxed and unboxed word splitters. If you choose a boxed word splitter (second example), then you get dynamic dispatch and can freely change the word splitter at runtime. If you choose an unboxed wordsplitter, you get static dispatch and cannot change the word splitter later. This change seems necessary since we will be adding more type parameters to the `Options` struct: one for the wrapping algorithm used and one for how we find words (splitting by space or by the full Unicode line breaking algorithm). Since both dynamic and static dispatch remains possible, this change should not cause big problems for Textwrap clients.
In #219, the type parameter for `Options` was relaxed to `?Sized`, which means that `Options<T>` can be a dynamically-sized type by using `dyn WordSplitter` as the type parameter. This allows code to freely assign both boxed and unboxed `WordSplitter`s to the same variable: ```rust let mut dynamic_opt: Box<Options<dyn WordSplitter>>; dynamic_opt = Box::new(Options::with_splitter(20, NoHyphenation)); dynamic_opt = Box::new(Options::with_splitter(20, Box::new(NoHyphenation))); ``` In both cases, dynamic dispatch would be used at runtime. This was called “proper outer boxing” in #219 and #215. By only boxing the word splitter (so-called “inner boxing”), the outer layer of indirection can be removed: ```rust let mut dynamic_opt: Options<Box<dyn WordSplitter>>; dynamic_opt = Options::with_splitter(20, Box::new(NoHyphenation)); dynamic_opt = Options::with_splitter(20, Box::new(HyphenSplitter)); ``` This also used dynamic dispatch at runtime. Static dispatching was also possible by using a fixed type. Trying to change the word splitter type is a compile time error: ```rust let mut static_opt: Options<NoHyphenation>; static_opt = Options::with_splitter(10, NoHyphenation); static_opt = Options::with_splitter(20, HyphenSplitter); // <- error! ``` In order to add a trait for the `WrapAlgorithm` enum (see #325), we’re now removing the `?Sized` bound on the `WordSplitter` type parameter. This makes the first block above a compile time error and you must now choose upfront between boxed and unboxed word splitters. If you choose a boxed word splitter (second example), then you get dynamic dispatch and can freely change the word splitter at runtime. If you choose an unboxed wordsplitter, you get static dispatch and cannot change the word splitter later. This change seems necessary since we will be adding more type parameters to the `Options` struct: one for the wrapping algorithm used and one for how we find words (splitting by space or by the full Unicode line breaking algorithm). Since both dynamic and static dispatch remains possible, this change should not cause big problems for Textwrap clients.
In #219, the type parameter for `Options` was relaxed to `?Sized`, which means that `Options<T>` can be a dynamically-sized type by using `dyn WordSplitter` as the type parameter. This allows code to freely assign both boxed and unboxed `WordSplitter`s to the same variable: ```rust let mut dynamic_opt: Box<Options<dyn WordSplitter>>; dynamic_opt = Box::new(Options::with_splitter(20, NoHyphenation)); dynamic_opt = Box::new(Options::with_splitter(20, Box::new(NoHyphenation))); ``` In both cases, dynamic dispatch would be used at runtime. This was called “proper outer boxing” in #219 and #215. By only boxing the word splitter (so-called “inner boxing”), the outer layer of indirection can be removed: ```rust let mut dynamic_opt: Options<Box<dyn WordSplitter>>; dynamic_opt = Options::with_splitter(20, Box::new(NoHyphenation)); dynamic_opt = Options::with_splitter(20, Box::new(HyphenSplitter)); ``` This also used dynamic dispatch at runtime. Static dispatching was also possible by using a fixed type. Trying to change the word splitter type is a compile time error: ```rust let mut static_opt: Options<NoHyphenation>; static_opt = Options::with_splitter(10, NoHyphenation); static_opt = Options::with_splitter(20, HyphenSplitter); // <- error! ``` In order to add a trait for the `WrapAlgorithm` enum (see #325), we’re now removing the `?Sized` bound on the `WordSplitter` type parameter. This makes the first block above a compile time error and you must now choose upfront between boxed and unboxed word splitters. If you choose a boxed word splitter (second example), then you get dynamic dispatch and can freely change the word splitter at runtime. If you choose an unboxed wordsplitter, you get static dispatch and cannot change the word splitter later. This change seems necessary since we will be adding more type parameters to the `Options` struct: one for the wrapping algorithm used and one for how we find words (splitting by space or by the full Unicode line breaking algorithm). Since both dynamic and static dispatch remains possible, this change should not cause big problems for Textwrap clients.
In #219, the type parameter for `Options` was relaxed to `?Sized`, which means that `Options<T>` can be a dynamically-sized type by using `dyn WordSplitter` as the type parameter. This allows code to freely assign both boxed and unboxed `WordSplitter`s to the same variable: ```rust let mut dynamic_opt: Box<Options<dyn WordSplitter>>; dynamic_opt = Box::new(Options::with_splitter(20, NoHyphenation)); dynamic_opt = Box::new(Options::with_splitter(20, Box::new(NoHyphenation))); ``` In both cases, dynamic dispatch would be used at runtime. This was called “proper outer boxing” in #219 and #215. By only boxing the word splitter (so-called “inner boxing”), the outer layer of indirection can be removed: ```rust let mut dynamic_opt: Options<Box<dyn WordSplitter>>; dynamic_opt = Options::with_splitter(20, Box::new(NoHyphenation)); dynamic_opt = Options::with_splitter(20, Box::new(HyphenSplitter)); ``` This also used dynamic dispatch at runtime. Static dispatching was also possible by using a fixed type. Trying to change the word splitter type is a compile time error: ```rust let mut static_opt: Options<NoHyphenation>; static_opt = Options::with_splitter(10, NoHyphenation); static_opt = Options::with_splitter(20, HyphenSplitter); // <- error! ``` In order to add a trait for the `WrapAlgorithm` enum (see #325), we’re now removing the `?Sized` bound on the `WordSplitter` type parameter. This makes the first block above a compile time error and you must now choose upfront between boxed and unboxed word splitters. If you choose a boxed word splitter (second example), then you get dynamic dispatch and can freely change the word splitter at runtime. If you choose an unboxed wordsplitter, you get static dispatch and cannot change the word splitter later. This change seems necessary since we will be adding more type parameters to the `Options` struct: one for the wrapping algorithm used and one for how we find words (splitting by space or by the full Unicode line breaking algorithm). Since both dynamic and static dispatch remains possible, this change should not cause big problems for Textwrap clients.
In #219, the type parameter for `Options` was relaxed to `?Sized`, which means that `Options<T>` can be a dynamically-sized type by using `dyn WordSplitter` as the type parameter. This allows code to freely assign both boxed and unboxed `WordSplitter`s to the same variable: ```rust let mut dynamic_opt: Box<Options<dyn WordSplitter>>; dynamic_opt = Box::new(Options::with_splitter(20, NoHyphenation)); dynamic_opt = Box::new(Options::with_splitter(20, Box::new(NoHyphenation))); ``` In both cases, dynamic dispatch would be used at runtime. This was called “proper outer boxing” in #219 and #215. By only boxing the word splitter (so-called “inner boxing”), the outer layer of indirection can be removed: ```rust let mut dynamic_opt: Options<Box<dyn WordSplitter>>; dynamic_opt = Options::with_splitter(20, Box::new(NoHyphenation)); dynamic_opt = Options::with_splitter(20, Box::new(HyphenSplitter)); ``` This also used dynamic dispatch at runtime. Static dispatching was also possible by using a fixed type. Trying to change the word splitter type is a compile time error: ```rust let mut static_opt: Options<NoHyphenation>; static_opt = Options::with_splitter(10, NoHyphenation); static_opt = Options::with_splitter(20, HyphenSplitter); // <- error! ``` In order to add a trait for the `WrapAlgorithm` enum (see #325), we’re now removing the `?Sized` bound on the `WordSplitter` type parameter. This makes the first block above a compile time error and you must now choose upfront between boxed and unboxed word splitters. If you choose a boxed word splitter (second example), then you get dynamic dispatch and can freely change the word splitter at runtime. If you choose an unboxed wordsplitter, you get static dispatch and cannot change the word splitter later. This change seems necessary since we will be adding more type parameters to the `Options` struct: one for the wrapping algorithm used and one for how we find words (splitting by space or by the full Unicode line breaking algorithm). Since both dynamic and static dispatch remains possible, this change should not cause big problems for Textwrap clients.
In #219, the type parameter for `Options` was relaxed to `?Sized`, which means that `Options<T>` can be a dynamically-sized type by using `dyn WordSplitter` as the type parameter. This allows code to freely assign both boxed and unboxed `WordSplitter`s to the same variable: ```rust let mut dynamic_opt: Box<Options<dyn WordSplitter>>; dynamic_opt = Box::new(Options::with_splitter(20, NoHyphenation)); dynamic_opt = Box::new(Options::with_splitter(20, Box::new(NoHyphenation))); ``` In both cases, dynamic dispatch would be used at runtime. This was called “proper outer boxing” in #219 and #215. By only boxing the word splitter (so-called “inner boxing”), the outer layer of indirection can be removed: ```rust let mut dynamic_opt: Options<Box<dyn WordSplitter>>; dynamic_opt = Options::with_splitter(20, Box::new(NoHyphenation)); dynamic_opt = Options::with_splitter(20, Box::new(HyphenSplitter)); ``` This also used dynamic dispatch at runtime. Static dispatching was also possible by using a fixed type. Trying to change the word splitter type is a compile time error: ```rust let mut static_opt: Options<NoHyphenation>; static_opt = Options::with_splitter(10, NoHyphenation); static_opt = Options::with_splitter(20, HyphenSplitter); // <- error! ``` In order to add a trait for the `WrapAlgorithm` enum (see #325), we’re now removing the `?Sized` bound on the `WordSplitter` type parameter. This makes the first block above a compile time error and you must now choose upfront between boxed and unboxed word splitters. If you choose a boxed word splitter (second example), then you get dynamic dispatch and can freely change the word splitter at runtime. If you choose an unboxed wordsplitter, you get static dispatch and cannot change the word splitter later. This change seems necessary since we will be adding more type parameters to the `Options` struct: one for the wrapping algorithm used and one for how we find words (splitting by space or by the full Unicode line breaking algorithm). Since both dynamic and static dispatch remains possible, this change should not fundamentally change the expressive power of the library.
Considering the issue #178 with the current state of
master
, using trait objects withfill
&wrap
seems to be a straight forward solution. However, until now, it is not supported by these functions. One problem is that they take theWrapOptions
by-value, which is incompatible with trait objects.Digging a bit deeper, it seems that the
WrapIter
already uses theWrapOptions
by-value, but that trait only defines function that require access via reference. Additionally, considering that theWrapIter
is typically a rather short-lived entity, it does seem justified to give it owned access to something it only borrows. This issue is also reflected by the implementation forOptions
, which as of now, implementsWrapOptions
only on a reference (i.e.&Options
).This PR rectifies this inconsistency by giving
WrapIter
only a reference to someWrapOptions
. Now, it is no longer required that the given type is sized, and it directly allows for trait objects to be used.However, the current implementation of this PR, has the drawback that the
usize
asWrapOptions
must also be given by-reference so a simplefill("text", 42)
becomesfill("text", &42)
, it shouldn't cause any issues, but the additional ampersand (&
) isn't the prettiest.As shown by a newly added test the following idiom using trait objects becomes now possible due to this PR:
This is a follow-up to my early comment, it implements what I called 'proper outer boxing' #215 (comment)