diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 4ab3982607..f8022a3593 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -583,7 +583,8 @@ fn is_base64(s: &str) -> bool { /// Writes each `from` text of the rewrite's pairs as its `to` text in every string and key of `v`, as /// [`replace_text`] does. Encoded file data, a base64 string under a key of [`BASE64_FIELDS`], stays -/// as it is. +/// as it is. A rewritten key that reads the same as another key of its object takes ` (2)`, ` (3)` +/// and so on; a key that is not rewritten keeps its name. fn replace_in_value(v: &mut Value, r: &Rewrite) { match v { Value::String(s) => { @@ -593,13 +594,30 @@ fn replace_in_value(v: &mut Value, r: &Rewrite) { } Value::Array(items) => items.iter_mut().for_each(|item| replace_in_value(item, r)), Value::Object(fields) => { + // The keys that stay are written first, so a rewritten key that reads as one of them, or + // as another rewritten key, is the one numbered and no field is lost. + let mut rewritten = Vec::new(); for (key, mut item) in std::mem::take(fields) { if BASE64_FIELDS.contains(&key.as_str()) && item.as_str().is_some_and(is_base64) { fields.insert(key, item); continue; } replace_in_value(&mut item, r); - fields.insert(replace_text(&key, r).unwrap_or(key), item); + match replace_text(&key, r) { + Some(written) => rewritten.push((written, item)), + None => { + fields.insert(key, item); + } + } + } + for (written, item) in rewritten { + let mut key = written.clone(); + let mut n = 2; + while fields.contains_key(&key) { + key = format!("{written} ({n})"); + n += 1; + } + fields.insert(key, item); } } _ => {} @@ -681,6 +699,10 @@ impl<'a> Found<'a> { match begins_known_id(text, at, end, r) { Some(len) => self.taken.push(span(at, at + len)), None => { + // A `file:` URL of the path is replaced whole, so no `file://` is left that + // would name a host, unless part of its scheme was taken already. + let url = span(url_start(text, at, from), end).0; + let start = if self.taken.iter().any(|&(a, b)| url < b && a < stop) { start } else { url }; self.taken.push((start, stop)); self.cuts.push((start, stop, to)); } @@ -689,6 +711,26 @@ impl<'a> Found<'a> { } } +/// Where a `file:` URL that holds `from`, an absolute path found at `at` in `text`, begins: `at` less +/// the `file:` before it, with `//` and a host (`localhost`, another host or none) between them, in any +/// case of its letters; before a drive, the host is followed by a `/` too. `at` when no such URL holds it. +fn url_start(text: &str, at: usize, from: &str) -> usize { + let drive = matches!(from.as_bytes(), [drive, b':', ..] if drive.is_ascii_alphabetic()); + if !from.starts_with('/') && !drive { + return at; + } + let Some(before) = text.get(..at) else { return at }; + let Some(scheme) = before.to_ascii_lowercase().rfind("file:") else { return at }; + let rest = &before[scheme + "file:".len()..]; + let host = |h: &str| h.chars().all(|c| c.is_alphanumeric() || matches!(c, '.' | '-' | '_' | ':' | '[' | ']' | '%')); + let holds = if drive { + rest.strip_prefix("//").and_then(|r| r.strip_suffix('/')).is_some_and(host) + } else { + rest.is_empty() || rest.strip_prefix("//").is_some_and(host) + }; + if holds { scheme } else { at } +} + /// `text` with its `%XX` escapes decoded, and for each byte of the decoded text the range of `text` /// it was read from. A decoded byte that is not part of UTF-8 text is read as U+FFFD. fn decoded_with_spans(text: &str) -> Option<(String, Vec<(usize, usize)>)> { diff --git a/crates/ui-egui/src/control/tests.rs b/crates/ui-egui/src/control/tests.rs index 0b19350f6f..babc710067 100644 --- a/crates/ui-egui/src/control/tests.rs +++ b/crates/ui-egui/src/control/tests.rs @@ -91,7 +91,8 @@ fn a_reply_keeps_a_hidden_path_only_inside_a_known_library_id() { } /// With matching in any case (as on Windows), a hidden path written in other letter cases is hidden -/// too; a path in a `file:` URL with `%XX` escapes is hidden on every platform. +/// too; a path in a `file:` URL with `%XX` escapes is hidden on every platform, the whole URL shown as +/// the file name. #[test] fn a_reply_hides_a_path_in_any_case_and_escaped() { let hidden = hiding(&[("loaded/C:/Libs/My Swatches/b.svg", "loaded/b.svg")], &[]); @@ -99,7 +100,7 @@ fn a_reply_hides_a_path_in_any_case_and_escaped() { let pairs = super::hidden_text(&hidden); let rewrite = super::Rewrite { pairs, hiding: &hidden, fold: true }; super::replace_in_value(&mut reply, &rewrite); - assert_eq!(reply["error"], "from b.svg and file:///b.svg", "{reply}"); + assert_eq!(reply["error"], "from b.svg and b.svg", "{reply}"); // Two libraries whose paths differ only in letter case keep their own ids. let two = hiding(&[("loaded//libs/b.svg", "loaded/b.svg"), ("loaded//libs/B.svg", "loaded/B.svg")], &[]); let mut reply = json!({"error": "x /libs/B.svg y /libs/b.svg z /LIBS/b.svg"}); @@ -112,11 +113,22 @@ fn a_reply_hides_a_path_in_any_case_and_escaped() { assert_eq!(reply["error"], "LOADED//LIBS/IN/A.SVG", "{reply}"); let mut reply = json!({"error": "file:///C:/Libs/My%20Swatches/b.svg"}); hide_under_a_root("escaped", &mut reply, &hidden); - assert_eq!(reply["error"], "file:///b.svg", "{reply}"); + assert_eq!(reply["error"], "b.svg", "{reply}"); let hidden = hiding(&[("loaded//libs/caf\u{e9}/b.svg", "loaded/b.svg")], &[]); let mut reply = json!({"error": "file:///libs/caf%C3%a9/b.svg and file:///libs%2Fcaf%c3%A9%2Fb.svg?next=%2Fdocs&x=%FF at 100%25"}); hide_under_a_root("escaped-mixed", &mut reply, &hidden); - assert_eq!(reply["error"], "file://b.svg and file://b.svg?next=%2Fdocs&x=%FF at 100%25", "{reply}"); + assert_eq!(reply["error"], "b.svg and b.svg?next=%2Fdocs&x=%FF at 100%25", "{reply}"); + // A `file:` URL without escapes is replaced whole too, with `localhost` or another host. + let mut reply = json!({"error": "file:///libs/caf\u{e9}/b.svg and FILE://localhost/libs/caf%C3%A9/b.svg"}); + hide_under_a_root("plain-url", &mut reply, &hidden); + assert_eq!(reply["error"], "b.svg and b.svg", "{reply}"); + let mut reply = json!({"error": "file://remote/libs/caf\u{e9}/b.svg; file:/libs/caf\u{e9}/b.svg; file: x/libs/caf\u{e9}/b.svg"}); + hide_under_a_root("other-host", &mut reply, &hidden); + assert_eq!(reply["error"], "b.svg; b.svg; file: xb.svg", "{reply}"); + let hidden = hiding(&[("loaded///host/share/c.svg", "loaded/c.svg")], &[]); + let mut reply = json!({"error": "file://host/share/c.svg, file:////host/share/c.svg, file://localhost//host/share/c.svg"}); + hide_under_a_root("host-url", &mut reply, &hidden); + assert_eq!(reply["error"], "c.svg, c.svg, c.svg", "{reply}"); } /// A path a failed request sent takes the place of a hidden library's id for the same path, written @@ -263,6 +275,19 @@ fn a_reply_replaces_each_hidden_text_once() { let mut reply = json!({"error": "file:///art/in%20work/a.svg"}); hide_under_a_root("escaped-readable", &mut reply, &hidden); assert_eq!(reply["error"], "file:///art/in%20work/a.svg", "{reply}"); + // A URL whose scheme another hidden path took already is cut at its path. + let hidden = hiding(&[("loaded//a", "loaded/a"), ("loaded/file", "loaded/file")], &[]); + let mut reply = json!({"error": "file:///a"}); + hide_under_a_root("scheme-taken", &mut reply, &hidden); + assert_eq!(reply["error"], "file://a", "{reply}"); + // Keys that come to read the same keep every field; a key that is not rewritten keeps its name. + let hidden = hiding(&[("loaded//out/a.svg", "loaded/a.svg")], &[]); + let mut reply = json!({"result": {"a.svg": 1, "file:///out/a.svg": 2, "/out/a.svg": 3}}); + hide_under_a_root("same-keys", &mut reply, &hidden); + assert_eq!(reply["result"]["a.svg"], 1, "{reply}"); + let mut values: Vec<_> = reply["result"].as_object().map(|o| o.values().filter_map(Value::as_u64).collect()).unwrap_or_default(); + values.sort_unstable(); + assert_eq!(values, [1, 2, 3], "{reply}"); } /// A library saved with another platform's absolute path is hidden, also when the working folder is diff --git a/crates/ui-egui/src/tests/automation.rs b/crates/ui-egui/src/tests/automation.rs index 073cefbd82..0ed949edf5 100644 --- a/crates/ui-egui/src/tests/automation.rs +++ b/crates/ui-egui/src/tests/automation.rs @@ -786,7 +786,7 @@ fn a_screenshot_reply_hides_paths_like_any_reply() { } /// A reply whose only trace of a hidden library's path is a `file:` URL with every separator escaped -/// shows the file name. +/// shows the file name alone. #[test] fn a_reply_hides_a_path_with_escaped_separators() { let (mut app, tx, base) = confined("escaped-separators"); @@ -794,7 +794,7 @@ fn a_reply_hides_a_path_with_escaped_separators() { app.ui.color_guide_limit = format!("loaded/{path}"); app.ui.status = format!("from file:{}", path.replace('/', "%2F").replace('\\', "%5C")); let r = request(&mut app, &tx, "test.status", json!({})); - assert_eq!(r["result"], "from file:b.svg", "{r}"); + assert_eq!(r["result"], "from b.svg", "{r}"); let _ = std::fs::remove_dir_all(base); } diff --git a/docs/mcp.md b/docs/mcp.md index 03bb0b9665..faae559e61 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -175,7 +175,8 @@ the suite) keep the agent to the folders you give it: path as the file name in every string and key, the status text (`ui.status`), dialogs, the command palette query (`ui.palette_query`), a script's report (`scriptReport`), a screenshot's reply and the error from an internal fault included, for the libraries hidden before the request ran and after it. The path is also matched with `%XX` escapes as a `file:` - URL writes it, and on Windows in any case of its letters. A hidden id or path stays only where it begins + URL writes it, and on Windows in any case of its letters; a `file:` URL of an absolute path is shown as the file + name alone, without `file:` or its host. A hidden id or path stays only where it begins another loaded library's whole id with no `.` or `..` part, so a readable library's id that begins with a hidden one stays. A reply also shows a path outside the read roots that the request sent by its file name, whether or not a library was loaded from it and whether the request succeeded or failed, so that the reply reads the same either way. The one exception is the `path`