Merge remote-tracking branch 'origin/main' into chat

Antonio Scandurra created

Change summary

zed/assets/themes/_base.toml |  32 
zed/assets/themes/dark.toml  |  18 
zed/assets/themes/light.toml |  18 
zed/src/theme.rs             | 659 ++++++++++++++++++++++++++++---------
zed/src/theme_selector.rs    |  11 
5 files changed, 544 insertions(+), 194 deletions(-)

Detailed changes

zed/assets/themes/_base.toml 🔗

@@ -1,9 +1,9 @@
 [ui]
-background = "$elevation_1"
+background = "$surfaces.1"
 
 [ui.tab]
-background = "$elevation_2"
-text = "$text_dull"
+background = "$surfaces.2"
+text = "$text_colors.dull"
 border = { color = "#000000", width = 1.0 }
 padding = { left = 10, right = 10 }
 icon_close = "#383839"
@@ -11,13 +11,13 @@ icon_dirty = "#556de8"
 icon_conflict = "#e45349"
 
 [ui.active_tab]
-extends = "ui.tab"
-background = "$elevation_3"
-text = "$text_bright"
+extends = "$ui.tab"
+background = "$surfaces.3"
+text = "$text_colors.bright"
 
 [ui.selector]
-background = "$elevation_4"
-text = "$text_bright"
+background = "$surfaces.4"
+text = "$text_colors.bright"
 padding = { top = 6.0, bottom = 6.0, left = 6.0, right = 6.0 }
 margin.top = 12.0
 corner_radius = 6.0
@@ -31,17 +31,17 @@ border = { color = "#000000", width = 1.0 }
 padding = { top = 6.0, bottom = 6.0, left = 6.0, right = 6.0 }
 
 [ui.selector.active_item]
-extends = "ui.selector.item"
+extends = "$ui.selector.item"
 background = "#094771"
 
 [editor]
-background = "$elevation_3"
-gutter_background = "$elevation_3"
-active_line_background = "$elevation_4"
-line_number = "$text_dull"
-line_number_active = "$text_bright"
-text = "$text_normal"
+background = "$surfaces.3"
+gutter_background = "$surfaces.3"
+active_line_background = "$surfaces.4"
+line_number = "$text_colors.dull"
+line_number_active = "$text_colors.bright"
+text = "$text_colors.normal"
 replicas = [
-    { selection = "#264f78", cursor = "$text_bright" },
+    { selection = "#264f78", cursor = "$text_colors.bright" },
     { selection = "#504f31", cursor = "#fcf154" },
 ]

zed/assets/themes/dark.toml 🔗

@@ -1,13 +1,15 @@
 extends = "_base"
 
-[variables]
-elevation_1 = "#050101"
-elevation_2 = "#131415"
-elevation_3 = "#1c1d1e"
-elevation_4 = "#3a3b3c"
-text_dull = "#5a5a5b"
-text_bright = "#ffffff"
-text_normal = "#d4d4d4"
+[surfaces]
+1 = "#050101"
+2 = "#131415"
+3 = "#1c1d1e"
+4 = "#3a3b3c"
+
+[text_colors]
+dull = "#5a5a5b"
+bright = "#ffffff"
+normal = "#d4d4d4"
 
 [syntax]
 keyword = { color = "#0086c0", weight = "bold" }

zed/assets/themes/light.toml 🔗

@@ -1,13 +1,15 @@
 extends = "_base"
 
-[variables]
-elevation_1 = "#ffffff"
-elevation_2 = "#f3f3f3"
-elevation_3 = "#ececec"
-elevation_4 = "#3a3b3c"
-text_dull = "#acacac"
-text_bright = "#111111"
-text_normal = "#333333"
+[surfaces]
+1 = "#ffffff"
+2 = "#f3f3f3"
+3 = "#ececec"
+4 = "#3a3b3c"
+
+[text_colors]
+dull = "#acacac"
+bright = "#111111"
+normal = "#333333"
 
 [syntax]
 keyword = "#0000fa"

