Use ArcStr in LocalizedString and LabelText

This lets us share our text between druid and the piet layout object, and starts the process of moving us away from using String where possible. This was motivated by explorations around making Label optionally work as a `Widget<ArcStr>`, and using lens-like adapters to generate that text from some other data types as needed; this would be a small improvement of the label API.
linebender · Sep 25, 2020 · 7a3251b · 7a3251b
1 parent ec7fa2c
commit 7a3251b
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 43 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -34,7 +34,7 @@ You can find its changes [documented below](#060---2020-06-01).
 - `TextLayout` type simplifies drawing text ([#1182] by [@cmyr])
 - Implementation of `Data` trait for `i128` and `u128` primitive data types. ([#1214] by [@koutoftimer])
 - `LineBreaking` enum allows configuration of label line-breaking ([#1195] by [@cmyr])
-- `TextAlignment` support in `TextLayout` and `Label` ([#1210] by [@cmyr])`
+- `TextAlignment` support in `TextLayout` and `Label` ([#1210] by [@cmyr])
 - `UpdateCtx` gets `env_changed` and `env_key_changed` methods ([#1207] by [@cmyr])
 - `Button::from_label` to construct a `Button` with a provided `Label`. ([#1226] by [@ForLoveOfCats])
 - Lens: Added Unit lens for type erased / display only widgets that do not need data. ([#1232] by [@rjwittams])
@@ -60,6 +60,7 @@ You can find its changes [documented below](#060---2020-06-01).
 - `AnimFrame` was moved from `lifecycle` to `event` ([#1155] by [@jneem])
 - Contexts' `text()` methods return `&mut PietText` instead of cloning ([#1205] by [@cmyr])
 - Window construction: WindowDesc decomposed to PendingWindow and WindowConfig to allow for sub-windows and reconfiguration. ([#1235] by [@rjwittams])
+- `LocalizedString` and `LabelText` use `ArcStr` instead of String ([#1245] by [@cmyr])
 - `LensWrap` widget moved into widget module ([#1251] by [@cmyr])
 
 ### Deprecated
@@ -464,6 +465,7 @@ Last release without a changelog :(
 [#1220]: https://github.com/linebender/druid/pull/1220
 [#1238]: https://github.com/linebender/druid/pull/1238
 [#1241]: https://github.com/linebender/druid/pull/1241
+[#1245]: https://github.com/linebender/druid/pull/1245
 [#1251]: https://github.com/linebender/druid/pull/1251
 
 [Unreleased]: https://github.com/linebender/druid/compare/v0.6.0...master

diff --git a/druid/src/localization.rs b/druid/src/localization.rs
@@ -40,8 +40,7 @@ use std::{fs, io};
 
 use log::{debug, error, warn};
 
-use crate::env::Env;
-use crate::shell::Application;
+use crate::{Application, ArcStr, Env};
 
 use fluent_bundle::{
     FluentArgs, FluentBundle, FluentError, FluentMessage, FluentResource, FluentValue,
@@ -91,9 +90,9 @@ struct ArgSource<T>(ArgClosure<T>);
 #[derive(Debug, Clone)]
 pub struct LocalizedString<T> {
     pub(crate) key: &'static str,
-    placeholder: Option<String>,
+    placeholder: Option<ArcStr>,
     args: Option<Vec<(&'static str, ArgSource<T>)>>,
-    resolved: Option<String>,
+    resolved: Option<ArcStr>,
     resolved_lang: Option<LanguageIdentifier>,
 }
 
@@ -253,7 +252,7 @@ impl L10nManager {
         &'args self,
         key: &str,
         args: impl Into<Option<&'args FluentArgs<'args>>>,
-    ) -> Option<String> {
+    ) -> Option<ArcStr> {
         let args = args.into();
         let value = match self
             .current_bundle
@@ -281,10 +280,11 @@ impl L10nManager {
                 result
                     .chars()
                     .filter(|c| c != &START_ISOLATE && c != &END_ISOLATE)
-                    .collect(),
+                    .collect::<String>()
+                    .into(),
             )
         } else {
-            Some(result)
+            Some(result.into())
         }
     }
     //TODO: handle locale change
@@ -305,18 +305,18 @@ impl<T> LocalizedString<T> {
     /// Add a placeholder value. This will be used if localization fails.
     ///
     /// This is intended for use during prototyping.
-    pub fn with_placeholder(mut self, placeholder: impl Into<String>) -> Self {
+    pub fn with_placeholder(mut self, placeholder: impl Into<ArcStr>) -> Self {
         self.placeholder = Some(placeholder.into());
         self
     }
 
     /// Return the localized value for this string, or the placeholder, if
     /// the localization is missing, or the key if there is no placeholder.
-    pub fn localized_str(&self) -> &str {
+    pub fn localized_str(&self) -> ArcStr {
         self.resolved
-            .as_deref()
-            .or_else(|| self.placeholder.as_deref())
-            .unwrap_or(self.key)
+            .clone()
+            .or_else(|| self.placeholder.clone())
+            .unwrap_or_else(|| self.key.into())
     }
 
     /// Add a named argument and a corresponding closure. This closure

diff --git a/druid/src/menu.rs b/druid/src/menu.rs
@@ -347,7 +347,7 @@ impl<T: Data> MenuDesc<T> {
                     item.platform_id = MenuItemId::next();
                     menu.add_item(
                         item.platform_id.as_u32(),
-                        item.title.localized_str(),
+                        &item.title.localized_str(),
                         item.hotkey.as_ref(),
                         item.enabled,
                         item.selected,

diff --git a/druid/src/widget/label.rs b/druid/src/widget/label.rs
@@ -17,41 +17,54 @@
 use crate::piet::{Color, PietText};
 use crate::widget::prelude::*;
 use crate::{
-    BoxConstraints, Data, FontDescriptor, KeyOrValue, LocalizedString, Point, Size, TextAlignment,
-    TextLayout,
+    ArcStr, BoxConstraints, Data, FontDescriptor, KeyOrValue, LocalizedString, Point, Size,
+    TextAlignment, TextLayout,
 };
 
 // added padding between the edges of the widget and the text.
 const LABEL_X_PADDING: f64 = 2.0;
 
 /// The text for the label.
 ///
-/// This can be one of three things; either a `String`, a [`LocalizedString`],
+/// This can be one of three things; either a [`ArcStr`], a [`LocalizedString`],
 /// or a closure with the signature, `Fn(&T, &Env) -> String`, where `T` is
-/// the `Data` at this point in the tree.
+/// the [`Data`] at this point in the tree.
 ///
+/// [`ArcStr`]: ../type.ArcStr.html
 /// [`LocalizedString`]: ../struct.LocalizedString.html
+/// [`Data`]: ../trait.Data.html
 pub enum LabelText<T> {
     /// Localized string that will be resolved through `Env`.
     Localized(LocalizedString<T>),
-    /// Specific text
-    Specific(String),
+    /// Static text.
+    Static(Static),
     /// The provided closure is called on update, and its return
     /// value is used as the text for the label.
     Dynamic(Dynamic<T>),
 }
 
 /// Text that is computed dynamically.
-#[doc(hidden)]
 pub struct Dynamic<T> {
     f: Box<dyn Fn(&T, &Env) -> String>,
-    resolved: String,
+    resolved: ArcStr,
+}
+
+/// Static text.
+pub struct Static {
+    /// The text.
+    string: ArcStr,
+    /// Whether or not the `resolved` method has been called yet.
+    ///
+    /// We want to return `true` from that method when it is first called,
+    /// so that callers will know to retrieve the text. This matches
+    /// the behaviour of the other variants.
+    resolved: bool,
 }
 
 /// A label that draws some text.
 ///
 /// A label is the easiest way to display text in Druid. A label is instantiated
-/// with some [`LabelText`] type, such as a `String` or a [`LocalizedString`],
+/// with some [`LabelText`] type, such as an [`ArcStr`] or a [`LocalizedString`],
 /// and also has methods for setting the default font, font-size, text color,
 /// and other attributes.
 ///
@@ -80,7 +93,7 @@ pub struct Dynamic<T> {
 /// # let _ = SizedBox::<()>::new(important_label);
 /// ```
 ///
-///
+/// [`ArcStr`]: ../type.ArcStr.html
 /// [`LabelText`]: struct.LabelText.html
 /// [`LocalizedString`]: ../struct.LocalizedString.html
 /// [`draw_at`]: #method.draw_at
@@ -218,11 +231,6 @@ impl<T: Data> Label<T> {
         self.needs_rebuild = true;
     }
 
-    /// Returns this label's current text.
-    pub fn text(&self) -> &str {
-        self.text.display_text()
-    }
-
     /// Set the text color.
     ///
     /// The argument can be either a `Color` or a [`Key<Color>`].
@@ -307,11 +315,26 @@ impl<T: Data> Label<T> {
     }
 }
 
+impl Static {
+    fn new(s: ArcStr) -> Self {
+        Static {
+            string: s,
+            resolved: false,
+        }
+    }
+
+    fn resolve(&mut self) -> bool {
+        let is_first_call = self.resolved;
+        self.resolved = true;
+        is_first_call
+    }
+}
+
 impl<T> Dynamic<T> {
     fn resolve(&mut self, data: &T, env: &Env) -> bool {
         let new = (self.f)(data, env);
-        let changed = new != self.resolved;
-        self.resolved = new;
+        let changed = new.as_str() != self.resolved.as_ref();
+        self.resolved = new.into();
         changed
     }
 }
@@ -320,18 +343,18 @@ impl<T: Data> LabelText<T> {
     /// Call callback with the text that should be displayed.
     pub fn with_display_text<V>(&self, mut cb: impl FnMut(&str) -> V) -> V {
         match self {
-            LabelText::Specific(s) => cb(s.as_str()),
-            LabelText::Localized(s) => cb(s.localized_str()),
-            LabelText::Dynamic(s) => cb(s.resolved.as_str()),
+            LabelText::Static(s) => cb(&s.string),
+            LabelText::Localized(s) => cb(&s.localized_str()),
+            LabelText::Dynamic(s) => cb(&s.resolved),
         }
     }
 
     /// Return the current resolved text.
-    pub fn display_text(&self) -> &str {
+    pub fn display_text(&self) -> ArcStr {
         match self {
-            LabelText::Specific(s) => s.as_str(),
+            LabelText::Static(s) => s.string.clone(),
             LabelText::Localized(s) => s.localized_str(),
-            LabelText::Dynamic(s) => s.resolved.as_str(),
+            LabelText::Dynamic(s) => s.resolved.clone(),
         }
     }
 
@@ -341,7 +364,7 @@ impl<T: Data> LabelText<T> {
     /// Returns `true` if the string has changed.
     pub fn resolve(&mut self, data: &T, env: &Env) -> bool {
         match self {
-            LabelText::Specific(_) => false,
+            LabelText::Static(s) => s.resolve(),
             LabelText::Localized(s) => s.resolve(data, env),
             LabelText::Dynamic(s) => s.resolve(data, env),
         }
@@ -389,13 +412,19 @@ impl<T: Data> Widget<T> for Label<T> {
 
 impl<T> From<String> for LabelText<T> {
     fn from(src: String) -> LabelText<T> {
-        LabelText::Specific(src)
+        LabelText::Static(Static::new(src.into()))
     }
 }
 
 impl<T> From<&str> for LabelText<T> {
     fn from(src: &str) -> LabelText<T> {
-        LabelText::Specific(src.to_string())
+        LabelText::Static(Static::new(src.into()))
+    }
+}
+
+impl<T> From<ArcStr> for LabelText<T> {
+    fn from(string: ArcStr) -> LabelText<T> {
+        LabelText::Static(Static::new(string))
     }
 }
 
@@ -410,7 +439,7 @@ impl<T, F: Fn(&T, &Env) -> String + 'static> From<F> for LabelText<T> {
         let f = Box::new(src);
         LabelText::Dynamic(Dynamic {
             f,
-            resolved: String::default(),
+            resolved: ArcStr::from(""),
         })
     }
 }
diff --git a/druid/src/win_handler.rs b/druid/src/win_handler.rs
@@ -700,7 +700,7 @@ impl<T: Data> AppState<T> {
         let env = self.env();
 
         pending.title.resolve(&data, &env);
-        builder.set_title(pending.title.display_text());
+        builder.set_title(pending.title.display_text().to_string());
 
         let platform_menu = pending
             .menu

diff --git a/druid/src/window.rs b/druid/src/window.rs
@@ -423,7 +423,7 @@ impl<T: Data> Window<T> {
 
     pub(crate) fn update_title(&mut self, data: &T, env: &Env) {
         if self.title.resolve(data, env) {
-            self.handle.set_title(self.title.display_text());
+            self.handle.set_title(&self.title.display_text());
         }
     }