Merge main into #1591

Cargo.lock

@@ -1848,6 +1848,7 @@ dependencies = [
  "serde",
  "serde_json",
  "thiserror 2.0.21",
+ "unicode-segmentation",
 ]
 
 [[package]]

crates/doc/Cargo.toml

@@ -15,3 +15,4 @@ serde = { workspace = true }
 serde_json = { workspace = true }
 thiserror = { workspace = true }
 imbl = { workspace = true }
+unicode-segmentation = { workspace = true }

crates/doc/src/text.rs

@@ -150,8 +150,12 @@ impl TextCase {
                     } else {
                         out.push(ch);
                         // An apostrophe only continues a word right after a letter or digit.
-                        let opening_apostrophe = matches!(ch, '\'' | '\u{2019}') && !last.is_some_and(char::is_alphanumeric);
-                        start = !is_word_char(ch) || opening_apostrophe;
+                        let opening_apostrophe =
+                            matches!(ch, '\'' | '\u{2019}') && !last.is_some_and(|c| c.is_alphanumeric() || is_combining_mark(c));
+                        // A combining mark attaches to the char before it and never changes word state.
+                        if !is_combining_mark(ch) {
+                            start = !is_word_char(ch) || opening_apostrophe;
+                        }
                     }
                     last = Some(ch);
                 }
@@ -166,9 +170,23 @@ impl TextCase {
     }
 }
 
-/// Letters, digits and apostrophes continue a word (so "don't" stays "Don't").
+/// Letters, digits, combining marks and apostrophes continue a word (so "don't" stays "Don't"
+/// and decomposed accents stay inside their word).
 fn is_word_char(c: char) -> bool {
-    c.is_alphanumeric() || matches!(c, '\'' | '\u{2019}')
+    c.is_alphanumeric() || is_combining_mark(c) || matches!(c, '\'' | '\u{2019}')
+}
+
+/// Combining marks such as the U+0301 accent in decomposed text: non-alphanumeric chars that
+/// extend the grapheme of the char before them.
+fn is_combining_mark(c: char) -> bool {
+    use unicode_segmentation::UnicodeSegmentation;
+    if c.is_ascii() || c.is_alphanumeric() {
+        return false;
+    }
+    let mut buf = [0u8; 8];
+    buf[0] = b'a';
+    let end = 1 + c.encode_utf8(&mut buf[1..]).len();
+    std::str::from_utf8(&buf[..end]).is_ok_and(|s| s.graphemes(true).count() == 1)
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, Default)]
@@ -969,4 +987,12 @@ mod title_case_tests {
         assert_eq!(TextCase::Title.apply_between("t", Some('n'), Some('\'')), "t");
         assert_eq!(TextCase::Title.apply_between("t", Some(' '), Some('\u{2019}')), "T");
     }
+
+    #[test]
+    fn title_case_combining_marks_do_not_break_words() {
+        assert_eq!(TextCase::Title.apply("re\u{301}sume\u{301}"), "Re\u{301}sume\u{301}");
+        assert_eq!(TextCase::Title.apply("re\u{301}sume\u{301} bon"), "Re\u{301}sume\u{301} Bon");
+        assert_eq!(TextCase::Title.apply("a \u{301}b"), "A \u{301}B");
+        assert_eq!(TextCase::Title.apply("cafe\u{301}'s"), "Cafe\u{301}'s");
+    }
 }

crates/engine/src/cmd.rs

@@ -65,6 +65,9 @@ struct Registry {
     index: HashMap<&'static str, usize>,
     /// The parameter names each command's doc lists (`list` order; see [`doc_param_names`]).
     names: Vec<Option<Vec<&'static str>>>,
+    /// Whether each command's doc lists a parameter it can't run without (`list` order; see
+    /// [`doc_needs_params`]).
+    needs: Vec<bool>,
 }
 
 fn registry() -> &'static Registry {
@@ -127,7 +130,8 @@ fn registry() -> &'static Registry {
             }
         }
         let names = list.iter().map(|s| doc_param_names(s.params)).collect();
-        Registry { list, index, names }
+        let needs = list.iter().map(|s| doc_needs_params(s.params)).collect();
+        Registry { list, index, names, needs }
     })
 }
 
@@ -147,6 +151,25 @@ pub fn param_names(id: &str) -> Option<&'static [&'static str]> {
     r.index.get(id).and_then(|i| r.names.get(*i)).and_then(|n| n.as_deref())
 }
 