zed/src/theme.rs 🔗

@@ -9,7 +9,7 @@ use json::{Map, Value};
 use parking_lot::Mutex;
 use serde::{Deserialize, Deserializer};
 use serde_json as json;
-use std::{cmp::Ordering, collections::HashMap, sync::Arc};
+use std::{collections::HashMap, fmt, mem, sync::Arc};
 
 const DEFAULT_HIGHLIGHT_ID: HighlightId = HighlightId(u32::MAX);
 pub const DEFAULT_THEME_NAME: &'static str = "dark";
@@ -91,6 +91,30 @@ pub struct SelectorItem {
     pub label: LabelStyle,
 }
 
+#[derive(Default)]
+struct KeyPathReferenceSet {
+    references: Vec<KeyPathReference>,
+    reference_ids_by_source: Vec<usize>,
+    reference_ids_by_target: Vec<usize>,
+    dependencies: Vec<(usize, usize)>,
+    dependency_counts: Vec<usize>,
+}
+
+#[derive(Clone, Default, PartialEq, Eq, PartialOrd, Ord)]
+struct KeyPathReference {
+    target: KeyPath,
+    source: KeyPath,
+}
+
+#[derive(Debug, Default, Clone, PartialEq, Eq, PartialOrd, Ord)]
+struct KeyPath(Vec<Key>);
+
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+enum Key {
+    Array(usize),
+    Object(String),
+}
+
 impl Default for Editor {
     fn default() -> Self {
         Self {
@@ -136,7 +160,7 @@ impl ThemeRegistry {
             return Ok(theme.clone());
         }
 
-        let theme_data = self.load(name)?;
+        let theme_data = self.load(name, true)?;
         let mut theme = serde_json::from_value::<Theme>(theme_data.as_ref().clone())?;
         theme.name = name.into();
         let theme = Arc::new(theme);
@@ -144,7 +168,7 @@ impl ThemeRegistry {
         Ok(theme)
     }
 
-    fn load(&self, name: &str) -> Result<Arc<Value>> {
+    fn load(&self, name: &str, evaluate_references: bool) -> Result<Arc<Value>> {
         if let Some(data) = self.theme_data.lock().get(name) {
             return Ok(data.clone());
         }
@@ -165,7 +189,7 @@ impl ThemeRegistry {
             .map(str::to_string)
         {
             let base_theme_data = self
-                .load(&base_name)
+                .load(&base_name, false)
                 .with_context(|| format!("failed to load base theme {}", base_name))?
                 .as_ref()
                 .clone();
@@ -175,44 +199,38 @@ impl ThemeRegistry {
             }
         }
 
-        // Evaluate `extends` fields in styles
-        // First, find the key paths of all objects with `extends` directives
-        let mut directives = Vec::new();
-        let mut key_path = Vec::new();
-        for (key, value) in theme_data.iter() {
-            if value.is_array() || value.is_object() {
-                key_path.push(Key::Object(key.clone()));
-                find_extensions(value, &mut key_path, &mut directives);
-                key_path.pop();
+        // Find all of the key path references in the object, and then sort them according
+        // to their dependencies.
+        if evaluate_references {
+            let mut key_path = KeyPath::default();
+            let mut references = KeyPathReferenceSet::default();
+            for (key, value) in theme_data.iter() {
+                key_path.0.push(Key::Object(key.clone()));
+                find_references(value, &mut key_path, &mut references);
+                key_path.0.pop();
             }
-        }
-        // If you extend something with an extend directive, process the source's extend directive first
-        directives.sort_unstable();
-
-        // Now update objects to include the fields of objects they extend
-        for ExtendDirective {
-            source_path,
-            target_path,
-        } in directives
-        {
-            let source = value_at(&mut theme_data, &source_path)?.clone();
-            let target = value_at(&mut theme_data, &target_path)?;
-            if let (Value::Object(mut source_object), Value::Object(target_object)) =
-                (source, target.take())
-            {
-                deep_merge_json(&mut source_object, target_object);
-                *target = Value::Object(source_object);
-            }
-        }
-
-        // Evaluate any variables
-        if let Some((key, variables)) = theme_data.remove_entry("variables") {
-            if let Some(variables) = variables.as_object() {
-                for value in theme_data.values_mut() {
-                    evaluate_variables(value, &variables, &mut Vec::new())?;
+            let sorted_references = references
+                .top_sort()
+                .map_err(|key_paths| anyhow!("cycle for key paths: {:?}", key_paths))?;
+
+            // Now update objects to include the fields of objects they extend
+            for KeyPathReference { source, target } in sorted_references {
+                if let Some(source) = value_at(&mut theme_data, &source).cloned() {
+                    let target = value_at(&mut theme_data, &target).unwrap();
+                    if let Value::Object(target_object) = target.take() {
+                        if let Value::Object(mut source_object) = source {
+                            deep_merge_json(&mut source_object, target_object);
+                            *target = Value::Object(source_object);
+                        } else {
+                            Err(anyhow!("extended key path {} is not an object", source))?;
+                        }
+                    } else {
+                        *target = source;
+                    }
+                } else {
+                    Err(anyhow!("invalid key path '{}'", source))?;
                 }
             }
-            theme_data.insert(key, variables);
         }
 
         let result = Arc::new(Value::Object(theme_data));
@@ -281,6 +299,293 @@ impl HighlightMap {
     }
 }
 
+impl KeyPathReferenceSet {
+    fn insert(&mut self, reference: KeyPathReference) {
+        let id = self.references.len();
+        let source_ix = self
+            .reference_ids_by_source
+            .binary_search_by_key(&&reference.source, |id| &self.references[*id].source)
+            .unwrap_or_else(|i| i);
+        let target_ix = self
+            .reference_ids_by_target
+            .binary_search_by_key(&&reference.target, |id| &self.references[*id].target)
+            .unwrap_or_else(|i| i);
+
+        self.populate_dependencies(id, &reference);
+        self.reference_ids_by_source.insert(source_ix, id);
+        self.reference_ids_by_target.insert(target_ix, id);
+        self.references.push(reference);
+    }
+
+    fn top_sort(mut self) -> Result<Vec<KeyPathReference>, Vec<KeyPath>> {
+        let mut results = Vec::with_capacity(self.references.len());
+        let mut root_ids = Vec::with_capacity(self.references.len());
+
+        // Find the initial set of references that have no dependencies.
+        for (id, dep_count) in self.dependency_counts.iter().enumerate() {
+            if *dep_count == 0 {
+                root_ids.push(id);
+            }
+        }
+
+        while results.len() < root_ids.len() {
+            // Just to guarantee a stable result when the inputs are randomized,
+            // sort references lexicographically in absence of any dependency relationship.
+            root_ids[results.len()..].sort_by_key(|id| &self.references[*id]);
+
+            let root_id = root_ids[results.len()];
+            let root = mem::take(&mut self.references[root_id]);
+            results.push(root);
+
+            // Remove this reference as a dependency from any of its dependent references.
+            if let Ok(dep_ix) = self
+                .dependencies
+                .binary_search_by_key(&root_id, |edge| edge.0)
+            {
+                let mut first_dep_ix = dep_ix;
+                let mut last_dep_ix = dep_ix + 1;
+                while first_dep_ix > 0 && self.dependencies[first_dep_ix - 1].0 == root_id {
+                    first_dep_ix -= 1;
+                }
+                while last_dep_ix < self.dependencies.len()
+                    && self.dependencies[last_dep_ix].0 == root_id
+                {
+                    last_dep_ix += 1;
+                }
+
+                // If any reference no longer has any dependencies, then then mark it as a root.
+                // Preserve the references' original order where possible.
+                for (_, successor_id) in self.dependencies.drain(first_dep_ix..last_dep_ix) {
+                    self.dependency_counts[successor_id] -= 1;
+                    if self.dependency_counts[successor_id] == 0 {
+                        root_ids.push(successor_id);
+                    }
+                }
+            }
+        }
+
+        // If any references never became roots, then there are reference cycles
+        // in the set. Return an error containing all of the key paths that are
+        // directly involved in cycles.
+        if results.len() < self.references.len() {
+            let mut cycle_ref_ids = (0..self.references.len())
+                .filter(|id| !root_ids.contains(id))
+                .collect::<Vec<_>>();
+
+            // Iteratively remove any references that have no dependencies,
+            // so that the error will only indicate which key paths are directly
+            // involved in the cycles.
+            let mut done = false;
+            while !done {
+                done = true;
+                cycle_ref_ids.retain(|id| {
+                    if self.dependencies.iter().any(|dep| dep.0 == *id) {
+                        true
+                    } else {
+                        done = false;
+                        self.dependencies.retain(|dep| dep.1 != *id);
+                        false
+                    }
+                });
+            }
+
+            let mut cycle_key_paths = Vec::new();
+            for id in cycle_ref_ids {
+                let reference = &self.references[id];
+                cycle_key_paths.push(reference.target.clone());
+                cycle_key_paths.push(reference.source.clone());
+            }
+            cycle_key_paths.sort_unstable();
+            return Err(cycle_key_paths);
+        }
+
+        Ok(results)
+    }
+
+    fn populate_dependencies(&mut self, new_id: usize, new_reference: &KeyPathReference) {
+        self.dependency_counts.push(0);
+
+        // If an existing reference's source path starts with the new reference's
+        // target path, then insert this new reference before that existing reference.
+        for id in Self::reference_ids_for_key_path(
+            &new_reference.target.0,
+            &self.references,
+            &self.reference_ids_by_source,
+            KeyPathReference::source,
+            KeyPath::starts_with,
+        ) {
+            Self::add_dependency(
+                (new_id, id),
+                &mut self.dependencies,
+                &mut self.dependency_counts,
+            );
+        }
+
+        // If an existing reference's target path starts with the new reference's
+        // source path, then insert this new reference after that existing reference.
+        for id in Self::reference_ids_for_key_path(
+            &new_reference.source.0,
+            &self.references,
+            &self.reference_ids_by_target,
+            KeyPathReference::target,
+            KeyPath::starts_with,
+        ) {
+            Self::add_dependency(
+                (id, new_id),
+                &mut self.dependencies,
+                &mut self.dependency_counts,
+            );
+        }
+
+        // If an existing reference's source path is a prefix of the new reference's
+        // target path, then insert this new reference before that existing reference.
+        for prefix in new_reference.target.prefixes() {
+            for id in Self::reference_ids_for_key_path(
+                prefix,
+                &self.references,
+                &self.reference_ids_by_source,
+                KeyPathReference::source,
+                PartialEq::eq,
+            ) {
+                Self::add_dependency(
+                    (new_id, id),
+                    &mut self.dependencies,
+                    &mut self.dependency_counts,
+                );
+            }
+        }
+
+        // If an existing reference's target path is a prefix of the new reference's
+        // source path, then insert this new reference after that existing reference.
+        for prefix in new_reference.source.prefixes() {
+            for id in Self::reference_ids_for_key_path(
+                prefix,
+                &self.references,
+                &self.reference_ids_by_target,
+                KeyPathReference::target,
+                PartialEq::eq,
+            ) {
+                Self::add_dependency(
+                    (id, new_id),
+                    &mut self.dependencies,
+                    &mut self.dependency_counts,
+                );
+            }
+        }
+    }
+
+    // Find all existing references that satisfy a given predicate with respect
+    // to a given key path. Use a sorted array of reference ids in order to avoid
+    // performing unnecessary comparisons.
+    fn reference_ids_for_key_path<'a>(
+        key_path: &[Key],
+        references: &[KeyPathReference],
+        sorted_reference_ids: &'a [usize],
+        reference_attribute: impl Fn(&KeyPathReference) -> &KeyPath,
+        predicate: impl Fn(&KeyPath, &[Key]) -> bool,
+    ) -> impl Iterator<Item = usize> + 'a {
+        let ix = sorted_reference_ids
+            .binary_search_by_key(&key_path, |id| &reference_attribute(&references[*id]).0)
+            .unwrap_or_else(|i| i);
+
+        let mut start_ix = ix;
+        while start_ix > 0 {
+            let reference_id = sorted_reference_ids[start_ix - 1];
+            let reference = &references[reference_id];
+            if !predicate(&reference_attribute(reference), key_path) {
+                break;
+            }
+            start_ix -= 1;
+        }
+
+        let mut end_ix = ix;
+        while end_ix < sorted_reference_ids.len() {
+            let reference_id = sorted_reference_ids[end_ix];
+            let reference = &references[reference_id];
+            if !predicate(&reference_attribute(reference), key_path) {
+                break;
+            }
+            end_ix += 1;
+        }
+
+        sorted_reference_ids[start_ix..end_ix].iter().copied()
+    }
+
+    fn add_dependency(
+        (predecessor, successor): (usize, usize),
+        dependencies: &mut Vec<(usize, usize)>,
+        dependency_counts: &mut Vec<usize>,
+    ) {
+        let dependency = (predecessor, successor);
+        if let Err(i) = dependencies.binary_search(&dependency) {
+            dependencies.insert(i, dependency);
+        }
+        dependency_counts[successor] += 1;
+    }
+}
+
+impl KeyPathReference {
+    fn source(&self) -> &KeyPath {
+        &self.source
+    }
+
+    fn target(&self) -> &KeyPath {
+        &self.target
+    }
+}
+
+impl KeyPath {
+    fn new(string: &str) -> Self {
+        Self(
+            string
+                .split(".")
+                .map(|key| Key::Object(key.to_string()))
+                .collect(),
+        )
+    }
+
+    fn starts_with(&self, other: &[Key]) -> bool {
+        self.0.starts_with(&other)
+    }
+
+    fn prefixes(&self) -> impl Iterator<Item = &[Key]> {
+        (1..self.0.len()).map(move |end_ix| &self.0[0..end_ix])
+    }
+}
+
+impl PartialEq<[Key]> for KeyPath {
+    fn eq(&self, other: &[Key]) -> bool {
+        self.0.eq(other)
+    }
+}
+
+impl fmt::Debug for KeyPathReference {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "KeyPathReference {{ {} <- {} }}",
+            self.target, self.source
+        )
+    }
+}
+
+impl fmt::Display for KeyPath {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for (i, key) in self.0.iter().enumerate() {
+            match key {
+                Key::Array(index) => write!(f, "[{}]", index)?,
+                Key::Object(key) => {
+                    if i > 0 {
+                        ".".fmt(f)?;
+                    }
+                    key.fmt(f)?;
+                }
+            }
+        }
+        Ok(())
+    }
+}
+
 impl Default for HighlightMap {
     fn default() -> Self {
         Self(Arc::new([]))
@@ -307,74 +612,45 @@ fn deep_merge_json(base: &mut Map<String, Value>, extension: Map<String, Value>)
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
-enum Key {
-    Array(usize),
-    Object(String),
-}
-
-#[derive(Debug, PartialEq, Eq)]
-struct ExtendDirective {
-    source_path: Vec<Key>,
-    target_path: Vec<Key>,
-}
-
-impl Ord for ExtendDirective {
-    fn cmp(&self, other: &Self) -> Ordering {
-        if self.target_path.starts_with(&other.source_path)
-            || other.source_path.starts_with(&self.target_path)
-        {
-            Ordering::Less
-        } else if other.target_path.starts_with(&self.source_path)
-            || self.source_path.starts_with(&other.target_path)
-        {
-            Ordering::Greater
-        } else {
-            Ordering::Equal
-        }
-    }
-}
-
-impl PartialOrd for ExtendDirective {
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-fn find_extensions(value: &Value, key_path: &mut Vec<Key>, directives: &mut Vec<ExtendDirective>) {
+fn find_references(value: &Value, key_path: &mut KeyPath, references: &mut KeyPathReferenceSet) {
     match value {
         Value::Array(vec) => {
             for (ix, value) in vec.iter().enumerate() {
-                key_path.push(Key::Array(ix));
-                find_extensions(value, key_path, directives);
-                key_path.pop();
+                key_path.0.push(Key::Array(ix));
+                find_references(value, key_path, references);
+                key_path.0.pop();
             }
         }
         Value::Object(map) => {
             for (key, value) in map.iter() {
                 if key == "extends" {
-                    if let Some(source_path) = value.as_str() {
-                        directives.push(ExtendDirective {
-                            source_path: source_path
-                                .split(".")
-                                .map(|key| Key::Object(key.to_string()))
-                                .collect(),
-                            target_path: key_path.clone(),
+                    if let Some(source_path) = value.as_str().and_then(|s| s.strip_prefix("$")) {
+                        references.insert(KeyPathReference {
+                            source: KeyPath::new(source_path),
+                            target: key_path.clone(),
                         });
                     }
-                } else if value.is_array() || value.is_object() {
-                    key_path.push(Key::Object(key.to_string()));
-                    find_extensions(value, key_path, directives);
-                    key_path.pop();
+                } else {
+                    key_path.0.push(Key::Object(key.to_string()));
+                    find_references(value, key_path, references);
+                    key_path.0.pop();
                 }
             }
         }
+        Value::String(string) => {
+            if let Some(source_path) = string.strip_prefix("$") {
+                references.insert(KeyPathReference {
+                    source: KeyPath::new(source_path),
+                    target: key_path.clone(),
+                });
+            }
+        }
         _ => {}
     }
 }
 
-fn value_at<'a>(object: &'a mut Map<String, Value>, key_path: &Vec<Key>) -> Result<&'a mut Value> {
-    let mut key_path = key_path.iter();
+fn value_at<'a>(object: &'a mut Map<String, Value>, key_path: &KeyPath) -> Option<&'a mut Value> {
+    let mut key_path = key_path.0.iter();
     if let Some(Key::Object(first_key)) = key_path.next() {
         let mut cur_value = object.get_mut(first_key);
         for key in key_path {
@@ -384,63 +660,15 @@ fn value_at<'a>(object: &'a mut Map<String, Value>, key_path: &Vec<Key>) -> Resu
                     Key::Object(key) => cur_value = value.get_mut(key),
                 }
             } else {
-                return Err(anyhow!("invalid key path"));
+                return None;
             }
         }
-        cur_value.ok_or_else(|| anyhow!("invalid key path"))
+        cur_value
     } else {
-        Err(anyhow!("invalid key path"))
+        None
     }
 }
 
-fn evaluate_variables(
-    value: &mut Value,
-    variables: &Map<String, Value>,
-    stack: &mut Vec<String>,
-) -> Result<()> {
-    match value {
-        Value::String(s) => {
-            if let Some(name) = s.strip_prefix("$") {
-                if stack.iter().any(|e| e == name) {
-                    Err(anyhow!("variable {} is defined recursively", name))?;
-                }
-                if validate_variable_name(name) {
-                    stack.push(name.to_string());
-                    if let Some(definition) = variables.get(name).cloned() {
-                        *value = definition;
-                        evaluate_variables(value, variables, stack)?;
-                    }
-                    stack.pop();
-                }
-            }
-        }
-        Value::Array(a) => {
-            for value in a.iter_mut() {
-                evaluate_variables(value, variables, stack)?;
-            }
-        }
-        Value::Object(object) => {
-            for value in object.values_mut() {
-                evaluate_variables(value, variables, stack)?;
-            }
-        }
-        _ => {}
-    }
-    Ok(())
-}
-
-fn validate_variable_name(name: &str) -> bool {
-    let mut chars = name.chars();
-    if let Some(first) = chars.next() {
-        if first.is_alphabetic() || first == '_' {
-            if chars.all(|c| c.is_alphanumeric() || c == '_') {
-                return true;
-            }
-        }
-    }
-    false
-}
-
 pub fn deserialize_syntax_theme<'de, D>(
     deserializer: D,
 ) -> Result<Vec<(String, TextStyle)>, D::Error>
@@ -463,9 +691,10 @@ where
 
 #[cfg(test)]
 mod tests {
-    use crate::assets::Assets;
+    use rand::{prelude::StdRng, Rng};
 
     use super::*;
+    use crate::assets::Assets;
 
     #[test]
     fn test_bundled_themes() {
@@ -488,13 +717,13 @@ mod tests {
                 "themes/_base.toml",
                 r##"
                 [ui.active_tab]
-                extends = "ui.tab"
+                extends = "$ui.tab"
                 border.color = "#666666"
-                text = "$bright_text"
+                text = "$text_colors.bright"
 
                 [ui.tab]
-                extends = "ui.element"
-                text = "$dull_text"
+                extends = "$ui.element"
+                text = "$text_colors.dull"
 
                 [ui.element]
                 background = "#111111"
@@ -502,7 +731,7 @@ mod tests {
 
                 [editor]
                 background = "#222222"
-                default_text = "$regular_text"
+                default_text = "$text_colors.regular"
                 "##,
             ),
             (
@@ -510,10 +739,10 @@ mod tests {
                 r##"
                 extends = "_base"
 
-                [variables]
-                bright_text = "#ffffff"
-                regular_text = "#eeeeee"
-                dull_text = "#dddddd"
+                [text_colors]
+                bright = "#ffffff"
+                regular = "#eeeeee"
+                dull = "#dddddd"
 
                 [editor]
                 background = "#232323"
@@ -522,7 +751,7 @@ mod tests {
         ]);
 
         let registry = ThemeRegistry::new(assets);
-        let theme_data = registry.load("light").unwrap();
+        let theme_data = registry.load("light", true).unwrap();
         assert_eq!(
             theme_data.as_ref(),
             &serde_json::json!({
@@ -533,7 +762,7 @@ mod tests {
                     "width": 2.0,
                     "color": "#666666"
                   },
-                  "extends": "ui.tab",
+                  "extends": "$ui.tab",
                   "text": "#ffffff"
                 },
                 "tab": {
@@ -542,7 +771,7 @@ mod tests {
                     "width": 2.0,
                     "color": "#00000000"
                   },
-                  "extends": "ui.element",
+                  "extends": "$ui.element",
                   "text": "#dddddd"
                 },
                 "element": {
@@ -558,10 +787,10 @@ mod tests {
                 "default_text": "#eeeeee"
               },
               "extends": "_base",
-              "variables": {
-                "bright_text": "#ffffff",
-                "regular_text": "#eeeeee",
-                "dull_text": "#dddddd"
+              "text_colors": {
+                "bright": "#ffffff",
+                "regular": "#eeeeee",
+                "dull": "#dddddd"
               }
             })
         );
@@ -598,6 +827,120 @@ mod tests {
         assert_eq!(theme.highlight_name(map.get(2)), Some("variable.builtin"));
     }
 
+    #[test]
+    fn test_key_path_reference_set_simple() {
+        let input_references = build_refs(&[
+            ("r", "a"),
+            ("a.b.c", "d"),
+            ("d.e", "f"),
+            ("t.u", "v"),
+            ("v.w", "x"),
+            ("v.y", "x"),
+            ("d.h", "i"),
+            ("v.z", "x"),
+            ("f.g", "d.h"),
+        ]);
+        let expected_references = build_refs(&[
+            ("d.h", "i"),
+            ("f.g", "d.h"),
+            ("d.e", "f"),
+            ("a.b.c", "d"),
+            ("r", "a"),
+            ("v.w", "x"),
+            ("v.y", "x"),
+            ("v.z", "x"),
+            ("t.u", "v"),
+        ])
+        .collect::<Vec<_>>();
+
+        let mut reference_set = KeyPathReferenceSet::default();
+        for reference in input_references {
+            reference_set.insert(reference);
+        }
+        assert_eq!(reference_set.top_sort().unwrap(), expected_references);
+    }
+
+    #[test]
+    fn test_key_path_reference_set_with_cycles() {
+        let input_references = build_refs(&[
+            ("x", "a.b"),
+            ("y", "x.c"),
+            ("a.b.c", "d.e"),
+            ("d.e.f", "g.h"),
+            ("g.h.i", "a"),
+        ]);
+
+        let mut reference_set = KeyPathReferenceSet::default();
+        for reference in input_references {
+            reference_set.insert(reference);
+        }
+
+        assert_eq!(
+            reference_set.top_sort().unwrap_err(),
+            &[
+                KeyPath::new("a"),
+                KeyPath::new("a.b.c"),
+                KeyPath::new("d.e"),
+                KeyPath::new("d.e.f"),
+                KeyPath::new("g.h"),
+                KeyPath::new("g.h.i"),
+            ]
+        );
+    }
+
+    #[gpui::test(iterations = 20)]
+    async fn test_key_path_reference_set_random(mut rng: StdRng) {
+        let examples: &[&[_]] = &[
+            &[
+                ("n.d.h", "i"),
+                ("f.g", "n.d.h"),
+                ("n.d.e", "f"),
+                ("a.b.c", "n.d"),
+                ("r", "a"),
+                ("q.q.q", "r.s"),
+                ("r.t", "q"),
+                ("x.x", "r.r"),
+                ("v.w", "x"),
+                ("v.y", "x"),
+                ("v.z", "x"),
+                ("t.u", "v"),
+            ],
+            &[
+                ("w.x.y.z", "t.u.z"),
+                ("x", "w.x"),
+                ("a.b.c1", "x.b1.c"),
+                ("a.b.c2", "x.b2.c"),
+            ],
+            &[
+                ("x.y", "m.n.n.o.q"),
+                ("x.y.z", "m.n.n.o.p"),
+                ("u.v.w", "x.y.z"),
+                ("a.b.c.d", "u.v"),
+                ("a.b.c.d.e", "u.v"),
+                ("a.b.c.d.f", "u.v"),
+                ("a.b.c.d.g", "u.v"),
+            ],
+        ];
+
+        for example in examples {
+            let expected_references = build_refs(example).collect::<Vec<_>>();
+            let mut input_references = expected_references.clone();
+            input_references.sort_by_key(|_| rng.gen_range(0..1000));
+            let mut reference_set = KeyPathReferenceSet::default();
+            for reference in input_references {
+                reference_set.insert(reference);
+            }
+            assert_eq!(reference_set.top_sort().unwrap(), expected_references);
+        }
+    }
+
+    fn build_refs<'a>(rows: &'a [(&str, &str)]) -> impl Iterator<Item = KeyPathReference> + 'a {
+        rows.iter().map(|(target, source)| KeyPathReference {
+            target: KeyPath::new(target),
+            source: KeyPath::new(source),
+        })
+    }
+
     struct TestAssets(&'static [(&'static str, &'static str)]);
 
     impl AssetSource for TestAssets {

zed/src/theme_selector.rs 🔗

@@ -107,10 +107,13 @@ impl ThemeSelector {
 
     fn confirm(&mut self, _: &(), cx: &mut ViewContext<Self>) {
         if let Some(mat) = self.matches.get(self.selected_index) {
-            if let Ok(theme) = self.registry.get(&mat.string) {
-                self.settings_tx.lock().borrow_mut().theme = theme;
-                cx.notify_all();
-                cx.emit(Event::Dismissed);
+            match self.registry.get(&mat.string) {
+                Ok(theme) => {
+                    self.settings_tx.lock().borrow_mut().theme = theme;
+                    cx.notify_all();
+                    cx.emit(Event::Dismissed);
+                }
+                Err(error) => log::error!("error loading theme {}: {}", mat.string, error),
             }
         }
     }