Restructure ColouredString via a common trait.

Now ColouredString and ColouredStringSlice both implement the trait
ColouredStringCommon, and as many methods as possible are provided by
that trait, depending only on the unimplemented trait methods to get
the text and colours and &str.

This reduces the amount of code, and also makes points of use more
flexible, because I can implement the common trait not just for both
the actual types, but also references to them.

The only disappointment is that I wasn't able to also fold the four
identical impls of std::ops::Add into one via a single
doubly-quantified impl. But instead I got to deploy my first
macro_rules!, so that's something.

In the next commits I'll clean up now-unnecessary boilerplate at call
sites.

diff --git a/src/coloured_string.rs b/src/coloured_string.rs
index 885649f76e85d048b75de8cd841cb50adf43924c..b36c2fc101d313c399e8694dd073ca69f942f582 100644 (file)
--- a/src/coloured_string.rs
+++ b/src/coloured_string.rs
@@ -1,160 +1,193 @@
 use unicode_width::UnicodeWidthStr;
 use unicode_width::UnicodeWidthChar;
 
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct ColouredString {
-    text: String,
-    colour: String,
-}
+pub trait ColouredStringCommon {
+    fn text(&self) -> & str;
+    fn colours(&self) -> & str;
 
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct ColouredStringSlice<'a> {
-    text: &'a str,
-    colour: &'a str,
-}
+    fn is_empty(&self) -> bool { self.text().is_empty() }
+    fn nchars(&self) -> usize { self.text().chars().count() }
+    fn width(&self) -> usize { UnicodeWidthStr::width(self.text()) }
 
