Merge pull request #2048 from MarekSpodBiedry/issue-2043

Separate the CJK commands from Insert Math Expression in the Type menu (#2043)

apps/designcraft/src/native_menu.rs

@@ -230,10 +230,12 @@ fn build(
     path: &str,
     recent: &mut Option<Submenu>,
 ) {
-    for e in entries {
+    for (i, e) in entries.iter().enumerate() {
         match e {
             Item::Sep => {
-                let _ = parent.append(&PredefinedMenuItem::separator());
+                if menus::has_shown_item(app, &entries[i + 1..]) {
+                    let _ = parent.append(&PredefinedMenuItem::separator());
+                }
             }
             Item::Sub(label, children) => {
                 let sub = Submenu::new(menu_title(app, label), true);

crates/ui-egui/src/menus.rs

@@ -39,7 +39,8 @@ pub use tables::{CLASSIC_GROUPS, MENUS, TYPOGRAPHY_GROUPS, WORKSPACE_OPEN_PANELS
 #[cfg(test)]
 use tree::menu_items;
 pub use tree::{
-    AppMenuItem, MAC_APP_MENU, OPEN_RECENT, all_menu_items, menu_key, menu_tree, native_label, native_menu_tree, recent_items, recent_label,
+    AppMenuItem, MAC_APP_MENU, OPEN_RECENT, all_menu_items, has_shown_item, menu_key, menu_tree, native_label, native_menu_tree, recent_items,
+    recent_label,
 };
 pub use ui_commands::{CJK_ONLY, UI_COMMANDS, shown, ui_label};
 

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

@@ -274,6 +274,7 @@ pub const MENUS: &[(&str, &[&str])] = &[
             "cmd:type.storyDirection|Horizontal|{\"vertical\": false}",
             "cmd:type.storyDirection|Vertical|{\"vertical\": true}",
             "<",
+            "-",
             "cmd:type.tateChuYoko",
             "ui:app.rubyDialog",
             "cmd:type.kenten",

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

@@ -2583,6 +2583,22 @@ fn cjk_menu_items_follow_show_cjk_features() {
     assert_eq!(app.ui.dialog.as_ref().map(|d| d.id.as_str()), Some("ruby"), "hidden, not removed");
 }
 
+/// The CJK commands sit in a group of their own, after a separator, and hiding them leaves no
+/// separator at the end of the menu.
+#[test]
+fn cjk_menu_items_have_their_own_group() {
+    let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default());
+    let ty = menu_tree().into_iter().find(|(m, _)| *m == "Type").unwrap().1;
+    let first = ty.iter().position(|i| matches!(i, Item::Cmd { id, .. } if CJK_ONLY.contains(&id.as_str()))).unwrap();
+    assert!(matches!(ty[first - 1], Item::Sep), "separator before the CJK group");
+    assert!(matches!(ty[first - 2], Item::Sub(..)), "the group comes after Story Direction");
+    app.ui.language = String::new();
+    app.run("prefs.set", json!({"cjkFeatures": false})).unwrap();
+    assert!(!has_shown_item(&app, &ty[first..]), "nothing left after the separator");
+    app.run("prefs.set", json!({"cjkFeatures": true})).unwrap();
+    assert!(has_shown_item(&app, &ty[first..]));
+}
+
 /// Preferences › Type › Show CJK Features: left as it is, it keeps following the interface
 /// language; changed, it is kept in prefs.json.
 #[test]

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

@@ -175,13 +175,25 @@ pub fn menu_key(menu: &str, label: &str) -> String {
     format!("{menu}/{label}")
 }
 
+/// Whether any shown item is in `rest`, the entries after a separator: the separator is drawn only then, so hidden
+/// CJK commands leave no separator at the end of a menu (in the window and in the native macOS menu bar).
+pub fn has_shown_item(app: &DesignApp, rest: &[Item]) -> bool {
+    rest.iter().any(|e| match e {
+        Item::Cmd { id, .. } => shown(app, id),
+        Item::Sep => false,
+        Item::Sub(..) => true,
+    })
+}
+
 /// Draws a menu's items; returns how many were hidden.
 pub(super) fn menu_items(app: &mut DesignApp, ui: &mut egui::Ui, items: &[Item], path: &str) -> usize {
     let mut hidden = 0;
-    for it in items {
+    for (i, it) in items.iter().enumerate() {
         match it {
             Item::Sep => {
-                ui.separator();
+                if has_shown_item(app, &items[i + 1..]) {
+                    ui.separator();
+                }
             }
             Item::Sub(name, children) => {
                 let sub = format!("{path}/{name}");