keymap: settle the public API before release

Outcome, ConflictMode, ProfileError and Options are non_exhaustive, with Options::with for
options built in one expression. Profiles keeps its fields private behind active() and sets(),
lists the sets loading dropped in problems(), and stores a format version (a store without one
reads as version 1).

crates/keymap/README.md

@@ -163,7 +163,8 @@ let keymap = Keymap::new(catalog, Scopes::new(), Platform::current()).with_overr
 store.update(|p| p.shortcut_sets = sets.clone());
 ```
 
-- `names`, `get`, `active_name`, `active_overrides` (hand them to `Keymap::set_overrides`).
+- `names`, `get`, `active_name`, `active_overrides` (hand them to `Keymap::set_overrides`);
+  `sets` (the user's sets) and `active` (the stored name, which `active_name` resolves to a set).
 - `select` switches sets; `duplicate(from, to)` copies a built-in or user set; `save_as`,
   `rename` and `delete` work on user sets only.
 - `store(overrides, builtins, copy_name)` saves the editor's result to the active set. Editing a
@@ -173,6 +174,10 @@ store.update(|p| p.shortcut_sets = sets.clone());
 - `export(name)` writes one set as JSON
   (`{"format":"craft-keymap","version":1,"name":…,"overrides":…}`); `import(text)` reads it into
   a new set. A file from a newer version is refused (`ProfileError::NewerVersion`).
+- Loading never fails. `problems()` lists what loading dropped: sets with a bad or repeated name,
+  overrides that are neither a list nor a map, sets past `MAX_SETS`, and a `version` newer than
+  this crate writes. The app shows them before the next save writes the sets without them.
+  Stored `Profiles` carry `version = 1`; a file without it reads as version 1.
 - `import_plan(&plan, &keymap, name)` applies an import plan (see Importing a shortcut set) to a
   copy of the keymap and keeps the result as a new set, so another application's set becomes one
   the user can switch to and away from.

crates/keymap/src/lib.rs

@@ -157,5 +157,5 @@ pub use keymap::{
     AssignError, Assigned, Bindable, Binding, BindingError, Conflict, ConflictKind, DropReason, Holder, Keymap, Kind, Lookup, MAX_BINDING_TEXT,
     MAX_BINDINGS, MAX_COMMANDS, MAX_CONFLICTS, Overrides, Policy, Problem, TypingRule,
 };
-pub use profiles::{Builtins, MAX_EXPORT_BYTES, MAX_SET_NAME, MAX_SETS, ProfileError, Profiles};
+pub use profiles::{Builtins, MAX_EXPORT_BYTES, MAX_SET_NAME, MAX_SETS, ProfileError, Profiles, SetProblem};
 pub use scope::{Active, MAX_SCOPE_NAME, Scope, ScopeKind, Scopes};

crates/keymap/src/profiles.rs

@@ -19,6 +19,7 @@ use std::collections::BTreeMap;
 use std::fmt;
 
 use serde::de::{self, IgnoredAny, MapAccess, SeqAccess, Visitor};
+use serde::ser::SerializeStruct;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use serde_json::{Value, json};
 
@@ -34,6 +35,10 @@ pub const MAX_EXPORT_BYTES: usize = 4 << 20;
 
 const FORMAT: &str = "craft-keymap";
 const VERSION: u64 = 1;
+/// The version stored [`Profiles`] carry; stored profiles without one read as version 1.
+const STORE_VERSION: u64 = 1;
+// Load notes are kept for `Profiles::problems`; a hostile file must not grow them without bound.
+const MAX_NOTES: usize = 64;
 const IMPORTED: &str = "Imported";
 
 /// The app's built-in sets: name and overrides against the catalog defaults.
@@ -42,6 +47,7 @@ pub type Builtins<'a> = [(&'a str, Overrides)];
 /// Why a set operation was refused.
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
+#[non_exhaustive]
 pub enum ProfileError {
     /// Empty, over [`MAX_SET_NAME`] chars, or with control characters.
     BadName,
@@ -80,23 +86,51 @@ impl fmt::Display for ProfileError {
 
 impl std::error::Error for ProfileError {}
 
+/// Something loading dropped from stored [`Profiles`]; [`Profiles::problems`] lists them.
+#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+#[non_exhaustive]
+pub enum SetProblem {
+    /// A set without a valid name: none, empty, over [`MAX_SET_NAME`] chars, with control
+    /// characters or with spaces around it. `name` is what was stored, cut to [`MAX_SET_NAME`]
+    /// chars.
+    BadName { name: String },
+    /// A second set of this name; the first one stays.
+    Duplicate { name: String },
+    /// Overrides that are neither a list nor a map.
+    BadOverrides { name: String },
+    /// Sets past [`MAX_SETS`].
+    TooMany { ignored: usize },
+    /// Stored by a newer craft-keymap; what this version reads is loaded.
+    NewerVersion(u64),
+}
+
 /// The user's sets and which set is active. The built-in sets are passed to each call.
 ///
-/// An `active` name that matches no set means the first built-in set (or no overrides when the app
+/// An active name that matches no set means the first built-in set (or no overrides when the app
 /// has none).
 ///
 /// A user set may carry a built-in's name, for instance after an update adds that built-in. Until
 /// [`Profiles::reconcile`] renames it, the user set is the one that name reads and writes. Call
 /// `reconcile` after loading; every method that changes the profiles calls it first.
-#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
-#[serde(default)]
+///
+/// Stored as `{ version, active, sets }`. Loading never fails: what it drops is listed by
+/// [`Profiles::problems`], and the next save writes the profiles without it.
+#[derive(Clone, Debug, Default)]
 pub struct Profiles {
-    #[serde(deserialize_with = "lenient_name")]
-    pub active: String,
-    #[serde(serialize_with = "sets_as_list", deserialize_with = "capped_sets")]
-    pub sets: BTreeMap<String, Overrides>,
+    active: String,
+    sets: BTreeMap<String, Overrides>,
+    notes: Vec<SetProblem>,
 }
 
+impl PartialEq for Profiles {
+    fn eq(&self, other: &Profiles) -> bool {
+        self.active == other.active && self.sets == other.sets
+    }
+}
+
+impl Eq for Profiles {}
+
 static NO_OVERRIDES: Overrides = Overrides::new();
 
 /// A name as given, trimmed; `None` when empty, too long or with control characters.
@@ -107,6 +141,22 @@ pub fn set_name(name: &str) -> Option<String> {
 }
 
 impl Profiles {
+    /// The stored active name, which may match no set; [`active_name`](Self::active_name) is the
+    /// set in use.
+    pub fn active(&self) -> &str {
+        &self.active
+    }
+
+    /// The user's sets.
+    pub fn sets(&self) -> &BTreeMap<String, Overrides> {
+        &self.sets
+    }
+
+    /// What loading dropped.
+    pub fn problems(&self) -> &[SetProblem] {
+        &self.notes
+    }
+
     /// Built-in names first, then the user's sets.
     pub fn names(&self, builtins: &Builtins) -> Vec<String> {
         builtins.iter().map(|(n, _)| n.to_string()).chain(self.sets.keys().cloned()).collect()
@@ -345,25 +395,105 @@ impl Profiles {
     }
 }
 
+impl Serialize for Profiles {
+    fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        let mut s = serializer.serialize_struct("Profiles", 3)?;
+        s.serialize_field("version", &STORE_VERSION)?;
+        s.serialize_field("active", &self.active)?;
+        s.serialize_field("sets", &SetList(&self.sets))?;
+        s.end()
+    }
+}
+
+impl<'de> Deserialize<'de> for Profiles {
+    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Profiles, D::Error> {
+        #[derive(Deserialize, Default)]
+        #[serde(default)]
+        struct Stored {
+            #[serde(deserialize_with = "lenient_version")]
+            version: Option<u64>,
+            #[serde(deserialize_with = "lenient_name")]
+            active: String,
+            #[serde(deserialize_with = "capped_sets")]
+            sets: Loaded,
+        }
+        let Stored { version, active, sets: Loaded { sets, mut notes, .. } } = Stored::deserialize(deserializer)?;
+        if let Some(v) = version.filter(|v| *v > STORE_VERSION) {
+            notes.insert(0, SetProblem::NewerVersion(v));
+            notes.truncate(MAX_NOTES);
+        }
+        Ok(Profiles { active, sets, notes })
+    }
+}
+
 fn lenient_name<'de, D: Deserializer<'de>>(deserializer: D) -> Result<String, D::Error> {
     let v = Value::deserialize(deserializer)?;
     Ok(v.as_str().and_then(set_name).unwrap_or_default())
 }
 
+fn lenient_version<'de, D: Deserializer<'de>>(deserializer: D) -> Result<Option<u64>, D::Error> {
+    Ok(Value::deserialize(deserializer)?.as_u64())
+}
+
 // An array of `{ name, overrides }` rather than a table, and each set's overrides in their array
 // form, so that craft-settings (which replaces arrays whole) drops a deleted set or entry from the
 // file.
-fn sets_as_list<S: Serializer>(sets: &BTreeMap<String, Overrides>, serializer: S) -> Result<S::Ok, S::Error> {
-    #[derive(Serialize)]
-    struct Set<'a> {
-        name: &'a str,
-        #[serde(serialize_with = "crate::keymap::overrides_list::serialize")]
-        overrides: &'a Overrides,
+struct SetList<'a>(&'a BTreeMap<String, Overrides>);
+
+impl Serialize for SetList<'_> {
+    fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        #[derive(Serialize)]
+        struct Set<'a> {
+            name: &'a str,
+            #[serde(serialize_with = "crate::keymap::overrides_list::serialize")]
+            overrides: &'a Overrides,
+        }
+        serializer.collect_seq(self.0.iter().map(|(name, overrides)| Set { name, overrides }))
     }
-    serializer.collect_seq(sets.iter().map(|(name, overrides)| Set { name, overrides }))
 }
 
-fn capped_sets<'de, D: Deserializer<'de>>(deserializer: D) -> Result<BTreeMap<String, Overrides>, D::Error> {
+/// The sets that loaded, and notes on the ones that did not.
+#[derive(Default)]
+struct Loaded {
+    sets: BTreeMap<String, Overrides>,
+    notes: Vec<SetProblem>,
+    ignored: usize,
+}
+
+impl Loaded {
+    fn note(&mut self, problem: SetProblem) {
+        if self.notes.len() < MAX_NOTES {
+            self.notes.push(problem);
+        }
+    }
+
+    /// Keeps a set with a valid name, stored as written, that is new and fits under [`MAX_SETS`];
+    /// notes why not otherwise.
+    fn add(&mut self, name: &str, overrides: Option<Overrides>) {
+        let shown = || name.chars().take(MAX_SET_NAME).collect::<String>();
+        if !set_name(name).is_some_and(|n| n == name) {
+            self.note(SetProblem::BadName { name: shown() });
+        } else if self.sets.contains_key(name) {
+            self.note(SetProblem::Duplicate { name: shown() });
+        } else if self.sets.len() >= MAX_SETS {
+            self.ignored += 1;
+        } else if let Some(o) = overrides {
+            self.sets.insert(name.to_string(), o);
+        } else {
+            self.note(SetProblem::BadOverrides { name: shown() });
+        }
+    }
+
+    fn done(mut self) -> Loaded {
+        if self.ignored > 0 {
+            let ignored = std::mem::take(&mut self.ignored);
+            self.note(SetProblem::TooMany { ignored });
+        }
+        self
+    }
+}
+
+fn capped_sets<'de, D: Deserializer<'de>>(deserializer: D) -> Result<Loaded, D::Error> {
     deserializer.deserialize_any(SetsVisitor)
 }
 
@@ -373,13 +503,13 @@ struct SetsVisitor;
 macro_rules! no_sets {
     ($($name:ident: $t:ty),*) => {
         $(fn $name<E: de::Error>(self, _: $t) -> Result<Self::Value, E> {
-            Ok(BTreeMap::new())
+            Ok(Loaded::default())
         })*
     };
 }
 
 impl<'de> Visitor<'de> for SetsVisitor {
-    type Value = BTreeMap<String, Overrides>;
+    type Value = Loaded;
 
     fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.write_str("a list of `{ name, overrides }` items or a map of set names to overrides")
@@ -388,11 +518,11 @@ impl<'de> Visitor<'de> for SetsVisitor {
     no_sets!(visit_bool: bool, visit_i64: i64, visit_u64: u64, visit_i128: i128, visit_u128: u128, visit_f64: f64, visit_str: &str, visit_bytes: &[u8], visit_char: char);
 
     fn visit_unit<E: de::Error>(self) -> Result<Self::Value, E> {
-        Ok(BTreeMap::new())
+        Ok(Loaded::default())
     }
 
     fn visit_none<E: de::Error>(self) -> Result<Self::Value, E> {
-        Ok(BTreeMap::new())
+        Ok(Loaded::default())
     }
 
     fn visit_some<D: Deserializer<'de>>(self, deserializer: D) -> Result<Self::Value, D::Error> {
@@ -400,39 +530,29 @@ impl<'de> Visitor<'de> for SetsVisitor {
     }
 
     fn visit_seq<A: SeqAccess<'de>>(self, mut access: A) -> Result<Self::Value, A::Error> {
-        let mut out = BTreeMap::new();
+        let mut out = Loaded::default();
         while let Some(item) = access.next_element::<Value>()? {
-            let Some(name) = item.get("name").and_then(Value::as_str) else { continue };
-            if !admits(&out, name) {
-                continue;
-            }
-            if let Some(o) = item.get("overrides").and_then(overrides_of) {
-                out.insert(name.to_string(), o);
-            }
+            let name = item.get("name").and_then(Value::as_str).unwrap_or_default();
+            out.add(name, item.get("overrides").and_then(overrides_of));
         }
-        Ok(out)
+        Ok(out.done())
     }
 
     fn visit_map<A: MapAccess<'de>>(self, mut access: A) -> Result<Self::Value, A::Error> {
-        let mut out = BTreeMap::new();
+        let mut out = Loaded::default();
         while let Some(key) = access.next_key::<String>()? {
-            if admits(&out, &key) {
-                if let Some(o) = overrides_of(&access.next_value::<Value>()?) {
-                    out.insert(key, o);
-                }
-            } else {
+            if out.sets.len() >= MAX_SETS && !out.sets.contains_key(&key) {
                 access.next_value::<IgnoredAny>()?;
+                out.ignored += 1;
+            } else {
+                let overrides = overrides_of(&access.next_value::<Value>()?);
+                out.add(&key, overrides);
             }
         }
-        Ok(out)
+        Ok(out.done())
     }
 }
 
-/// A valid name, stored as written, with room under [`MAX_SETS`].
-fn admits(sets: &BTreeMap<String, Overrides>, name: &str) -> bool {
-    set_name(name).is_some_and(|n| n == name) && (sets.len() < MAX_SETS || sets.contains_key(name))
-}
-
 /// A set's overrides: a list or a map, anything else is no set.
 fn overrides_of(v: &Value) -> Option<Overrides> {
     match v {

crates/keymap/src/ui/mod.rs

@@ -31,6 +31,7 @@ use state::Env;
 
 /// What the dialog reports for a frame.
 #[derive(Clone, Debug)]
+#[non_exhaustive]
 pub enum Outcome {
     /// Closed with OK: save `Profiles` (with sets) or `keymap.overrides()`.
     Ok,
@@ -55,6 +56,7 @@ pub enum Outcome {
 
 /// What happens when a recorded chord is held by other commands.
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
+#[non_exhaustive]
 pub enum ConflictMode {
     /// Ask with the conflict bar: Take It, Take It and Go to Conflict, Keep Both, Cancel.
     #[default]
@@ -97,7 +99,9 @@ pub type Accept = Box<dyn Fn(&Bindable, &Binding) -> Result<(), String>>;
 /// Called for every control once per frame with (automation id, rect, label).
 pub type OnWidget = Box<dyn Fn(&str, egui::Rect, &str)>;
 
-/// How the dialog looks and behaves. [`Options::default`] gives the defaults noted on each field.
+/// How the dialog looks and behaves. [`Options::default`] gives the defaults noted on each field;
+/// set fields on it, or with [`Options::with`].
+#[non_exhaustive]
 pub struct Options {
     /// "Keyboard Shortcuts".
     pub title: &'static str,
@@ -160,6 +164,15 @@ impl Default for Options {
     }
 }
 
+impl Options {
+    /// `self` with `f` applied, for options built in one expression:
+    /// `Options::default().with(|o| o.keyboard = true)`.
+    pub fn with(mut self, f: impl FnOnce(&mut Options)) -> Options {
+        f(&mut self);
+        self
+    }
+}
+
 /// The format id of craft-keymap's own JSON in [`Outcome::Import`].
 pub const JSON_FORMAT: &str = "craft-keymap";
 

crates/keymap/src/ui/state.rs

@@ -1354,21 +1354,21 @@ mod tests {
         st.new_set(&mut fx.env(), "Mine", None);
         record(&mut st, &mut fx, add("object.front"), &["F2"]);
         st.save_set(&mut fx.env());
-        let kept = fx.p.sets["Mine"].clone();
+        let kept = fx.p.sets()["Mine"].clone();
         assert!(!kept.is_empty());
         st.switch_set(&mut fx.env(), "Fixture Defaults");
         for name in ["Mine", " mine ", "fixture defaults"] {
             for based_on in [None, Some("Fixture Defaults")] {
                 st.new_set(&mut fx.env(), name, based_on);
                 assert!(st.message.as_ref().is_some_and(|m| m.refused), "{name:?} from {based_on:?}");
-                assert_eq!((fx.p.sets.len(), fx.p.active.as_str()), (1, "Fixture Defaults"));
-                assert_eq!(fx.p.sets["Mine"], kept);
+                assert_eq!((fx.p.sets().len(), fx.p.active()), (1, "Fixture Defaults"));
+                assert_eq!(fx.p.sets()["Mine"], kept);
             }
         }
         st.new_set(&mut fx.env(), "Other", None);
         st.rename_set(&mut fx.env(), "MINE");
         assert!(st.message.as_ref().is_some_and(|m| m.refused));
-        assert_eq!(fx.p.sets["Mine"], kept);
+        assert_eq!(fx.p.sets()["Mine"], kept);
     }
 
     #[test]
@@ -1379,11 +1379,11 @@ mod tests {
         let mut st = open(&mut fx);
         record(&mut st, &mut fx, add("object.front"), &["F2"]);
         st.new_set(&mut fx.env(), "Other", None);
-        assert_eq!(fx.p.active, "Other");
+        assert_eq!(fx.p.active(), "Other");
         st.ask_delete_set(&fx.env());
         assert_eq!(st.prompt, Some(Prompt::DeleteSet("Other".into())));
         st.answer(&mut fx.env(), Answer::Yes);
-        assert!(!fx.p.sets.contains_key("Other"));
+        assert!(!fx.p.sets().contains_key("Other"));
         st.cancel(&mut fx.env());
         assert_eq!((fx.km.overrides(), &fx.p), (&km0, &p0));
     }
@@ -1397,13 +1397,13 @@ mod tests {
         st.ask_delete_set(&fx.env());
         assert!(st.modal());
         st.switch_set(&mut fx.env(), "Mine");
-        assert_eq!((st.prompt.clone(), fx.p.active.as_str()), (None, "Mine"), "switching closes the prompt");
+        assert_eq!((st.prompt.clone(), fx.p.active()), (None, "Mine"), "switching closes the prompt");
 
         record(&mut st, &mut fx, add("object.front"), &["F2"]);
         st.prompt = Some(Prompt::DeleteSet("Other".into()));
         st.answer(&mut fx.env(), Answer::Yes);
-        assert!(!fx.p.sets.contains_key("Other"));
-        assert_eq!((fx.p.active.as_str(), fx.keys("object.front")), ("Mine", vec!["F2".to_string()]), "the active set keeps its edits");
+        assert!(!fx.p.sets().contains_key("Other"));
+        assert_eq!((fx.p.active(), fx.keys("object.front")), ("Mine", vec!["F2".to_string()]), "the active set keeps its edits");
         assert!(st.can_undo());
     }
 
@@ -1419,9 +1419,9 @@ mod tests {
         st.ask_delete_set(&fx.env());
         st.answer(&mut fx.env(), Answer::Yes);
         assert!(st.message.is_none(), "{:?}", st.message);
-        assert!(fx.p.sets.is_empty() && fx.km.overrides().is_empty() && !st.can_undo());
+        assert!(fx.p.sets().is_empty() && fx.km.overrides().is_empty() && !st.can_undo());
         st.ok(&mut fx.env());
-        assert!(fx.p.sets.is_empty(), "OK stores no copy of the deleted set");
+        assert!(fx.p.sets().is_empty(), "OK stores no copy of the deleted set");
     }
 
     #[test]
@@ -1432,7 +1432,7 @@ mod tests {
         assert!(DialogState::is_modified(&fx.env()));
         st.ok(&mut fx.env());
         assert!(matches!(st.finish(), Some(Outcome::Ok)));
-        assert_eq!(fx.p.active, "Custom");
+        assert_eq!(fx.p.active(), "Custom");
         assert_eq!(fx.p.get("Custom", &fx.b).and_then(|o| o.get("object.front")), Some(&["F2".to_string()][..]));
     }
 
@@ -1445,7 +1445,7 @@ mod tests {
         st.switch_set(&mut fx.env(), "Fixture Defaults");
         assert!(matches!(&st.prompt, Some(Prompt::SaveChanges { set, .. }) if set == "Mine"));
         st.answer(&mut fx.env(), Answer::No);
-        assert_eq!(fx.p.active, "Fixture Defaults");
+        assert_eq!(fx.p.active(), "Fixture Defaults");
         assert!(fx.km.overrides().is_empty() && !st.can_undo(), "the set loads with a fresh undo history");
         assert!(fx.p.get("Mine", &fx.b).is_some_and(Overrides::is_empty), "Don't Save keeps the stored set");
     }
@@ -1466,7 +1466,7 @@ mod tests {
         let options = ImportOptions { contexts: vec![("Default".into(), ContextRule::Home)], ..ImportOptions::default() };
         st.review_import(&mut fx.env(), set, options);
         st.import_reviewed(&mut fx.env());
-        assert_eq!(fx.p.active, "Theirs");
+        assert_eq!(fx.p.active(), "Theirs");
         assert_eq!(fx.p.get("Theirs", &fx.b).and_then(|o| o.get("object.front")), None);
         assert!(fx.p.get("Mine", &fx.b).is_some_and(Overrides::is_empty));
     }
@@ -1550,7 +1550,7 @@ mod tests {
         assert_eq!(r.map(|r| (r.name.as_str(), r.plan.commands.len(), r.plan.report.unmatched.len())), Some(("Theirs", 1, 1)));
         assert!(fx.km.overrides().is_empty(), "review changes nothing");
         st.import_reviewed(&mut fx.env());
-        assert_eq!(fx.p.active, "Theirs");
+        assert_eq!(fx.p.active(), "Theirs");
         assert_eq!(fx.keys("file.save"), ["Cmd+Alt+S"]);
         assert!(st.review.as_ref().is_some_and(|r| r.done.is_some()));
         st.escape(&mut fx.env());

crates/keymap/tests/profiles.rs

@@ -1,6 +1,6 @@
 use craft_keymap::{
     Bindable, Binding, CommandRef, ContextRule, Flavor, ForeignEntry, ForeignKey, ForeignSet, ImportOptions, Keymap, MAX_SETS, Overrides, Platform,
-    Policy, ProfileError, Profiles, Scope, Scopes, Start, plan,
+    Policy, ProfileError, SetProblem, Profiles, Scope, Scopes, Start, plan,
 };
 use serde_json::json;
 
@@ -46,7 +46,7 @@ fn profiles_copy_built_ins_on_edit_and_round_trip_as_json() {
     assert_eq!(p.delete("Classic", &builtins), Err(ProfileError::BuiltIn));
     assert_eq!(p.rename("Custom 3", "classic", &builtins), Err(ProfileError::NameTaken));
     assert_eq!(p.rename("Custom 3", "Mine", &builtins), Ok("Mine".into()));
-    assert_eq!(p.active, "Mine");
+    assert_eq!(p.active(), "Mine");
     assert_eq!(p.save_as(&"x".repeat(65), Overrides::new(), &builtins), Err(ProfileError::BadName));
     p.delete("Mine", &builtins).unwrap();
     assert_eq!(p.active_name(&builtins), "Craft");
@@ -59,10 +59,10 @@ fn profiles_copy_built_ins_on_edit_and_round_trip_as_json() {
     let text = serde_json::to_string(&p).unwrap();
     assert_eq!(serde_json::from_str::<Profiles>(&text).unwrap(), p);
     let junk: Profiles = serde_json::from_str(r#"{"active": 5, "sets": {"": {}, "ok": {"a": ["A"]}, "bad": 3}}"#).unwrap();
-    assert_eq!(junk.sets.keys().collect::<Vec<_>>(), ["ok"]);
+    assert_eq!(junk.sets().keys().collect::<Vec<_>>(), ["ok"]);
     let junk: Profiles =
         serde_json::from_str(r#"{"sets": [{"name": "ok", "overrides": [{"command": "a", "keys": ["A"]}, 4]}, {"name": ""}, 7]}"#).unwrap();
-    assert_eq!(junk.sets["ok"].get("a"), Some(&["A".to_string()][..]));
+    assert_eq!(junk.sets()["ok"].get("a"), Some(&["A".to_string()][..]));
     assert!(text.contains(r#""overrides":[{"command":"edit.copy","keys":[]}]"#), "{text}");
 }
 
@@ -71,7 +71,7 @@ fn rename_keeps_a_set_whose_name_differs_only_in_case() {
     let builtins = [("Craft", Overrides::new())];
     let mut p: Profiles = serde_json::from_str(r#"{"sets": {"Mine": {"a": "A"}, "mine": {"b": "B"}}}"#).unwrap();
     assert_eq!(p.rename("Mine", "mine", &builtins), Err(ProfileError::NameTaken));
-    assert_eq!(p.sets["mine"].get("b"), Some(&["B".to_string()][..]));
+    assert_eq!(p.sets()["mine"].get("b"), Some(&["B".to_string()][..]));
     assert_eq!(p.rename("Mine", "MINE", &builtins), Err(ProfileError::NameTaken));
     p.delete("mine", &builtins).unwrap();
     assert_eq!(p.rename("Mine", "MINE", &builtins), Ok("MINE".into()));
@@ -81,8 +81,8 @@ fn rename_keeps_a_set_whose_name_differs_only_in_case() {
 fn sets_that_are_not_a_list_or_map_load_as_none() {
     for text in [r#"{"active": "Mine", "sets": 5}"#, r#"{"active": "Mine", "sets": null}"#, r#"{"active": "Mine", "sets": "x"}"#] {
         let p: Profiles = serde_json::from_str(text).unwrap_or_else(|e| panic!("{text}: {e}"));
-        assert!(p.sets.is_empty(), "{text}");
-        assert_eq!(p.active, "Mine", "{text}");
+        assert!(p.sets().is_empty(), "{text}");
+        assert_eq!(p.active(), "Mine", "{text}");
     }
 }
 
@@ -107,9 +107,7 @@ fn a_user_set_named_like_a_new_built_in_keeps_its_edits() {
     assert_eq!(p.get("Classic", &builtins), Some(&classic));
     assert_eq!(builtins[1].1, classic);
 
-    let mut q = Profiles::default();
-    q.sets.insert("CRAFT".into(), mine.clone());
-    q.active = "CRAFT".into();
+    let mut q: Profiles = serde_json::from_value(json!({"active": "CRAFT", "sets": [{"name": "CRAFT", "overrides": {"edit.copy": "Cmd+K"}}]})).unwrap();
     assert!(q.reconcile(&builtins));
     assert_eq!(q.active_name(&builtins), "CRAFT 2");
     assert_eq!(q.active_overrides(&builtins), &mine);
@@ -128,7 +126,7 @@ fn an_exported_set_imports_as_a_new_set_with_the_same_keymap() {
 
     let text = p.export("Mine", &builtins).unwrap();
     assert_eq!(p.import(&text, &builtins), Ok("Mine 2".into()));
-    assert_eq!(p.active, "Mine 2");
+    assert_eq!(p.active(), "Mine 2");
 
     let mut again = keymap();
     again.set_overrides(p.active_overrides(&builtins).clone());
@@ -146,14 +144,14 @@ fn duplicate_copies_built_in_and_user_sets() {
     let mut p = Profiles::default();
 
     assert_eq!(p.duplicate("Classic", "Classic", &builtins), Ok("Classic 2".into()));
-    assert_eq!(p.active, "Classic 2");
+    assert_eq!(p.active(), "Classic 2");
     assert_eq!(p.active_overrides(&builtins), &classic);
     assert_eq!(p.duplicate("Classic 2", " Mine ", &builtins), Ok("Mine".into()));
     assert_eq!(p.duplicate("Mine", "mine", &builtins), Ok("mine 2".into()), "names compare without case");
     assert_eq!(p.duplicate("Nope", "X", &builtins), Err(ProfileError::NotFound));
     assert_eq!(p.duplicate("Craft", "", &builtins), Err(ProfileError::BadName));
 
-    while p.sets.len() < MAX_SETS {
+    while p.sets().len() < MAX_SETS {
         p.duplicate("Craft", "Spare", &builtins).unwrap();
     }
     assert_eq!(p.duplicate("Craft", "Spare", &builtins), Err(ProfileError::TooMany));
@@ -173,7 +171,7 @@ fn a_foreign_set_imports_as_a_new_set_and_leaves_the_keymap_alone() {
     assert_eq!(name, "Other App");
     assert_eq!(report.changed.len(), 1);
     assert!(km.overrides().is_empty());
-    assert_eq!(p.active, "Other App");
+    assert_eq!(p.active(), "Other App");
     let mut imported = keymap();
     imported.set_overrides(p.active_overrides(&builtins).clone());
     assert_eq!(imported.bindings(&"file.save".into()), [b("Cmd+Shift+S")]);
@@ -181,3 +179,31 @@ fn a_foreign_set_imports_as_a_new_set_and_leaves_the_keymap_alone() {
     let (name, _) = p.import_plan(&planned, &km, Some("Craft"), &builtins).unwrap();
     assert_eq!(name, "Craft 2", "a built-in's name is not reused");
 }
+
+#[test]
+fn loading_lists_the_sets_it_drops_and_the_store_has_a_version() {
+    let text = r#"{"version": 2, "active": "ok", "sets": [
+        {"name": "ok", "overrides": {}}, {"name": " pad ", "overrides": {}}, {"name": "ok", "overrides": {"a": "A"}}, {"name": "bad", "overrides": 3}
+    ]}"#;
+    let p: Profiles = serde_json::from_str(text).unwrap();
+    assert_eq!(p.sets().keys().collect::<Vec<_>>(), ["ok"]);
+    assert!(p.sets()["ok"].is_empty(), "the first of two sets with one name stays");
+    let name = |n: &str| n.to_string();
+    assert_eq!(
+        p.problems(),
+        [
+            SetProblem::NewerVersion(2),
+            SetProblem::BadName { name: name(" pad ") },
+            SetProblem::Duplicate { name: name("ok") },
+            SetProblem::BadOverrides { name: name("bad") }
+        ]
+    );
+
+    let many: Vec<_> = (0..MAX_SETS + 3).map(|i| json!({"name": format!("s{i}"), "overrides": {}})).collect();
+    let p: Profiles = serde_json::from_value(json!({ "sets": many })).unwrap();
+    assert_eq!((p.sets().len(), p.problems()), (MAX_SETS, &[SetProblem::TooMany { ignored: 3 }][..]));
+
+    assert_eq!(serde_json::to_value(&p).unwrap()["version"], 1);
+    let unversioned: Profiles = serde_json::from_str(r#"{"active": "x", "sets": []}"#).unwrap();
+    assert!(unversioned.problems().is_empty());
+}

crates/keymap/tests/settings.rs

@@ -77,7 +77,7 @@ fn overrides_round_trip_through_craft_settings() {
     let problems = km.problems();
     assert!(problems.contains(&Problem::Dropped { key: "edit.cut".into(), reason: DropReason::NotAList }), "{problems:?}");
     assert!(problems.iter().any(|p| matches!(p, Problem::BadBinding { text, .. } if text == "Cmd+Nope")), "{problems:?}");
-    assert_eq!(store.get().profiles.sets.len(), 2);
+    assert_eq!(store.get().profiles.sets().len(), 2);
 
     store.update(|p| p.general.language = "de".into());
     store.flush().unwrap();
@@ -101,8 +101,8 @@ fn overrides_round_trip_through_craft_settings() {
     assert!(!line.contains("window.panel") && !text.contains("Spare"), "{text}");
     let reloaded = open(tmp.path());
     assert!(keymap(reloaded.get().shortcuts.clone()).bindings(&layers).is_empty());
-    assert_eq!(reloaded.get().profiles.sets.keys().collect::<Vec<_>>(), ["Mine"]);
-    assert_eq!(reloaded.get().profiles.sets["Mine"].get("text.bold"), Some(&["Text: Cmd+B".to_string()][..]));
+    assert_eq!(reloaded.get().profiles.sets().keys().collect::<Vec<_>>(), ["Mine"]);
+    assert_eq!(reloaded.get().profiles.sets()["Mine"].get("text.bold"), Some(&["Text: Cmd+B".to_string()][..]));
 }
 
 #[test]
@@ -140,7 +140,7 @@ fn a_sets_value_that_is_not_a_list_keeps_the_rest_of_the_profiles() {
         let text = format!("schema_version = 1\n\n[profiles]\nactive = \"Mine\"\nsets = {value}\n");
         std::fs::write(tmp.path().join("config.toml"), text).unwrap();
         let store = open(tmp.path());
-        assert_eq!(store.get().profiles.active, "Mine", "{value}");
-        assert!(store.get().profiles.sets.is_empty(), "{value}");
+        assert_eq!(store.get().profiles.active(), "Mine", "{value}");
+        assert!(store.get().profiles.sets().is_empty(), "{value}");
     }
 }

crates/keymap/tests/ui_dialog.rs

@@ -648,17 +648,17 @@ fn cancel_restores_keymap_and_sets() {
     let mut h = with_sets(p);
     click(&mut h, "Set:");
     click_last(&mut h, "Mine");
-    assert_eq!(profiles(&h).active, "Mine");
+    assert_eq!(profiles(&h).active(), "Mine");
     click(&mut h, "Save: ⌘S");
     press(&mut h, Modifiers::NONE, EguiKey::F2);
     assert!(has(&h, "(modified)"));
     new_set(&mut h, "Other");
-    assert_eq!(profiles(&h).active, "Other");
+    assert_eq!(profiles(&h).active(), "Other");
     assert_eq!(h.state().bindings("file.save"), ["F2"], "based on the current shortcuts");
     click(&mut h, "Delete");
     assert!(has(&h, "Delete the set “Other”?"));
     click_last(&mut h, "Delete");
-    assert!(!profiles(&h).sets.contains_key("Other"));
+    assert!(!profiles(&h).sets().contains_key("Other"));
     click(&mut h, "Cancel");
     assert!(h.state().canceled());
     assert_eq!(profiles(&h), &before);
@@ -692,8 +692,8 @@ fn ok_stores_edits_of_a_builtin_as_a_copy() {
     click(&mut h, "OK");
     assert!(h.state().outcomes.iter().any(|o| matches!(o, Outcome::Ok)));
     let p = profiles(&h);
-    assert_eq!(p.active, "Custom");
-    assert_eq!(p.sets.get("Custom").and_then(|o| o.get("file.save")), Some(&["F2".to_string()][..]));
+    assert_eq!(p.active(), "Custom");
+    assert_eq!(p.sets().get("Custom").and_then(|o| o.get("file.save")), Some(&["F2".to_string()][..]));
 }
 
 #[test]
@@ -717,7 +717,7 @@ fn set_export_import_round_trip() {
     let (d, state) = dialog(fresh.state_mut());
     assert_eq!(d.import_json(state, &json), Ok("Name 2".into()));
     fresh.run();
-    assert_eq!(profiles(&fresh).active, "Name 2");
+    assert_eq!(profiles(&fresh).active(), "Name 2");
     assert_eq!(fresh.state().km.overrides(), &edited);
     assert_eq!(fresh.get_by_label("Set:").accesskit_node().value().as_deref(), Some("Name 2"), "the picker shows it");
 }
@@ -756,7 +756,7 @@ fn foreign_import_review_lists_skipped_and_unreadable() {
     assert!(h.query_by_label_contains("numeric keypad key read as main key").is_some());
     assert!(h.state().km.overrides().is_empty(), "the review changes nothing");
     click(&mut h, "Import as New Set");
-    assert_eq!(profiles(&h).active, "Theirs");
+    assert_eq!(profiles(&h).active(), "Theirs");
     assert_eq!(h.state().bindings("file.save"), ["Cmd+Alt+S"]);
     assert!(has(&h, "Imported as “Theirs”"));
     click(&mut h, "Done");

crates/keymap/tests/ui_snapshot.rs

@@ -230,7 +230,7 @@ fn dialog_screenshots() {
                     }
                     (p, builtins)
                 });
-                let options = Options { keyboard: case.keyboard, ..Options::default() };
+                let options = Options::default().with(|o| o.keyboard = case.keyboard);
                 let km = if case.long { with_long_label(fixture(case.platform)) } else { fixture(case.platform) };
                 let text: Box<dyn Text> = match case.words {
                     Words::English => Box::new(English),