Skip to content

Commit

Permalink
Remove the “outer boxing” support for Options
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mgeisler committed May 1, 2021
1 parent e9f9d78 commit 0b090bc
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 154 deletions.
7 changes: 6 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
99 changes: 0 additions & 99 deletions examples/multi-layouts.rs

This file was deleted.

2 changes: 1 addition & 1 deletion src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
68 changes: 19 additions & 49 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(),
}
}
}
Expand Down Expand Up @@ -1836,46 +1836,34 @@ 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!(
Expand All @@ -1886,24 +1874,6 @@ mod tests {
}
}

#[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!["| |"]);
Expand Down
29 changes: 25 additions & 4 deletions src/splitting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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
Expand All @@ -51,14 +53,33 @@ pub trait WordSplitter: std::fmt::Debug {
fn split_points(&self, word: &str) -> Vec<usize>;
}

impl<S: WordSplitter + ?Sized> WordSplitter for Box<S> {
/// Internal helper-trait used to implement `Clone` for `Box<dyn
/// WordSplitter>`.
pub trait WordSplitterClone {
fn clone_box(&self) -> Box<dyn WordSplitter>;
}

/// Blanket implementation of the `clone_box` helper method.
impl<T: WordSplitter + Clone + 'static> WordSplitterClone for T {
fn clone_box(&self) -> Box<dyn WordSplitter> {
Box::new(self.clone())
}
}

/// Blanket implementation of `Clone` for all `Box<dyn WordSplitter>`.
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> {
use std::ops::Deref;
self.deref().split_points(word)
}
}

impl<T: ?Sized + WordSplitter> WordSplitter for &T {
impl<T: WordSplitter> WordSplitter for &'static T {
fn split_points(&self, word: &str) -> Vec<usize> {
(*self).split_points(word)
}
Expand Down

0 comments on commit 0b090bc

Please sign in to comment.