+/// Whether command `id` can't run without parameters: its doc lists one that isn't optional (no
+/// `?`), so a call with none is refused (`image.place` needs `data` or `path`). A UI that runs
+/// commands without parameters (the Actions palette) leaves these out. `false` for an unknown id
+/// or a doc that isn't a `{…}` list.
+pub fn needs_params(id: &str) -> bool {
+    let r = registry();
+    r.index.get(id).and_then(|i| r.needs.get(*i)).is_some_and(|n| *n)
+}
+
+/// Whether a `{…}` params doc lists an item whose name has no `?` (`{data (base64) | path, x?}`:
+/// yes; `{ids?, direction? (h|v)}`, `{}` or a doc of another form: no).
+fn doc_needs_params(doc: &'static str) -> bool {
+    let required = |item: &str| {
+        let first = item.trim().split(|c: char| c.is_whitespace() || c == '(').next().unwrap_or_default();
+        first.starts_with(|c: char| c.is_ascii_alphabetic() || c == '_') && !first.contains('?')
+    };
+    doc_items(doc).is_some_and(|items| items.into_iter().any(required))
+}
+
 /// Refuse params naming something the command doesn't take: a misspelt name would otherwise be
 /// ignored and the command answer as if it hadn't been given. `ids` and `id` are always taken:
 /// the layers a command acts on (`params::targets` reads either).

crates/engine/src/unknown_params_tests.rs

@@ -89,3 +89,21 @@ fn batch_steps_refuse_unknown_keys_and_parameters() {
         err(&mut s, "command.batch", json!({"commands": [{"command": "select.all"}, {"command": "comment.list", "params": {"filterr": "open"}}]}));
     assert!(e.contains("comment.list") && e.contains("step 1: unknown parameter `filterr`"), "{e}");
 }
+
+#[test]
+fn commands_that_cant_run_without_parameters_say_so() {
+    // A parameter without `?` (or one of alternatives) is needed; `?` ones and none aren't.
+    // (`file.placeImage` too: its menu item asks for the file, a call has to name it.)
+    for id in ["image.place", "file.placeImage", "comment.add", "comment.reply", "tool.set"] {
+        assert!(crate::cmd::needs_params(id), "{id}");
+    }
+    for id in ["view.zoomIn", "edit.delete", "edit.undo", "no.such.command"] {
+        assert!(!crate::cmd::needs_params(id), "{id}");
+    }
+    // What it says holds: such a command refuses no parameters.
+    let mut s = Session::new();
+    s.execute("file.new", &json!({})).unwrap();
+    for id in ["image.place", "comment.add", "tool.set"] {
+        assert!(s.execute(id, &json!({})).is_err(), "{id} ran without its parameters");
+    }
+}

crates/ui-egui/src/menus/palette.rs

@@ -256,9 +256,10 @@ const MAX_HITS: usize = 60;
 
 /// Commands grouped by top-level menu, filtered by the query. Without one, the menus' items in
 /// catalog order; a search covers every command (the engine's and the UI's own, "Other" for those
