diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 9eb865023..0074e5432 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -320,7 +320,8 @@ const PANEL_FIELDS: [&str; 7] = /// request may not read, each with the id a reply shows instead: `loaded/`, as a library /// loaded without a path is named. The UI state refers to a library by its bare id in a library panel /// and the Color Guide's limit, by a panel id (`library::loaded/`) in the fields of -/// [`PANEL_FIELDS`] and the workspaces' layouts, and by either in the `library` and `panel` params of +/// [`PANEL_FIELDS`], the workspaces' layouts and the keys of shortcut overrides (`panel:`, +/// saved and in an open Keyboard Shortcuts dialog), and by either in the `library` and `panel` params of /// recorded actions' steps. Text the person types (a command palette query, a workspace's or an /// action's name) is not read. The session's loaded libraries whose file the request may not read are /// hidden too. The ids are numbered as the library commands number them @@ -332,6 +333,12 @@ fn unreadable_library_ids(app: &VectorcraftApp) -> Hiding { for field in PANEL_FIELDS { collect_library_panel_ids(&ui[field], &mut rest); } + // A shortcut for showing a panel is keyed `panel:`, saved and in an open Keyboard Shortcuts dialog. + for overrides in [&ui["shortcut_overrides"], &ui["dialog"]["fields"]["overrides"]] { + for panel in overrides.as_object().into_iter().flat_map(|o| o.keys()).filter_map(|key| key.strip_prefix("panel:")) { + collect_library_panel_ids(&json!(panel), &mut rest); + } + } collect_library_params(&ui["action_sets"], &mut rest); for workspace in ["custom_workspaces", "workspace_changes"].iter().filter_map(|k| ui[k].as_array()).flatten() { for (_, layout) in workspace.as_object().into_iter().flatten().filter(|(k, _)| k.as_str() != "name") { @@ -389,15 +396,18 @@ impl Hiding { } /// Adds the sent paths of `pairs`, each as a hidden id with the id shown. A hidden id this one holds - /// for the same path (written with either separator, relative or as the absolute path it reads as - /// ([`absolute_form`]), and in any case of ASCII letters with `fold`) is shown as the sent path is. + /// for the same path (written with either separator when it is a Windows path + /// ([`splits_at_backslash`]), relative or as the absolute path it reads as ([`absolute_form`]), and + /// in any case of ASCII letters with `fold`) is shown as the sent path is. fn set_hidden(&mut self, pairs: Vec<(String, String)>, fold: bool) { let key = |id: &str| { let path = id.strip_prefix("loaded/").unwrap_or(id); - let id = absolute_form(path).unwrap_or_else(|| path.to_string()).replace('\\', "/"); - if fold { id.to_ascii_lowercase() } else { id } + let id = absolute_form(path).unwrap_or_else(|| path.to_string()); + let windows = splits_at_backslash(&id); + let id = if windows { id.replace('\\', "/") } else { id }; + (windows, if fold { id.to_ascii_lowercase() } else { id }) }; - let sent: Vec = pairs.iter().map(|(id, _)| key(id)).collect(); + let sent: Vec<(bool, String)> = pairs.iter().map(|(id, _)| key(id)).collect(); for (id, shown) in &mut self.hidden { if let Some(at) = sent.iter().position(|k| *k == key(id)) { shown.clone_from(&pairs[at].1); @@ -499,13 +509,21 @@ fn hide_library_ids(v: &mut Value, hiding: &Hiding) { /// The text [`hide_library_ids`] replaces, in two groups: the hidden libraries' text, then the text of /// the paths the request sent ([`Hiding::sent`]), each longest first. For each id, that is the id with -/// the id shown, and the path in it with the file name shown, the path written with either separator -/// too, and as `{:?}` quotes it (each `\`, quote and control character escaped). A relative path is -/// also replaced as the absolute path the file access check reads it as, against the working folder. +/// the id shown, and the path in it with the file name shown, a Windows path ([`splits_at_backslash`]) +/// written with either separator too, and as `{:?}` quotes it (each `\`, quote and control character +/// escaped). A relative path is also replaced as the absolute path the file access check reads it as, +/// against the working folder. fn hidden_text(hiding: &Hiding) -> Vec> { vec![text_pairs(&hiding.hidden), text_pairs(&hiding.sent)] } +/// Is `path` a Windows path, whose parts a `\` separates as well as a `/`: any path on Windows, and +/// elsewhere one that starts with a drive (`C:`) or a `\`? Outside Windows, a `\` in any other path is +/// part of a name. +fn splits_at_backslash(path: &str) -> bool { + cfg!(windows) || path.starts_with('\\') || matches!(path.as_bytes(), [drive, b':', ..] if drive.is_ascii_alphabetic()) +} + /// The absolute path that `path`, a relative path (one that does not start with a separator, a drive /// or `~`), reads as from the working folder, as the file access check reads it; `None` for any other /// path. @@ -525,10 +543,15 @@ fn text_pairs(ids: &[(String, String)]) -> Vec<(String, String)> { let path = &id["loaded/".len()..]; let name = shown.strip_prefix("loaded/").unwrap_or(shown); for path in std::iter::once(path.to_string()).chain(absolute_form(path)) { - let (forward, back) = (path.replace('\\', "/"), path.replace('/', "\\")); - let debug = [quoted(&path), quoted(&forward), quoted(&back)].map(|p| (p, quoted(name))); - let plain = [path, forward, back].map(|p| (p, name.to_string())); - for (path, name) in plain.into_iter().chain(debug) { + // A Windows path is also written with the other separator; outside Windows, a native + // path's `\` is part of a name, so that path is written only as it is. + let mut spellings = vec![path.clone()]; + if splits_at_backslash(&path) { + spellings.extend([path.replace('\\', "/"), path.replace('/', "\\")].into_iter().filter(|p| splits_at_backslash(p))); + } + let debug: Vec<_> = spellings.iter().map(|p| (quoted(p), quoted(name))).collect(); + let plain = spellings.into_iter().map(|p| (p, name.to_string())); + for (path, name) in plain.chain(debug) { for (from, to) in [(format!("loaded/{path}"), format!("loaded/{name}")), (path, name)] { if !pairs.iter().any(|(f, _)| *f == from) { pairs.push((from, to)); @@ -1033,9 +1056,10 @@ fn modifiers(p: &Value) -> egui::Modifiers { /// The params that name a library or a library panel, at any depth of a request (`ui.set {panel}`, /// `engine.execute {params: {panel}}`, `ui.libraryPanel {panel, library}`), with the docking -/// commands' destinations (`window.panel.move {anchor, before}`, `window.panel.float {onto, above, -/// below}`) and a list of panels. -const LIBRARY_PARAMS: [&str; 8] = ["panel", "library", "anchor", "before", "onto", "above", "below", "panels"]; +/// commands' destinations (`window.panel.move {anchor, before}`, `ui.dock {target}`, +/// `window.panel.float {onto, above, below}`) and a list of panels. Only a whole id is translated +/// ([`swap_whole_ids`]), so other text in these params stays. +const LIBRARY_PARAMS: [&str; 9] = ["panel", "library", "anchor", "target", "before", "onto", "above", "below", "panels"]; /// The param of `window.panel.layout` that holds a serialized docking action: each panel id in it is /// translated. diff --git a/crates/ui-egui/src/control/tests.rs b/crates/ui-egui/src/control/tests.rs index 83d8b3ddb..1fe6c4473 100644 --- a/crates/ui-egui/src/control/tests.rs +++ b/crates/ui-egui/src/control/tests.rs @@ -140,6 +140,20 @@ fn a_sent_path_replaces_a_hidden_id_for_the_same_path() { assert_eq!(reply["error"], "can't read \"icon.svg\" or icon.svg", "{reply}"); } +/// Outside Windows, a `\` in a native path is part of a name: a sent path that differs from a hidden +/// library's path only by `/` in place of that `\` names another file, so the library keeps its id +/// and a reply does not read either path as a spelling of the other. +#[cfg(not(windows))] +#[test] +fn a_backslash_in_a_native_path_keeps_two_paths_apart() { + let mut hidden = hiding(&[(r"loaded//out/weird\name.svg", r"loaded/weird\name.svg")], &[]); + hidden.set_hidden(vec![("loaded//out/weird/name.svg".to_string(), "loaded/name.svg".to_string())], false); + assert_eq!(hidden.hidden[0].1, r"loaded/weird\name.svg", "{hidden:?}"); + let mut reply = json!({"error": r"no /out/weird\name.svg, /out/weird/name.svg or \out\weird\name.svg"}); + hide_under_a_root("native-backslash", &mut reply, &hidden); + assert_eq!(reply["error"], r"no weird\name.svg, name.svg or \out\weird\name.svg", "{reply}"); +} + /// A sent path whose text overlaps part of a hidden library's path does not keep that library's path /// from being hidden: the hidden libraries are matched first. #[test] diff --git a/crates/ui-egui/src/tests/automation.rs b/crates/ui-egui/src/tests/automation.rs index 3b93e2396..da94f4519 100644 --- a/crates/ui-egui/src/tests/automation.rs +++ b/crates/ui-egui/src/tests/automation.rs @@ -977,6 +977,17 @@ fn ui_set_reopens_a_hidden_librarys_panel_by_its_shown_id() { ); assert_eq!(r["ok"], true, "{r}"); assert!(!r.to_string().contains(hidden.trim_matches('"')), "no path outside the root: {r}"); + // So does `ui.dock`'s move, whose destination is `target`. + let r = request( + &mut app, + &tx, + "engine.execute", + json!({"command": "ui.dock", "params": {"operation": "move", "panel": "properties", "target": shown, "zone": "tab"}}), + ); + assert_eq!(r["ok"], true, "{r}"); + let layout = crate::panel_docking::layout_of(&app.ui); + assert!(layout.contains(&real) && !layout.contains(&shown.to_string()), "no second panel: {layout:?}"); + assert!(!r.to_string().contains(hidden.trim_matches('"')), "no path outside the root: {r}"); // An error that quotes the panel shows the id the client gave, and other text is not rewritten. let r = request(&mut app, &tx, "ui.set", json!({"panel": "library:bogus:loaded/b.svg"})); assert_eq!(r["ok"], false, "{r}"); @@ -986,6 +997,24 @@ fn ui_set_reopens_a_hidden_librarys_panel_by_its_shown_id() { let _ = std::fs::remove_dir_all(base); } +/// A shortcut for showing the panel of a library outside the read root, saved and in an open Keyboard +/// Shortcuts dialog, is keyed by the panel id shown, also when nothing else refers to the library. +#[test] +fn ui_inspect_hides_a_library_panels_path_in_shortcut_overrides() { + let (mut app, tx, base) = confined("shortcut-hidden-panel"); + let real = format!("panel:library:swatches:loaded/{}", s(base.join("outside/b.svg"))); + crate::keymap::set_override(&mut app.ui.shortcut_overrides, &real, "Cmd+Alt+Shift+F7"); + app.ui.dialog = Some(crate::state::Dialog::new("shortcuts", json!({"overrides": { real.clone(): "Cmd+Alt+Shift+F8" }}))); + let r = request(&mut app, &tx, "ui.inspect", json!({})); + let shown = "panel:library:swatches:loaded/b.svg"; + let ui = &r["result"]["ui"]; + assert_eq!(ui["shortcut_overrides"][shown], "Cmd+Alt+Shift+F7", "{r}"); + assert_eq!(ui["dialog"]["fields"]["overrides"][shown], "Cmd+Alt+Shift+F8", "{r}"); + let hidden = json!(s(base.join("outside"))).to_string(); + assert!(!r.to_string().contains(hidden.trim_matches('"')), "no path outside the root: {r}"); + let _ = std::fs::remove_dir_all(base); +} + /// A client without automation roots sees every loaded library's full id in `ui.inspect`. #[test] fn ui_inspect_shows_full_library_ids_without_a_read_root() { diff --git a/docs/mcp.md b/docs/mcp.md index e8e0bf79e..4d804e3f9 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -140,10 +140,13 @@ the suite) keep the agent to the folders you give it: empty folder and the scratch disk choices `Startup` and `None` stay). A library loaded from a file outside the roots shows as `loaded/` wherever `ui_inspect` and `ui.docking` name it: its id, its panel's id, the Color Guide's `ui.color_guide_limit`, a closed panel's key in `ui.docking_hidden` and the `library` and `panel` - params of a recorded action's steps in `ui.action_sets`, also when its panel is closed. A path written with + params of a recorded action's steps in `ui.action_sets`, and the keys of shortcut overrides (`panel:` + in `ui.shortcut_overrides` and an open Keyboard Shortcuts dialog's `ui.dialog.fields.overrides`), also when its + panel is closed. A path written with either separator counts as a path, also one saved on another platform, and outside Windows a path that starts with a drive (`C:`) or a `\` counts as outside the roots. Outside Windows, any other path keeps a `\` in its - file name (`/libs/a\b.ase` shows as `loaded/a\b.ase`). When another hidden library has the same + file name (`/libs/a\b.ase` shows as `loaded/a\b.ase`), and a reply matches it only as written, not with `/` + in place of the `\`. When another hidden library has the same file name, the libraries take `loaded/`, `loaded/ (2)`, `(3)` and so on in the order they are first shown. Libraries first shown in the same reply are numbered in the order of their full ids, the libraries loaded in this session first, then those only the UI state names. Each one keeps its id for the rest @@ -151,8 +154,8 @@ the suite) keep the agent to the folders you give it: file name is loaded. A hidden library skips an id that a library loaded without a path already has, and only a library loaded later from data under that file name, which takes the id itself, moves a hidden one to the next free number. The other entries keep their order, and the app's own state is not changed. A request may name such a library or its panel by the id - shown in a `panel`, `library` or `panels` param, in a docking destination (`anchor`, `before`, `onto`, `above`, - `below`) and in `window.panel.layout`'s `action` (`ui.set {panel: "library:swatches:loaded/"}` + shown in a `panel`, `library` or `panels` param, in a docking destination (`anchor`, `target`, `before`, `onto`, + `above`, `below`) and in `window.panel.layout`'s `action` (`ui.set {panel: "library:swatches:loaded/"}` reopens its panel), and the reply to such a request shows that id and file name wherever the library's id or path appears, an error included. Every reply under the read roots shows a hidden library's id as the id shown and its path as the file name in every string and key, the status text (`ui.status`), dialogs, the command palette query