Remove the “outer boxing” support for Options

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.

Cargo.toml

@@ -24,11 +24,16 @@ path = "benches/linear.rs"
 default = ["unicode-width", "smawk"]
 
 [dependencies]
-hyphenation = { version = "0.8", optional = true, features = ["embed_en-us"] }
 smawk = { version = "0.3", optional = true }
 terminal_size = { version = "0.1", optional = true }
 unicode-width = { version= "0.1", optional = true }
 
+[dependencies.hyphenation]
+git = "https://github.com/tapeinosyne/hyphenation"
+rev = "d8d501a3731d"  # Until `Standard` implements `Clone`
+optional = true
+features = ["embed_en-us"]
+
 [dev-dependencies]
 criterion = "0.3"
 lipsum = "0.7"

src/core.rs

@@ -858,7 +858,7 @@ mod tests {
 
     #[test]
     fn split_words_adds_penalty() {
-        #[derive(Debug)]
+        #[derive(Clone, Debug)]
         struct FixedSplitPoint;
         impl WordSplitter for FixedSplitPoint {
             fn split_points(&self, _: &str) -> Vec<usize> {

src/lib.rs

@@ -184,7 +184,7 @@ pub mod core;
 
 /// Holds settings for wrapping and filling text.
 #[derive(Debug, Clone)]
-pub struct Options<'a, S: ?Sized = Box<dyn WordSplitter>> {
+pub struct Options<'a, S = Box<dyn WordSplitter>> {
     /// The width in columns at which the text will be wrapped.
     pub width: usize,
     /// Indentation used for the first line of output. See the
@@ -207,15 +207,15 @@ pub struct Options<'a, S: ?Sized = Box<dyn WordSplitter>> {
     pub splitter: S,
 }
 
-impl<'a, S: ?Sized> From<&'a Options<'a, S>> for Options<'a, &'a S> {
+impl<'a, S: Clone> From<&'a Options<'a, S>> for Options<'a, S> {
     fn from(options: &'a Options<'a, S>) -> Self {
         Self {
             width: options.width,
             initial_indent: options.initial_indent,
             subsequent_indent: options.subsequent_indent,
             break_words: options.break_words,
             wrap_algorithm: options.wrap_algorithm,
-            splitter: &options.splitter,
+            splitter: options.splitter.clone(),
         }
     }
 }
@@ -1836,74 +1836,40 @@ mod tests {
         assert_eq!(unfill("foo   bar").0, "foo   bar");
     }
 
-    #[test]
-    fn trait_object() {
-        let opt_a: Options<NoHyphenation> = Options::with_splitter(20, NoHyphenation);
-        let opt_b: Options<HyphenSplitter> = 10.into();
-
-        let mut dyn_opt: &Options<dyn WordSplitter> = &opt_a;
-        assert_eq!(wrap("foo bar-baz", dyn_opt), vec!["foo bar-baz"]);
-
-        // Just assign a totally different option
-        dyn_opt = &opt_b;
-        assert_eq!(wrap("foo bar-baz", dyn_opt), vec!["foo bar-", "baz"]);
-    }
-
     #[test]
     fn trait_object_vec() {
-        // Create a vector of referenced trait-objects
-        let mut vector: Vec<&Options<dyn WordSplitter>> = Vec::new();
+        // Create a vector of Options containing trait-objects.
+        let mut vector: Vec<Options<Box<dyn WordSplitter>>> = Vec::new();
         // Expected result from each options
         let mut results = Vec::new();
 
-        let opt_usize: Options<_> = 10.into();
-        vector.push(&opt_usize);
+        let opt_full_type: Options<Box<dyn WordSplitter>> =
+            Options::new(10).splitter(Box::new(HyphenSplitter));
+        vector.push(opt_full_type);
         results.push(vec!["over-", "caffinated"]);
 
-        #[cfg(feature = "hyphenation")]
-        let dictionary = Standard::from_embedded(Language::EnglishUS).unwrap();
-        #[cfg(feature = "hyphenation")]
-        let opt_hyp = Options::new(8).splitter(dictionary);
-        #[cfg(feature = "hyphenation")]
-        vector.push(&opt_hyp);
-        #[cfg(feature = "hyphenation")]
-        results.push(vec!["over-", "caffi-", "nated"]);
-
         // Actually: Options<Box<dyn WordSplitter>>
-        let opt_box: Options = Options::new(10)
+        let opt_abbreviated_type: Options = Options::new(10)
             .break_words(false)
             .splitter(Box::new(NoHyphenation));
-        vector.push(&opt_box);
+        vector.push(opt_abbreviated_type);
         results.push(vec!["over-caffinated"]);
 
+        #[cfg(feature = "hyphenation")]
+        {
+            let dictionary = Standard::from_embedded(Language::EnglishUS).unwrap();
+            let opt_hyp: Options<Box<dyn WordSplitter>> =
+                Options::new(8).splitter(Box::new(dictionary));
+            vector.push(opt_hyp);
+            results.push(vec!["over-", "caffi-", "nated"]);
+        }
+
         // Test each entry
         for (opt, expected) in vector.into_iter().zip(results) {
-            assert_eq!(
-                // Just all the totally different options
-                wrap("over-caffinated", opt),
-                expected
-            );
+            assert_eq!(wrap("over-caffinated", opt), expected);
         }
     }
 
-    #[test]
-    fn outer_boxing() {
-        let mut wrapper: Box<Options<dyn WordSplitter>> = Box::new(Options::new(80));
-
-        // We must first deref the Box into a trait object and pass it by-reference
-        assert_eq!(wrap("foo bar baz", &*wrapper), vec!["foo bar baz"]);
-
-        // Replace the `Options` with a `usize`
-        wrapper = Box::new(Options::from(5));
-
-        // Deref per-se works as well, it already returns a reference
-        use std::ops::Deref;
-        assert_eq!(
-            wrap("foo bar baz", wrapper.deref()),
-            vec!["foo", "bar", "baz"]
-        );
-    }
-
     #[test]
     fn wrap_columns_empty_text() {
         assert_eq!(wrap_columns("", 1, 10, "| ", "", " |"), vec!["|        |"]);

src/splitting.rs

@@ -5,6 +5,8 @@
 //! functionality. [`HyphenSplitter`] is the default implementation of
 //! this treat: it will simply split words on existing hyphens.
 
+use std::ops::Deref;
+
 /// The `WordSplitter` trait describes where words can be split.
 ///
 /// If the textwrap crate has been compiled with the `hyphenation`
@@ -33,7 +35,7 @@
 /// details.
 ///
 /// [hyphenation]: https://docs.rs/hyphenation/
-pub trait WordSplitter: std::fmt::Debug {
+pub trait WordSplitter: WordSplitterClone + std::fmt::Debug {
     /// Return all possible indices where `word` can be split.
     ///
     /// The indices returned must be in range `0..word.len()`. They
@@ -51,16 +53,28 @@ pub trait WordSplitter: std::fmt::Debug {
     fn split_points(&self, word: &str) -> Vec<usize>;
 }
 
-impl<S: WordSplitter + ?Sized> WordSplitter for Box<S> {
-    fn split_points(&self, word: &str) -> Vec<usize> {
-        use std::ops::Deref;
-        self.deref().split_points(word)
+// The internal `WordSplitterClone` trait is allows us to implement
+// `Clone` for `Box<dyn WordSplitter>`. This in used in the
+// `From<&Options<'_, S>> for Options<'a, S>` implementation.
+pub trait WordSplitterClone {
+    fn clone_box(&self) -> Box<dyn WordSplitter>;
+}
+
+impl<T: WordSplitter + Clone + 'static> WordSplitterClone for T {
+    fn clone_box(&self) -> Box<dyn WordSplitter> {
+        Box::new(self.clone())
     }
 }
 
-impl<T: ?Sized + WordSplitter> WordSplitter for &T {
+impl Clone for Box<dyn WordSplitter> {
+    fn clone(&self) -> Box<dyn WordSplitter> {
+        self.deref().clone_box()
+    }
+}
+
+impl WordSplitter for Box<dyn WordSplitter> {
     fn split_points(&self, word: &str) -> Vec<usize> {
-        (*self).split_points(word)
+        self.deref().split_points(word)
     }
 }