Don't make `fonts::Features` `Copy`

Antonio Scandurra created 2 years ago

Change summary

crates/editor/src/display_map.rs              | 16 ++++++++--------
crates/editor/src/display_map/block_map.rs    |  6 +++---
crates/editor/src/display_map/wrap_map.rs     |  2 +-
crates/editor/src/editor.rs                   |  3 +--
crates/editor/src/movement.rs                 |  2 +-
crates/editor/src/test.rs                     |  2 +-
crates/gpui/examples/text.rs                  |  2 +-
crates/gpui/src/font_cache.rs                 | 12 ++++++------
crates/gpui/src/fonts.rs                      |  6 +++---
crates/gpui/src/text_layout.rs                |  4 ++--
crates/settings/src/settings.rs               | 14 +++-----------
crates/terminal_view/src/terminal_element.rs  |  9 +++++----
crates/theme_testbench/src/theme_testbench.rs |  2 +-
13 files changed, 36 insertions(+), 44 deletions(-)

Detailed changes

crates/editor/src/display_map.rs

@@ -786,7 +786,7 @@ pub mod tests {
         let buffer_start_excerpt_header_height = rng.gen_range(1..=5);
         let excerpt_header_height = rng.gen_range(1..=5);
         let family_id = font_cache
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = font_cache
             .select_font(family_id, &Default::default())
@@ -1045,7 +1045,7 @@ pub mod tests {
         let font_cache = cx.font_cache();
 
         let family_id = font_cache
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = font_cache
             .select_font(family_id, &Default::default())
@@ -1137,7 +1137,7 @@ pub mod tests {
         let buffer = MultiBuffer::build_simple(&text, cx);
         let family_id = cx
             .font_cache()
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = cx
             .font_cache()
@@ -1222,7 +1222,7 @@ pub mod tests {
 
         let font_cache = cx.font_cache();
         let family_id = font_cache
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = font_cache
             .select_font(family_id, &Default::default())
@@ -1312,7 +1312,7 @@ pub mod tests {
         let font_cache = cx.font_cache();
 
         let family_id = font_cache
-            .load_family(&["Courier"], Default::default())
+            .load_family(&["Courier"], &Default::default())
             .unwrap();
         let font_id = font_cache
             .select_font(family_id, &Default::default())
@@ -1386,7 +1386,7 @@ pub mod tests {
 
         let font_cache = cx.font_cache();
         let family_id = font_cache
-            .load_family(&["Courier"], Default::default())
+            .load_family(&["Courier"], &Default::default())
             .unwrap();
         let font_id = font_cache
             .select_font(family_id, &Default::default())
@@ -1504,7 +1504,7 @@ pub mod tests {
         let buffer = MultiBuffer::build_simple(text, cx);
         let font_cache = cx.font_cache();
         let family_id = font_cache
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = font_cache
             .select_font(family_id, &Default::default())
@@ -1564,7 +1564,7 @@ pub mod tests {
         let buffer = MultiBuffer::build_simple("aaa\n\t\tbbb", cx);
         let font_cache = cx.font_cache();
         let family_id = font_cache
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = font_cache
             .select_font(family_id, &Default::default())

crates/editor/src/display_map/block_map.rs

@@ -1017,7 +1017,7 @@ mod tests {
 
         let family_id = cx
             .font_cache()
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = cx
             .font_cache()
@@ -1190,7 +1190,7 @@ mod tests {
 
         let family_id = cx
             .font_cache()
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = cx
             .font_cache()
@@ -1249,7 +1249,7 @@ mod tests {
         let tab_size = 1.try_into().unwrap();
         let family_id = cx
             .font_cache()
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = cx
             .font_cache()

crates/editor/src/display_map/wrap_map.rs

@@ -1054,7 +1054,7 @@ mod tests {
         };
         let tab_size = NonZeroU32::new(rng.gen_range(1..=4)).unwrap();
         let family_id = font_cache
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = font_cache
             .select_font(family_id, &Default::default())

crates/editor/src/editor.rs

@@ -6769,7 +6769,6 @@ fn build_style(
         }
     } else {
         let font_family_id = settings.buffer_font_family;
-        let font_features = settings.buffer_font_features;
         let font_family_name = cx.font_cache().family_name(font_family_id).unwrap();
         let font_properties = Default::default();
         let font_id = font_cache
@@ -6780,7 +6779,7 @@ fn build_style(
             text: TextStyle {
                 color: settings.theme.editor.text_color,
                 font_family_name,
-                font_features,
+                font_features: settings.buffer_font_features.clone(),
                 font_family_id,
                 font_id,
                 font_size,

crates/editor/src/movement.rs

@@ -589,7 +589,7 @@ mod tests {
         cx.set_global(Settings::test(cx));
         let family_id = cx
             .font_cache()
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = cx
             .font_cache()

crates/editor/src/test.rs

@@ -27,7 +27,7 @@ pub fn marked_display_snapshot(
 
     let family_id = cx
         .font_cache()
-        .load_family(&["Helvetica"], Default::default())
+        .load_family(&["Helvetica"], &Default::default())
         .unwrap();
     let font_id = cx
         .font_cache()

crates/gpui/examples/text.rs

@@ -58,7 +58,7 @@ impl gpui::Element for TextElement {
         let font_size = 12.;
         let family = cx
             .font_cache
-            .load_family(&["SF Pro Display"], Default::default())
+            .load_family(&["SF Pro Display"], &Default::default())
             .unwrap();
         let normal = RunStyle {
             font_id: cx

crates/gpui/src/font_cache.rs

@@ -59,21 +59,21 @@ impl FontCache {
             .map(|family| family.name.clone())
     }
 
-    pub fn load_family(&self, names: &[&str], features: Features) -> Result<FamilyId> {
+    pub fn load_family(&self, names: &[&str], features: &Features) -> Result<FamilyId> {
         for name in names {
             let state = self.0.upgradable_read();
 
             if let Some(ix) = state
                 .families
                 .iter()
-                .position(|f| f.name.as_ref() == *name && f.font_features == features)
+                .position(|f| f.name.as_ref() == *name && f.font_features == *features)
             {
                 return Ok(FamilyId(ix));
             }
 
             let mut state = RwLockUpgradableReadGuard::upgrade(state);
 
-            if let Ok(font_ids) = state.fonts.load_family(name, &features) {
+            if let Ok(font_ids) = state.fonts.load_family(name, features) {
                 if font_ids.is_empty() {
                     continue;
                 }
@@ -87,7 +87,7 @@ impl FontCache {
 
                 state.families.push(Family {
                     name: Arc::from(*name),
-                    font_features: features,
+                    font_features: features.clone(),
                     font_ids,
                 });
                 return Ok(family_id);
@@ -263,7 +263,7 @@ mod tests {
         let arial = fonts
             .load_family(
                 &["Arial"],
-                Features {
+                &Features {
                     calt: Some(false),
                     ..Default::default()
                 },
@@ -283,7 +283,7 @@ mod tests {
         let arial_with_calt = fonts
             .load_family(
                 &["Arial"],
-                Features {
+                &Features {
                     calt: Some(true),
                     ..Default::default()
                 },

crates/gpui/src/fonts.rs

@@ -21,7 +21,7 @@ pub struct FontId(pub usize);
 
 pub type GlyphId = u32;
 
-#[derive(Copy, Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize, JsonSchema)]
+#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize, JsonSchema)]
 pub struct Features {
     pub calt: Option<bool>,
     pub case: Option<bool>,
@@ -155,7 +155,7 @@ impl TextStyle {
         font_cache: &FontCache,
     ) -> Result<Self> {
         let font_family_name = font_family_name.into();
-        let font_family_id = font_cache.load_family(&[&font_family_name], font_features)?;
+        let font_family_id = font_cache.load_family(&[&font_family_name], &font_features)?;
         let font_id = font_cache.select_font(font_family_id, &font_properties)?;
         Ok(Self {
             color,
@@ -299,7 +299,7 @@ impl Default for TextStyle {
 
             let font_family_name = Arc::from("Courier");
             let font_family_id = font_cache
-                .load_family(&[&font_family_name], Default::default())
+                .load_family(&[&font_family_name], &Default::default())
                 .unwrap();
             let font_id = font_cache
                 .select_font(font_family_id, &Default::default())

crates/gpui/src/text_layout.rs

@@ -664,7 +664,7 @@ mod tests {
         let font_cache = cx.font_cache().clone();
         let font_system = cx.platform().fonts();
         let family = font_cache
-            .load_family(&["Courier"], Default::default())
+            .load_family(&["Courier"], &Default::default())
             .unwrap();
         let font_id = font_cache.select_font(family, &Default::default()).unwrap();
 
@@ -728,7 +728,7 @@ mod tests {
         let text_layout_cache = TextLayoutCache::new(font_system.clone());
 
         let family = font_cache
-            .load_family(&["Helvetica"], Default::default())
+            .load_family(&["Helvetica"], &Default::default())
             .unwrap();
         let font_id = font_cache.select_font(family, &Default::default()).unwrap();
         let normal = RunStyle {

crates/settings/src/settings.rs

@@ -406,7 +406,7 @@ impl Settings {
             buffer_font_family: font_cache
                 .load_family(
                     &[defaults.buffer_font_family.as_ref().unwrap()],
-                    buffer_font_features,
+                    &buffer_font_features,
                 )
                 .unwrap(),
             buffer_font_family_name: defaults.buffer_font_family.unwrap(),
@@ -473,7 +473,7 @@ impl Settings {
         }
         if family_changed {
             if let Some(id) = font_cache
-                .load_family(&[&self.buffer_font_family_name], self.buffer_font_features)
+                .load_family(&[&self.buffer_font_family_name], &self.buffer_font_features)
                 .log_err()
             {
                 self.buffer_font_family = id;
@@ -504,14 +504,6 @@ impl Settings {
         merge(&mut self.default_dock_anchor, data.default_dock_anchor);
         merge(&mut self.base_keymap, data.base_keymap);
 
-        // Ensure terminal font is loaded, so we can request it in terminal_element layout
-        if let Some(terminal_font) = &data.terminal.font_family {
-            // TODO: enable font features for the terminal as well.
-            font_cache
-                .load_family(&[terminal_font], Default::default())
-                .log_err();
-        }
-
         self.editor_overrides = data.editor;
         self.git_overrides = data.git.unwrap_or_default();
         self.journal_overrides = data.journal;
@@ -647,7 +639,7 @@ impl Settings {
             buffer_font_features: Default::default(),
             buffer_font_family: cx
                 .font_cache()
-                .load_family(&["Monaco"], Default::default())
+                .load_family(&["Monaco"], &Default::default())
                 .unwrap(),
             buffer_font_size: 14.,
             active_pane_magnification: 1.,

crates/terminal_view/src/terminal_element.rs

@@ -514,11 +514,12 @@ impl TerminalElement {
         let font_features = settings
             .terminal_overrides
             .font_features
-            .or(settings.terminal_defaults.font_features)
-            .unwrap_or(settings.buffer_font_features);
+            .as_ref()
+            .or(settings.terminal_defaults.font_features.as_ref())
+            .unwrap_or(&settings.buffer_font_features);
 
         let family_id = font_cache
-            .load_family(&[font_family_name], font_features)
+            .load_family(&[font_family_name], &font_features)
             .log_err()
             .unwrap_or(settings.buffer_font_family);
 
@@ -536,7 +537,7 @@ impl TerminalElement {
             color: settings.theme.editor.text_color,
             font_family_id: family_id,
             font_family_name: font_cache.family_name(family_id).unwrap(),
-            font_features,
+            font_features: font_features.clone(),
             font_id,
             font_size,
             font_properties: Default::default(),

crates/theme_testbench/src/theme_testbench.rs

@@ -243,7 +243,7 @@ impl ThemeTestbench {
             color: style.foreground,
             font_family_id: family_id,
             font_family_name: font_cache.family_name(family_id).unwrap(),
-            font_features: settings.buffer_font_features,
+            font_features: settings.buffer_font_features.clone(),
             font_id,
             font_size,
             font_properties: Default::default(),