From 49b54041197564bdb889ad87337801511c68b500 Mon Sep 17 00:00:00 2001 From: Martin Geisler Date: Sun, 18 Apr 2021 14:10:06 +0200 Subject: [PATCH] =?UTF-8?q?Remove=20the=20=E2=80=9Couter=20boxing=E2=80=9D?= =?UTF-8?q?=20support=20for=20`Options`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In #219, the type parameter for `Options` was relaxed to `?Sized`, which means that `Options` 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>; 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>; 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; 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. --- Cargo.toml | 7 ++- examples/multi-layouts.rs | 99 --------------------------------------- src/core.rs | 2 +- src/lib.rs | 74 ++++++++--------------------- src/splitting.rs | 28 ++++++++--- 5 files changed, 48 insertions(+), 162 deletions(-) delete mode 100644 examples/multi-layouts.rs diff --git a/Cargo.toml b/Cargo.toml index 697708f4..6a6d9bd7 100644 --- a/Cargo.toml +++ b/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" diff --git a/examples/multi-layouts.rs b/examples/multi-layouts.rs deleted file mode 100644 index b38f9d1e..00000000 --- a/examples/multi-layouts.rs +++ /dev/null @@ -1,99 +0,0 @@ -use textwrap::{NoHyphenation, Options}; - -#[cfg(feature = "hyphenation")] -use hyphenation::{Language, Load, Standard}; - -// Pretend this is an external crate -mod library { - use textwrap::{Options, WordSplitter}; - - #[derive(Debug, Default)] - pub struct Layout<'a> { - // Use trait-objects which can be easily converted from any concrete `Options` - styles: Vec>>, - } - - impl<'a> Layout<'a> { - pub fn new() -> Self { - Default::default() - } - - // Similar signature like `wrap` has, so it takes (nearly) everything that `warp` takes. - pub fn add>>(&mut self, option: T) { - self.styles.push(Box::new(option.into())); - } - - pub fn print(&self, text: &str) { - // Now we can easily go over all the arbitrary Options and use them for layouting. - for opt in self.styles.iter() { - println!(); - - // the debug output of the hyphenation is very long - //println!("Options: {:#?}", opt); - - println!("{:0width$}", 0, width = opt.width); - - // Just use the textwrap functions as usual. - // However, we have to first coerce it into a trait-object - let dyn_opt: &Options<'a, dyn WordSplitter> = opt; - println!("{}", textwrap::fill(text, dyn_opt)); - } - } - } -} - -pub fn main() { - // pretend we are a user of the library module above - - // Just some options (see below for usage) - let some_opt = Options::new(25).initial_indent("----"); - - // The struct from the 'library' that we are using - let mut layout = library::Layout::new(); - - // Add some arbitrary options. We can use here the same as for `fill` & `wrap`. - - // Plain Options - layout.add(Options::new(20)); - - // usize - layout.add(30); - - // Customized Options - let opt = Options::new(30); - let opt = opt.subsequent_indent("****"); - layout.add(opt.clone()); // notice, here we pass opt by-value instead of by-reference - - // We can use boxed splitters too (however, we have to coerce the Options) - let opt: Options = opt.splitter(Box::new(NoHyphenation)); - layout.add(opt); - - // We can also pass-in references, however, those need to outlive the local - // `layout`, so here, it must be declared before `layout` (drop order). - layout.add(&some_opt); - - // If you like, you can download more dictionaries from - // https://github.com/tapeinosyne/hyphenation/tree/master/dictionaries - // Place the dictionaries in the examples/ directory. Here we - // just load the embedded en-us dictionary. - #[cfg(feature = "hyphenation")] - for lang in &[Language::EnglishUS] { - let dictionary = Standard::from_embedded(*lang).or_else(|_| { - let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) - .join("examples") - .join(format!("{}.standard.bincode", lang.code())); - Standard::from_path(*lang, &path) - }); - - if let Ok(dict) = dictionary { - layout.add(Options::with_splitter(25, dict)); - } - } - - let example = "Memory safety without garbage collection. \ - Concurrency without data races. \ - Zero-cost abstractions."; - - // Printout above text in all different layouts - layout.print(example); -} diff --git a/src/core.rs b/src/core.rs index c26bba76..ced5e5a7 100644 --- a/src/core.rs +++ b/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 { diff --git a/src/lib.rs b/src/lib.rs index 6f1b72a7..13be32e6 100644 --- a/src/lib.rs +++ b/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> { +pub struct Options<'a, S = Box> { /// 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,7 +207,7 @@ pub struct Options<'a, S: ?Sized = Box> { 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, @@ -215,7 +215,7 @@ impl<'a, S: ?Sized> From<&'a Options<'a, S>> for Options<'a, &'a S> { 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 = Options::with_splitter(20, NoHyphenation); - let opt_b: Options = 10.into(); - - let mut dyn_opt: &Options = &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> = Vec::new(); + // Create a vector of Options containing trait-objects. + let mut vector: Vec>> = 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> = + 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> - 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> = + 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> = 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!["| |"]); diff --git a/src/splitting.rs b/src/splitting.rs index e92b1888..4cee9106 100644 --- a/src/splitting.rs +++ b/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; } -impl WordSplitter for Box { - fn split_points(&self, word: &str) -> Vec { - use std::ops::Deref; - self.deref().split_points(word) +// The internal `WordSplitterClone` trait is allows us to implement +// `Clone` for `Box`. This in used in the +// `From<&Options<'_, S>> for Options<'a, S>` implementation. +pub trait WordSplitterClone { + fn clone_box(&self) -> Box; +} + +impl WordSplitterClone for T { + fn clone_box(&self) -> Box { + Box::new(self.clone()) } } -impl WordSplitter for &T { +impl Clone for Box { + fn clone(&self) -> Box { + self.deref().clone_box() + } +} + +impl WordSplitter for Box { fn split_points(&self, word: &str) -> Vec { - (*self).split_points(word) + self.deref().split_points(word) } }