-impl ColouredString {
-    pub fn plain(text: &str) -> Self {
-        ColouredString {
-            text: text.to_owned(),
-            colour: " ".repeat(text.chars().count()),
+    fn slice<'a>(&'a self) -> ColouredStringSlice<'a> {
+        ColouredStringSlice {
+            text: self.text(),
+            colours: self.colours(),
         }
     }
-    pub fn uniform(text: &str, colour: char) -> Self {
-        ColouredString {
-            text: text.to_owned(),
-            colour: colour.to_string().repeat(text.chars().count()),
+
+    fn truncate(&self, width: usize) -> ColouredStringSlice {
+        self.split(width).next().unwrap()
+    }
+
+    fn chars<'a>(&'a self) -> ColouredStringCharIterator<'a> {
+        ColouredStringCharIterator {
+            cs: self.slice(),
+            textpos: 0,
+            colourpos: 0,
         }
     }
-    pub fn general(text: &str, colour: &str) -> Self {
-        assert_eq!(text.chars().count(), colour.chars().count(),
-                   "Mismatched lengths in ColouredString::general");
-        ColouredString {
-            text: text.to_owned(),
-            colour: colour.to_owned(),
+    fn frags<'a>(&'a self) -> ColouredStringFragIterator<'a> {
+        ColouredStringFragIterator {
+            cs: self.slice(),
+            textpos: 0,
+            colourpos: 0,
         }
     }
-
-    pub fn repeat(self, count: usize) -> Self {
-        ColouredString {
-            text: self.text.repeat(count),
-            colour: self.colour.repeat(count),
+    fn split<'a>(&'a self, width: usize) -> ColouredStringSplitIterator<'a> {
+        ColouredStringSplitIterator {
+            cs: self.slice(),
+            width,
+            textpos: 0,
+            colourpos: 0,
+            delivered_empty: false,
         }
     }
 
-    pub fn is_empty(&self) -> bool { self.text.is_empty() }
-    pub fn nchars(&self) -> usize { self.text.chars().count() }
-    pub fn width(&self) -> usize { UnicodeWidthStr::width(&self.text as &str) }
-
-    pub fn text(&self) -> &str { &self.text }
-    pub fn colours(&self) -> &str { &self.colour }
-    pub fn recolour(&self, colour: char) -> Self {
-        Self::uniform(&self.text, colour)
+    fn repeat(&self, count: usize) -> ColouredString {
+        ColouredString::general(&self.text().repeat(count),
+                                &self.colours().repeat(count))
     }
 
-    pub fn slice(&self) -> ColouredStringSlice {
-        ColouredStringSlice {
-            text: &self.text,
-            colour: &self.colour,
-        }
+    fn recolour(&self, colour: char) -> ColouredString {
+        ColouredString::uniform(self.text(), colour)
     }
+}
 
-    pub fn truncate(&self, width: usize) -> ColouredStringSlice {
-        self.split(width).next().unwrap()
-    }
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct ColouredString {
+    text: String,
+    colours: String,
+}
 
-    pub fn push_str(&mut self, more: &ColouredStringSlice<'_>) {
-        self.text.push_str(more.text);
-        self.colour.push_str(more.colour);
-    }
+impl ColouredStringCommon for ColouredString {
+    fn text(&self) -> &str { &self.text }
+    fn colours(&self) -> &str { &self.colours }
 }
 
-impl<'a> ColouredStringSlice<'a> {
-    pub fn general(text: &'a str, colour: &'a str) -> Self {
-        assert_eq!(text.chars().count(), colour.chars().count(),
-                   "Mismatched lengths in ColouredStringSlice::general");
-        ColouredStringSlice {
-            text,
-            colour,
-        }
-    }
+impl<'a> ColouredStringCommon for &'a ColouredString {
+    fn text(&self) -> &str { &self.text }
+    fn colours(&self) -> &str { &self.colours }
+}
 
-    pub fn to_owned(&self) -> ColouredString {
+// I'd have liked here to write
+// impl<T: ColouredStringCommon> From<T> for ColouredString { ... }
+// on the basis that this code can sensibly make an owned ColouredString
+// out of anything that provides .text() and .colours().
+//
+// But the problem is that that conflicts with the standard reflexive
+// definition in which ColouredString itself _already_ implements
+// From<ColouredString> via the identity function. Of course that _is_
+// a much more sensible definition in that one case! And I don't know
+// of any language feature that lets me offer the definition below for
+// only everything _else_ satisfying the type bound.
+impl<'a> From<ColouredStringSlice<'a>> for ColouredString {
+    fn from(value: ColouredStringSlice<'a>) -> Self {
         ColouredString {
-            text: self.text.to_owned(),
-            colour: self.colour.to_owned(),
+            text: value.text().to_owned(),
+            colours: value.colours().to_owned(),
         }
     }
-
-    pub fn nchars(&self) -> usize { self.text.chars().count() }
-    pub fn width(&self) -> usize { UnicodeWidthStr::width(self.text as &str) }
-    pub fn text(&self) -> &'a str { self.text }
-
-    pub fn truncate(&'a self, width: usize) -> ColouredStringSlice<'a> {
-        self.split(width).next().unwrap()
-    }
 }
 
-impl std::ops::Add<ColouredString> for ColouredString {
-    type Output = Self;
-    fn add(self, rhs: Self) -> Self {
+impl ColouredString {
+    pub fn plain(text: &str) -> Self {
         ColouredString {
-            text: self.text + &rhs.text,
-            colour: self.colour + &rhs.colour,
+            text: text.to_owned(),
+            colours: " ".repeat(text.chars().count()),
         }
     }
-}
 
-impl std::ops::Add<&ColouredString> for ColouredString {
-    type Output = Self;
-    fn add(self, rhs: &ColouredString) -> Self {
+    pub fn uniform(text: &str, colour: char) -> Self {
         ColouredString {
-            text: self.text + &rhs.text,
-            colour: self.colour + &rhs.colour,
+            text: text.to_owned(),
+            colours: colour.to_string().repeat(text.chars().count()),
         }
     }
-}
 
-impl std::ops::Add<&str> for ColouredString {
-    type Output = Self;
-    fn add(self, rhs: &str) -> Self {
+    pub fn general(text: &str, colours: &str) -> Self {
+        assert_eq!(text.chars().count(), colours.chars().count(),
+                   "Mismatched lengths in ColouredString::general");
         ColouredString {
-            text: self.text + rhs,
-            colour: self.colour + &(" ".repeat(rhs.chars().count())),
+            text: text.to_owned(),
+            colours: colours.to_owned(),
         }
     }
-}
 
-impl std::ops::Add<ColouredString> for &ColouredString {
-    type Output = ColouredString;
-    fn add(self, rhs: ColouredString) -> ColouredString {
+    fn concat<T: ColouredStringCommon, U: ColouredStringCommon>
+        (lhs: T, rhs: U) -> ColouredString
+    {
         ColouredString {
-            text: self.text.to_owned() + &rhs.text,
-            colour: self.colour.to_owned() + &rhs.colour,
+            text: lhs.text().to_owned() + rhs.text(),
+            colours: lhs.colours().to_owned() + rhs.colours(),
         }
     }
+
+    pub fn push_str(&mut self, more: impl ColouredStringCommon) {
+        self.text.push_str(more.text());
+        self.colours.push_str(more.colours());
+    }
 }
 
-impl std::ops::Add<ColouredStringSlice<'_>> for ColouredString {
-    type Output = ColouredString;
-    fn add(self, rhs: ColouredStringSlice<'_>) -> ColouredString {
-        ColouredString {
-            text: self.text + rhs.text,
-            colour: self.colour + rhs.colour,
-        }
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct ColouredStringSlice<'a> {
+    text: &'a str,
+    colours: &'a str,
+}
+
+impl<'a> ColouredStringCommon for ColouredStringSlice<'a> {
+    fn text(&self) -> &str { self.text }
+    fn colours(&self) -> &str { self.colours }
+}
+
+impl<'a> ColouredStringCommon for &ColouredStringSlice<'a> {
+    fn text(&self) -> &str { self.text }
+    fn colours(&self) -> &str { self.colours }
+}
+
+impl<'a> ColouredStringSlice<'a> {
+    pub fn general(text: &'a str, colours: &'a str) -> Self {
+        assert_eq!(text.chars().count(), colours.chars().count(),
+                   "Mismatched lengths in ColouredStringSlice::general");
+
+        ColouredStringSlice { text, colours }
     }
+
+    pub fn to_owned(&self) -> ColouredString { self.clone().into() }
 }
 
-impl std::ops::Add<ColouredString> for &str {
-    type Output = ColouredString;
-    fn add(self, rhs: ColouredString) -> ColouredString {
-        ColouredString {
-            text: self.to_owned() + &rhs.text,
-            colour: (" ".repeat(self.chars().count())) + &rhs.colour,
+// We want to be able to use the + operator to concatenate any two
+// types that impl ColouredStringCommon. The logic of _how_ to do this
+// is already written, in the ColouredString::concat() constructor
+// above. Now we just need to make the + operator invoke it.
+//
+// I'd like to do this by writing a single item along the lines of
+// impl<T: ColouredStringCommon, U: ColouredStringCommon> Add<U> for T { ... }
+// but that's not allowed, because if a user of this module implemented
+// ColouredStringCommon for one of _their_ types, then this would end up
+// trying to implement a trait that's not mine (Add) on a type that's not
+// mine (the user's type).
+//
+// Making ColouredStringCommon into a sealed trait doesn't cause the
+// compiler to recognise that risk as nonexistent, which is a shame.
+// So instead let's do some macro business, to avoid copy-pasting four
+// times.
+macro_rules! impl_Add {
+    ($type:ty, $($life:lifetime)?) => {
+        impl<$($life,)? U: ColouredStringCommon> std::ops::Add<U> for $type {
+            type Output = ColouredString;
+            fn add(self, rhs: U) -> ColouredString {
+                ColouredString::concat(self, rhs)
+            }
         }
-    }
+    };
 }
 
+impl_Add!(ColouredString,);
+impl_Add!(&ColouredString,);
+impl_Add!(ColouredStringSlice<'a>, 'a);
+impl_Add!(&ColouredStringSlice<'a>, 'a);
+
 pub struct ColouredStringCharIterator<'a> {
     cs: ColouredStringSlice<'a>,
     textpos: usize,
@@ -175,64 +208,12 @@ pub struct ColouredStringSplitIterator<'a> {
     delivered_empty: bool,
 }
 
-impl<'a> ColouredStringSlice<'a> {
-    pub fn chars(&self) -> ColouredStringCharIterator<'a> {
-        ColouredStringCharIterator {
-            cs: self.clone(),
-            textpos: 0,
-            colourpos: 0,
-        }
-    }
-    pub fn frags(&self) -> ColouredStringFragIterator<'a> {
-        ColouredStringFragIterator {
-            cs: self.clone(),
-            textpos: 0,
-            colourpos: 0,
-        }
-    }
-    pub fn split(&'a self, width: usize) -> ColouredStringSplitIterator<'a> {
-        ColouredStringSplitIterator {
-            cs: self.clone(),
-            width,
-            textpos: 0,
-            colourpos: 0,
-            delivered_empty: false,
-        }
-    }
-}
-
-impl<'a> ColouredString {
-    pub fn chars(&'a self) -> ColouredStringCharIterator<'a> {
-        ColouredStringCharIterator {
-            cs: self.slice(),
-            textpos: 0,
-            colourpos: 0,
-        }
-    }
-    pub fn frags(&'a self) -> ColouredStringFragIterator<'a> {
-        ColouredStringFragIterator {
-            cs: self.slice(),
-            textpos: 0,
-            colourpos: 0,
-        }
-    }
-    pub fn split(&'a self, width: usize) -> ColouredStringSplitIterator<'a> {
-        ColouredStringSplitIterator {
-            cs: self.slice(),
-            width,
-            textpos: 0,
-            colourpos: 0,
-            delivered_empty: false,
-        }
-    }
-}
-
 impl<'a> Iterator for ColouredStringCharIterator<'a> {
     type Item = ColouredStringSlice<'a>;
     fn next(&mut self) -> Option<Self::Item> {
         let textslice = &self.cs.text[self.textpos..];
         let mut textit = textslice.char_indices();
-        let colourslice = &self.cs.colour[self.colourpos..];
+        let colourslice = &self.cs.colours[self.colourpos..];
         let mut colourit = colourslice.char_indices();
         if let (Some(_), Some(_)) = (textit.next(), colourit.next()) {
             let (textend, colourend) = match (textit.next(), colourit.next()) {
@@ -243,7 +224,7 @@ impl<'a> Iterator for ColouredStringCharIterator<'a> {
             self.colourpos += colourend;
             Some(ColouredStringSlice {
                     text: &textslice[..textend],
-                    colour: &colourslice[..colourend],
+                    colours: &colourslice[..colourend],
                 })
         } else {
             None
@@ -256,7 +237,7 @@ impl<'a> Iterator for ColouredStringFragIterator<'a> {
     fn next(&mut self) -> Option<Self::Item> {
         let textslice = &self.cs.text[self.textpos..];
         let mut textit = textslice.char_indices();
-        let colourslice = &self.cs.colour[self.colourpos..];
+        let colourslice = &self.cs.colours[self.colourpos..];
         let mut colourit = colourslice.char_indices();
         if let Some((_, colour)) = colourit.next() {
             // Expect this not to run out, because colour didn't
@@ -291,7 +272,7 @@ impl<'a> Iterator for ColouredStringSplitIterator<'a> {
     fn next(&mut self) -> Option<Self::Item> {
         let textslice = &self.cs.text[self.textpos..];
         let mut textit = textslice.char_indices();
-        let colourslice = &self.cs.colour[self.colourpos..];
+        let colourslice = &self.cs.colours[self.colourpos..];
         let mut colourit = colourslice.char_indices();
         match (textit.next(), colourit.next()) {
             (None, None) => {
@@ -329,7 +310,7 @@ impl<'a> Iterator for ColouredStringSplitIterator<'a> {
                 self.colourpos += colourend;
                 Some(ColouredStringSlice {
                         text: &textslice[..textend],
-                        colour: &colourslice[..colourend],
+                        colours: &colourslice[..colourend],
                     })
             }
             _ => panic!("length mismatch in CSSI"),
@@ -356,10 +337,6 @@ fn test_concat() {
     assert_eq!(ColouredString::general("xyz", "pqr") +
                ColouredString::general("abcde", "ijklm"),
                ColouredString::general("xyzabcde", "pqrijklm"));
-    assert_eq!(ColouredString::general("xyz", "pqr") + "abcde",
-               ColouredString::general("xyzabcde", "pqr     "));
-    assert_eq!("abcde" + ColouredString::general("xyz", "pqr"),
-               ColouredString::general("abcdexyz", "     pqr"));
 }
 
 #[test]

diff --git a/src/editor.rs b/src/editor.rs
index f452a77c6db772453818ac865326827ab1bb4454..ebb0e02fb208624fe3eaa79f313bf285025386ec 100644 (file)
--- a/src/editor.rs
+++ b/src/editor.rs
@@ -3,7 +3,7 @@ use unicode_width::UnicodeWidthChar;
 
 use super::activity_stack::{NonUtilityActivity, UtilityActivity};
 use super::client::{Client, ClientError};
-use super::coloured_string::ColouredString;
+use super::coloured_string::*;
 use super::file::SearchDirection;
 use super::text::*;
 use super::tui::{

diff --git a/src/file.rs b/src/file.rs
index ffc1c2bc451d4297e882c84b46fdcf01f911296c..d33733e4d4c0ae61ca91a921e062b193a3e5f572 100644 (file)
--- a/src/file.rs
+++ b/src/file.rs
@@ -7,7 +7,7 @@ use super::activity_stack::{
     NonUtilityActivity, UtilityActivity, OverlayActivity,
 };
 use super::client::{Client, ClientError, FeedId, FeedExtend, Boosts, Replies};
-use super::coloured_string::ColouredString;
+use super::coloured_string::*;
 use super::text::*;
 use super::tui::{
     ActivityState, CursorPosition, LogicalAction,

diff --git a/src/html.rs b/src/html.rs
index 9e4debed905f965a5a57d5820b539767ca489a37..83faca18967f75d959979d4bdadc8fbf1cdbf5cb 100644 (file)
--- a/src/html.rs
+++ b/src/html.rs
@@ -5,7 +5,7 @@ use html2text::render::text_renderer::{
 };
 use std::cell::RefCell;
 
-use super::coloured_string::ColouredString;
+use super::coloured_string::*;
 
 #[derive(Clone, Debug, Default)]
 struct OurDecorator<'a> {

diff --git a/src/text.rs b/src/text.rs
index 2a5fa64c7f97d9a833bed15f6c5fb7d6f456084d..f95a2460e30b2ed58fc726f9701bba87f15988a8 100644 (file)
--- a/src/text.rs
+++ b/src/text.rs
@@ -9,7 +9,7 @@ use super::html;
 use super::client::{Client, ClientError};
 use super::types::*;
 use super::tui::OurKey;
-use super::coloured_string::{ColouredString, ColouredStringSlice};
+use super::coloured_string::*;
 
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub enum HighlightType { User, Status, WholeStatus, PollOption }

diff --git a/src/tui.rs b/src/tui.rs
index b34aa73b7f3229ddbbf27d45a2ee2029f99f8585..60c8e2098827089a6c287232f2d59cca8cd61014 100644 (file)
--- a/src/tui.rs
+++ b/src/tui.rs
@@ -21,7 +21,7 @@ use super::activity_stack::*;
 use super::client::{
     Client, ClientError, FeedId, FeedExtend, StreamId, StreamUpdate,
 };
-use super::coloured_string::{ColouredString, ColouredStringSlice};
+use super::coloured_string::*;
 use super::config::ConfigLocation;
 use super::menu::*;
 use super::file::*;