-/// no menu lists), best matches first (labels starting with the query, then the shortest), each
-/// group placed by its best row. Labels match in the UI language and in English. The Plugins
-/// menu's items come with the plugins ([`plugin_rows`]).
+/// no menu lists, except those that need parameters or read like a menu item), best matches first
+/// (labels starting with the query, then the shortest), each group placed by its best row. Labels
+/// match in the UI language and in English. The Plugins menu's items come with the plugins
+/// ([`plugin_rows`]).
 pub(crate) fn command_sections(q: &str) -> Vec<(String, Vec<ActionRow>)> {
     let ql = q.trim().to_lowercase();
     let mut menu_of = std::collections::HashMap::new();
@@ -266,9 +267,13 @@ pub(crate) fn command_sections(q: &str) -> Vec<(String, Vec<ActionRow>)> {
         menu_of.entry(m.id).or_insert(m);
     }
     let menu_items = MENU.iter().filter(|m| m.label != SEP).map(|m| (m.id, m.label));
-    let engine = figcraft_engine::all_commands().iter().map(|c| (c.id, c.label));
+    // A row runs its command without parameters (as its menu item does, dialogs included): one
+    // that needs them is offered only through a menu item.
+    let engine = figcraft_engine::all_commands().iter().filter(|c| !figcraft_engine::cmd::needs_params(c.id)).map(|c| (c.id, c.label));
     let candidates: Box<dyn Iterator<Item = (&str, &str)>> = if ql.is_empty() { Box::new(menu_items) } else { Box::new(menu_items.chain(engine)) };
     let mut seen = std::collections::HashSet::new();
+    // The labels menu items show: a command no menu lists that reads the same is left out.
+    let mut menu_labels = std::collections::HashSet::new();
     // By top-level menu; `None`: no menu lists it.
     let mut hits: Vec<(Option<&str>, ActionRow)> = Vec::new();
     for (id, english) in candidates {
@@ -284,6 +289,12 @@ pub(crate) fn command_sections(q: &str) -> Vec<(String, Vec<ActionRow>)> {
             Some(m) => i18n::menu_item(id, m.label),
             None => i18n::command(id, english),
         };
+        let same = same_label(&label);
+        if menu.is_some() {
+            menu_labels.insert(same);
+        } else if menu_labels.contains(&same) {
+            continue;
+        }
         if matches(&ql, &label, &format!("{english} {id}")) {
             hits.push((group, command_row(id, label.into_owned())));
         }
@@ -303,6 +314,11 @@ pub(crate) fn command_sections(q: &str) -> Vec<(String, Vec<ActionRow>)> {
     groups
 }
 
+/// `label` as two rows that read the same compare: any case, without a trailing ellipsis.
+fn same_label(label: &str) -> String {
+    label.trim().trim_end_matches(['…', '.']).trim_end().to_lowercase()
+}
+
 /// Installed plugins' commands, filtered by the query.
 fn plugin_rows(app: &FigApp, q: &str) -> Vec<ActionRow> {
     let ql = q.to_lowercase();

crates/ui-egui/src/palette_tests.rs

@@ -113,7 +113,10 @@ fn rows(groups: &[(String, Vec<crate::menus::ActionRow>)]) -> Vec<&crate::menus:
 #[test]
 fn search_covers_every_command() {
     let menu = figcraft_engine::catalog::MENU;
-    let spec = figcraft_engine::all_commands().iter().find(|c| !menu.iter().any(|m| m.id == c.id)).expect("a command no menu lists");
+    let spec = figcraft_engine::all_commands()
+        .iter()
+        .find(|c| !menu.iter().any(|m| m.id == c.id) && !figcraft_engine::cmd::needs_params(c.id))
+        .expect("a command no menu lists that runs without parameters");
     let groups = crate::menus::command_sections(spec.id);
     let key = format!("command:{}", spec.id);
     let group = groups.iter().find(|(_, rows)| rows.iter().any(|r| r.key == key)).map(|(g, _)| g.as_str());
@@ -184,3 +187,32 @@ fn context_menu_survives_a_same_frame_tap() {
     u.button(far, false);
     assert!(u.app.ui.context_menu.is_none(), "a later outside press closes the menu");
 }
+
+/// Every row a search for `q` offers: (key, label).
+fn offered(q: &str) -> Vec<(String, String)> {
+    crate::menus::command_sections(q).into_iter().flat_map(|(_, rows)| rows.into_iter().map(|r| (r.key, r.label))).collect()
+}
+
+#[test]
+fn a_search_offers_only_what_runs_and_one_row_per_label() {
+    // Place image runs what the menu item does (the file dialog); the command that needs the
+    // image's data or path isn't offered on its own (#1574).
+    let found = offered("place image");
+    assert!(found.iter().any(|(k, _)| k == "command:file.placeImage"), "{found:?}");
+    assert!(!found.iter().any(|(k, _)| k == "command:image.place"), "{found:?}");
+    // Generally: no row needs parameters a palette run can't give unless a menu item runs it, and
+    // a row no menu item gives doesn't read like another row.
+    for q in ["place", "image", "comment", "tool", "export", "page", "a"] {
+        let found = offered(q);
+        let same = |l: &str| l.trim().trim_end_matches(['…', '.']).trim_end().to_lowercase();
+        for (key, label) in &found {
+            let id = key.trim_start_matches("command:");
+            if figcraft_engine::catalog::MENU.iter().any(|m| m.id == id) {
+                continue;
+            }
+            assert!(!figcraft_engine::cmd::needs_params(id), "{q:?} offers {id}, which needs parameters");
+            let twins = found.iter().filter(|(_, l)| same(l) == same(label)).count();
+            assert_eq!(twins, 1, "{q:?} offers {label:?} twice: {found:?}");
+        }
+    }
+}