diff --git a/crates/data/src/lib.rs b/crates/data/src/lib.rs index 8fc974d76..a556bbd6c 100644 --- a/crates/data/src/lib.rs +++ b/crates/data/src/lib.rs @@ -59,7 +59,7 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, thiserror::Error)] pub enum DataError { #[error("i/o: {0}")] - Io(std::io::Error), + Io(#[source] std::io::Error), #[error("{0}")] Format(String), /// The server refused the login: no password, or a wrong one. @@ -599,6 +599,13 @@ impl Dataset { mod tests { use super::*; + #[test] + fn an_io_error_keeps_its_source() { + let e = DataError::Io(std::io::Error::new(std::io::ErrorKind::NotFound, "gone")); + let source = std::error::Error::source(&e).map(ToString::to_string); + assert_eq!(source.as_deref(), Some("gone")); + } + #[test] fn features_are_shared_until_changed() { let mut d = Dataset::new("t", GeometryType::Point, vec![Field::new("a", FieldType::Integer)]); diff --git a/crates/ui-egui/src/diagnosticlog.rs b/crates/ui-egui/src/diagnosticlog.rs index 53d705583..1e9163974 100644 --- a/crates/ui-egui/src/diagnosticlog.rs +++ b/crates/ui-egui/src/diagnosticlog.rs @@ -122,21 +122,8 @@ fn entry(ui: &mut Ui, record: &Record, t: &Tokens) { /// The entry's UTC date and time: with the date, a session past UTC midnight reads in order. fn time(timestamp_ms: u64) -> String { let seconds = timestamp_ms / 1000; - let (year, month, day) = civil_date(seconds / 86_400); - format!("{year:04}-{month:02}-{day:02} {:02}:{:02}:{:02} UTC", seconds / 3600 % 24, seconds / 60 % 60, seconds % 60) -} - -/// The proleptic Gregorian date `days` after 1970-01-01 (Howard Hinnant's `civil_from_days`). -fn civil_date(days: u64) -> (u64, u64, u64) { - let z = days.saturating_add(719_468); - let era = z / 146_097; - let doe = z - era * 146_097; - let yoe = (doe - doe / 1_460 + doe / 36_524 - doe / 146_096) / 365; - let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); - let mp = (5 * doy + 2) / 153; - let day = doy - (153 * mp + 2) / 5 + 1; - let month = if mp < 10 { mp + 3 } else { mp - 9 }; - (yoe + era * 400 + u64::from(month <= 2), month, day) + let (date, minute) = rustgis_engine::map::graphics::civil(seconds); + format!("{date} {minute}:{:02} UTC", seconds % 60) } #[cfg(test)] diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 16127dce2..263a5b6c5 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1126,14 +1126,26 @@ impl RustGisApp { /// The layer to export: `layer` (an id or name, as commands take it; the dialog's is checked /// against its `instance`, so a reused id isn't taken for it), else the active layer. fn export_target(&self, p: &Value) -> Result { - if p.get("layer").is_none_or(Value::is_null) { - return self.session.active_layer.ok_or_else(|| "select a layer in the Layers pane first".to_string()); + let gone = || tl!("the layer Export Features was opened on is gone").to_string(); + match p.get("layer") { + None | Some(Value::Null) => { + return self.session.active_layer.ok_or_else(|| tl!("select a layer in the Layers pane first").to_string()); + } + Some(Value::Number(_) | Value::String(_)) => {} + // Not taken for the active layer: with replaceTable that would overwrite a table. + Some(other) => { + return Err(crate::i18n::fmt("\"layer\" must be a layer's id or name, not {value}", &[("value", &other.to_string())])); + } } let id = self.session.layer_id(p)?; - let l = self.session.map().layer(id).ok_or("the layer Export Features was opened on is gone")?; - match p.get("instance").and_then(Value::as_u64) { - Some(instance) if instance != l.instance_id() => Err("the layer Export Features was opened on is gone".into()), - _ => Ok(id), + let l = self.session.map().layer(id).ok_or_else(gone)?; + match p.get("instance") { + None | Some(Value::Null) => Ok(id), + Some(v) => match v.as_u64() { + Some(instance) if instance == l.instance_id() => Ok(id), + Some(_) => Err(gone()), + None => Err(crate::i18n::fmt("\"instance\" must be a layer's instance number, not {value}", &[("value", &v.to_string())])), + }, } } @@ -1362,8 +1374,6 @@ impl RustGisApp { Ok(v) if v.is_null() => self.session.layout_favourite = Value::String("classic".into()), Ok(v) => self.session.layout_favourite = v, Err(why) => { - // Notices are shown as written, and the language is set only in `start`. - i18n::set_current(self.chosen_lang()); self.session.layout_favourite = Value::String("classic".into()); self.session.notify( crate::i18n::fmt( @@ -1394,8 +1404,6 @@ impl RustGisApp { match CoordReadout::from_code(&code) { Some(format) => self.map.coord_format = format, None => { - // Notices are shown as written, and the language is set only in `start`. - i18n::set_current(self.chosen_lang()); self.session .notify(tl!("The saved coordinate format isn't known to this version, so decimal degrees are shown."), false); } @@ -1941,7 +1949,8 @@ impl RustGisApp { self.themes.iter().find(|t| t.name == self.ui.theme_name).or_else(|| self.themes.iter().find(|t| t.dark == self.ui.dark)) } - /// The theme in use: the one named, else the nearest built-in for the current brightness. + /// The theme in use: the one named, else the first loaded theme of the current brightness, + /// which can be a user theme, else the built-in of that brightness. pub fn current_theme(&self) -> theme::Theme { self.drawn().cloned().unwrap_or_else(|| { let builtin = theme::Theme::builtin(); @@ -1995,7 +2004,7 @@ impl RustGisApp { /// Selects a loaded theme and saves its name and brightness; unknown names leave the choice unchanged. fn use_theme(&mut self, name: &str) -> Result<(), String> { let Some(picked) = self.themes.iter().find(|t| t.name == name).cloned() else { - return Err(i18n::fmt("No theme is named {name}.", &[("name", name)])); + return Err(i18n::fmt("No theme is named {name}.", &[("name", &theme::clip(name))])); }; self.ui.theme_name = picked.name.clone(); self.ui.dark = picked.dark; @@ -3556,22 +3565,28 @@ impl RustGisApp { // pass the app's own pixel budget or the renderer's largest side (and says so). A page // that would be copied below 36 dpi is an error. "ui.captureLayout" => { - let li = self.session.active_layout.ok_or("open a layout first")?; + let li = self.session.active_layout.ok_or_else(|| tl!("open a layout first").to_string())?; // Text still being typed into the Format pane is part of the page. layoutview::commit_drafts(self); - let (w_mm, h_mm) = self.session.project.layouts.get(li).map(|l| (l.width, l.height)).ok_or("open a layout first")?; + let (w_mm, h_mm) = self + .session + .project + .layouts + .get(li) + .map(|l| (l.width, l.height)) + .ok_or_else(|| tl!("open a layout first").to_string())?; let area = (w_mm / 25.4) * (h_mm / 25.4); let side = rustgis_engine::render::layout::MAX_SIDE * 25.4 / w_mm.max(h_mm); let dpi = CAPTURE_DPI.min((CAPTURE_PIXELS / area).sqrt()).min(side); if dpi < CAPTURE_MIN_DPI { - return Err("the page is too large to copy to the clipboard: export it instead".into()); + return Err(tl!("the page is too large to copy to the clipboard: export it instead").into()); } let session = &mut self.session; let png = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { rustgis_engine::cmd::layout::layout_bytes(session, li, false, dpi, false) })) - .unwrap_or_else(|_| Err("the capture failed unexpectedly".into()))?; - let pm = rustgis_engine::render::decode_png(&png).ok_or("could not draw the layout")?; + .unwrap_or_else(|_| Err(tl!("the capture failed unexpectedly").into()))?; + let pm = rustgis_engine::render::decode_png(&png).ok_or_else(|| tl!("could not draw the layout").to_string())?; let size = [pm.width() as usize, pm.height() as usize]; self.ui.copy_image = Some(egui::ColorImage::from_rgba_premultiplied(size, pm.data())); // The dpi reported is the one the image has, on its smaller scale (a whole pixel @@ -3891,7 +3906,7 @@ impl RustGisApp { } "ui.exportLayer" if self.services.download.is_some() => { let id = self.export_target(p)?; - let l = self.session.map().layer(id).ok_or("select a layer in the Layers pane first")?; + let l = self.session.map().layer(id).ok_or_else(|| tl!("select a layer in the Layers pane first").to_string())?; // Any format the dialog offers here (`dialogs::export_formats`): not GeoPackage or // FlatGeobuf, which the data crate refuses in the browser. let ext = p.get("format").and_then(Value::as_str).unwrap_or("geojson").to_ascii_lowercase(); @@ -4482,11 +4497,14 @@ impl RustGisApp { } } Err(e) => { + // A waiting command's error names the address, as the notice does. + // Redacted whole, as the notice is: a key with a space in it isn't left half there. + let e = if e.starts_with(&url) { e } else { format!("{url}: {e}") }; + let e = rustgis_engine::Session::redacted(&e).into_string(); self.session.request_failed(&p, &e); if let Some(id) = claim { self.session.download_failed(id); } - let e = if e.starts_with(&url) { e } else { format!("{url}: {e}") }; self.landed_in_dialog(&p, Err(&e)); self.session.notify(e, true); } @@ -4718,10 +4736,17 @@ impl RustGisApp { /// Tells how a service layer added outside the add-service dialog (or in it) went: in the /// dialog's own line, or as a notice for the Connections tree. - fn report_step(&mut self, dialog: bool, r: Result) { - match (dialog, r) { + /// `shown` is the line the step put in the dialog: the dialog is told only while it still + /// shows that line (a failure clears it), so a late step never overwrites a newer add's line + /// and is told as a notice instead. + fn report_step(&mut self, dialog: bool, shown: Option<&str>, r: Result) { + let in_dialog = dialog && self.dialogs.open == Some("addService") && shown.is_some() && self.dialogs.info.as_deref() == shown; + match (in_dialog, r) { (true, Ok(m)) => self.dialogs.info = Some(m), - (true, Err(e)) => self.dialogs.error = Some(connectionui::translated(&e)), + (true, Err(e)) => { + self.dialogs.info = None; + self.dialogs.error = Some(connectionui::translated(&e)); + } (false, Ok(m)) => self.session.notify(m, false), (false, Err(e)) => self.session.notify(connectionui::translated(&e), true), } @@ -4787,7 +4812,7 @@ impl RustGisApp { } Err(e) => { self.notice_service_query_loss(resolve.withheld_addresses); - self.report_step(dialog, Err(e)); + self.report_step(dialog, pending.as_deref(), Err(e)); continue; } } @@ -4816,7 +4841,7 @@ impl RustGisApp { connectiontree::name_recent(self, before, origin.as_deref()); } let r = r.map(|_| serviceui::added(&title)); - self.report_step(dialog, r); + self.report_step(dialog, pending.as_deref(), r); self.map.invalidate(); } ServiceStep::Coverage { mut params, slot, dialog, origin, opened } => { @@ -4848,7 +4873,7 @@ impl RustGisApp { } connectiontree::run_add_url(self, p, origin.as_deref()) }); - self.report_step(dialog, r.map(|_| serviceui::downloading(&title))); + self.report_step(dialog, pending.as_deref(), r.map(|_| serviceui::downloading(&title))); } } } @@ -5295,8 +5320,9 @@ impl RustGisApp { // egui times clicks by the frame they arrive in: at a slow frame rate the two clicks of a // double-click land too far apart for its default delay (#1321). It reads the delay when // the next frame begins, before that frame's `logic`, so it's set here, from this frame's - // work so far (a map drawn on the CPU) and from the last frame's length, painting included - // (`stable_dt` leaves out the time spent idle between frames). + // work so far (a map drawn on the CPU) and from egui's `stable_dt`. That is the last + // frame's real length only when the frame before it asked for an immediate repaint, and + // egui's `predicted_dt` otherwise, so a frame slow only in painting can go unseen. if let Some(start) = self.frame_start.take() { let dt = ctx.input(|i| i.stable_dt).max(start.elapsed().as_secs_f32()); ctx.options_mut(|o| o.input_options.max_double_click_delay = double_click_delay(dt)); @@ -6433,6 +6459,34 @@ mod download_service_tests { } } + /// A failed step clears its own "still working" line beside the error; one that lands after a + /// newer add leaves that add's line alone and is told as a notice. + #[test] + fn a_failed_service_step_clears_only_its_own_line() { + for (newer, open) in [(false, true), (true, true), (false, false)] { + let mut app = RustGisApp::new(rustgis_engine::Session::new(rustgis_engine::Project::default())); + let params = json!({"url":"https://h.invalid/wcs", "name":"Heights"}); + let slot = std::sync::Arc::new(std::sync::Mutex::new(Some(Err("the server is down".to_string())))); + let opened = app.session.opened; + app.ui.service_steps.push(super::ServiceStep::Coverage { params, slot, dialog: true, origin: None, opened }); + app.dialogs.info = Some(if newer { "Reading the tileset of Roads" } else { "Describing Heights" }.into()); + app.dialogs.open = open.then_some("addService"); + app.poll_service_steps(); + if !open { + // Closed, the dialog would keep the error unseen: it is told as a notice. + assert!(app.dialogs.error.is_none(), "{:?}", app.dialogs.error); + assert!(app.session.notices.iter().any(|n| n.error && n.text.contains("the server is down"))); + } else if newer { + assert_eq!(app.dialogs.info.as_deref(), Some("Reading the tileset of Roads"), "the newer line stays"); + assert!(app.dialogs.error.is_none(), "{:?}", app.dialogs.error); + assert!(app.session.notices.iter().any(|n| n.error && n.text.contains("the server is down"))); + } else { + assert!(app.dialogs.info.is_none(), "the step's own line is cleared: {:?}", app.dialogs.info); + assert!(app.dialogs.error.as_deref().is_some_and(|e| e.contains("the server is down")), "{:?}", app.dialogs.error); + } + } + } + #[test] fn retired_service_steps_clear_only_their_own_pending_dialog() { for coverage in [false, true] { @@ -6596,6 +6650,14 @@ mod download_service_tests { // A layer by name is that layer; one not there is refused, not taken for the active one. let e = app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "addTable", "layer": "Nowhere"})).unwrap_err(); assert!(e.contains("Nowhere"), "{e}"); + // A layer or instance of the wrong kind is refused, not taken for the active or a reused layer. + let e = + app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "replaceTable", "tableName": "Cities", "layer": true})).unwrap_err(); + assert!(e.contains("\"layer\""), "{e}"); + let e = app + .run("ui.exportLayer", json!({"format": "gpkg", "mode": "replaceTable", "tableName": "Cities", "layer": id, "instance": "7"})) + .unwrap_err(); + assert!(e.contains("\"instance\""), "{e}"); // Cancelling the pick writes nothing. let before = std::fs::read(&path).unwrap(); app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "replaceTable", "tableName": "Cities"})).unwrap(); diff --git a/crates/ui-egui/src/mapview.rs b/crates/ui-egui/src/mapview.rs index 89eb8c1d8..0f12f265c 100644 --- a/crates/ui-egui/src/mapview.rs +++ b/crates/ui-egui/src/mapview.rs @@ -1538,6 +1538,16 @@ struct MovingLabel { /// what the Move Label tool grabs, as it is drawn now. Labels not of a layer's features aren't; /// nor any while a moving globe or tilted map draws them from where they were placed. fn label_at(app: &RustGisApp, rect: Rect, p: Pos2) -> Option { + label_at_where(app, rect, p, |_| true) +} + +/// The topmost moved label under the pointer, even beneath one never moved (Show all labels). +fn pinned_label_at(app: &RustGisApp, rect: Rect, p: Pos2) -> Option { + label_at_where(app, rect, p, |l| l.collide.pinned) +} + +/// The topmost label under the pointer that `keep` accepts. +fn label_at_where(app: &RustGisApp, rect: Rect, p: Pos2, keep: impl Fn(&PlacedLabel) -> bool) -> Option { if app.map.key.is_none_or(|key| key.rev != app.session.revision || key.sources != rustgis_engine::map::source::generation()) { return None; } @@ -1557,7 +1567,7 @@ fn label_at(app: &RustGisApp, rect: Rect, p: Pos2) -> Option { return None; } let at = p - rect.min; - app.map.labels.iter().rev().filter(|l| l.collide.source.is_some() && !l.text.is_empty()).find_map(|l| { + app.map.labels.iter().rev().filter(|l| l.collide.source.is_some() && !l.text.is_empty() && keep(l)).find_map(|l| { let mut l = l.clone(); // Where it is drawn now: its text at its own size, its callout's feature where it is. (l.x, l.y) = (shift.x + l.x * s, shift.y + l.y * s); @@ -2725,8 +2735,7 @@ fn interact(app: &mut RustGisApp, ui: &mut Ui, resp: &egui::Response, rect: Rect // Labels move by dragging them; Shift-clicking a moved one puts it back. Tool::MoveLabel => { if shift - && let Some((layer, feature, class)) = - label_at(app, rect, p).filter(|l| l.collide.pinned).and_then(|l| l.collide.source) + && let Some((layer, feature, class)) = pinned_label_at(app, rect, p).and_then(|l| l.collide.source) && let Some(name) = label_class_name(app, layer, class) && let Err(error) = app.run("labels.move", json!({"layer": layer, "class": name, "feature": feature, "to": null})) { @@ -4626,6 +4635,13 @@ mod tests { let pinned = h.state().map.labels.iter().find(|l| l.collide.pinned).map(|l| rect.min + vec2(l.x, l.y)); let shift = egui::Modifiers { shift: true, ..none }; let at = pinned.expect("the moved label drawn"); + // A label never moved drawn over the moved one doesn't hide it from Shift-click. + let mut over = h.state().map.labels.iter().find(|l| l.collide.pinned).cloned().expect("the moved label"); + (over.collide.pinned, over.text) = (false, "Over".into()); + h.state_mut().map.labels.push(over); + assert_eq!(label_at(h.state(), rect, at).map(|l| l.text), Some("Over".into()), "the unmoved label is on top"); + assert!(pinned_label_at(h.state(), rect, at).is_some_and(|l| l.collide.pinned), "the moved one is still found"); + // Clicked with the unmoved label still on top: the moved one is put back. selection_pointer(&mut h, at, None, shift); selection_pointer(&mut h, at, Some(true), shift); selection_pointer(&mut h, at, Some(false), shift); diff --git a/crates/ui-egui/src/modelbuilder.rs b/crates/ui-egui/src/modelbuilder.rs index 5001ca555..e8ef8c0b4 100644 --- a/crates/ui-egui/src/modelbuilder.rs +++ b/crates/ui-egui/src/modelbuilder.rs @@ -323,8 +323,12 @@ pub fn command(app: &mut RustGisApp, p: &Value) -> Result<(), String> { pub fn open(app: &mut RustGisApp, name: Option<&str>) -> Result<(), String> { forget_old_project(app); if let Some(n) = name { - if !app.session.project.models.iter().any(|m| m.name.eq_ignore_ascii_case(n)) { + let Some(m) = app.session.project.models.iter().find(|m| m.name.eq_ignore_ascii_case(n)) else { return Err(crate::i18n::fmt("no model '{name}'", &[("name", n)])); + }; + // A model that can't be shown is refused before the user is asked to discard changes. + if m.graph.is_none() { + model::checked_steps_to_graph(&m.steps)?; } if app.mb.open && app.mb.dirty() && !app.mb.saved_as.as_deref().is_some_and(|s| s.eq_ignore_ascii_case(n)) { app.mb.confirm = Some(Pending::Open(n.to_string())); @@ -2075,5 +2079,11 @@ mod tests { assert_eq!(app.mb.saved_as.as_deref(), Some("Keep")); assert_eq!(app.session.project.models, models); } + // With unsaved changes, it is refused before the user is asked to discard them. + app.mb.name = "Keep edited".into(); + assert!(app.mb.dirty()); + let error = app.run("ui.modelBuilder", json!({"name": "Large"})).unwrap_err(); + assert!(error.contains("1001 steps"), "{error}"); + assert!(app.mb.confirm.is_none(), "no discard prompt for a model that can't be opened"); } } diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 6f30d91a2..5749de63f 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -1960,6 +1960,7 @@ fn layer_row(app: &mut RustGisApp, ui: &mut Ui, l: &Layer, scale: f64, reorder: ("renderer", tl_raw!(rl.style.renderer.label())), ], ), + None if rl.error.is_none() && rl.in_place => tl!("Reading in place…").to_string(), None => crate::i18n::fmt( "The raster's data couldn't be read: {error}", &[("error", rl.error.as_deref().or(rl.path.as_deref()).unwrap_or(""))], @@ -6435,7 +6436,12 @@ fn raster_symbology(app: &mut RustGisApp, ui: &mut Ui, l: &Layer, t: &Tokens) { ui.label(egui::RichText::new(&l.name).font(semibold(14.0))); let Some(rl) = l.raster.clone() else { return }; let Some(d) = rl.data.clone() else { - ui.label(egui::RichText::new(tl!("The raster's data couldn't be read.")).color(t.red)); + // Read in place, its overview is on its way until it lands, or fails (as Details says). + if rl.error.is_none() && rl.in_place { + ui.label(egui::RichText::new(tl!("Reading in place…")).color(t.text_dim)); + } else { + ui.label(egui::RichText::new(tl!("The raster's data couldn't be read.")).color(t.red)); + } return; }; let style = rl.style.clone(); diff --git a/crates/ui-egui/src/pluginsui.rs b/crates/ui-egui/src/pluginsui.rs index 469f2c10d..8503d3f38 100644 --- a/crates/ui-egui/src/pluginsui.rs +++ b/crates/ui-egui/src/pluginsui.rs @@ -397,10 +397,14 @@ fn tell_unloaded(app: &mut RustGisApp) { } let text = match failed.as_slice() { [] => return, - [(id, reason)] => crate::i18n::fmt( - "The plugin {id} wasn't loaded. Open Plugins › Plugin Manager to see it. {reason}", - &[("id", id), ("reason", reason)], - ), + // The way to it first, so a long reason cut from the notice doesn't take it along. + [(id, reason)] => match reason.trim().trim_end_matches('.') { + "" => crate::i18n::fmt("The plugin {id} wasn't loaded. Open Plugins › Plugin Manager to see it.", &[("id", id)]), + reason => crate::i18n::fmt( + "The plugin {id} wasn't loaded (open Plugins › Plugin Manager to see it): {reason}.", + &[("id", id), ("reason", reason)], + ), + }, _ => { let ids: Vec<&str> = failed.iter().map(|(id, _)| id.as_str()).collect(); crate::i18n::fmt( @@ -1249,8 +1253,8 @@ mod tests { let mut app = app_fetching(|_| Err("unreachable".into())); let told = start(&mut app, vec![record("demo", "0")]); assert_eq!(told.len(), 1, "{told:?}"); - assert!(told[0].0.starts_with("The plugin demo wasn't loaded."), "{told:?}"); - assert!(told[0].0.ends_with("unreachable"), "{told:?}"); + assert!(told[0].0.starts_with("The plugin demo wasn't loaded (open Plugins › Plugin Manager to see it): "), "{told:?}"); + assert!(told[0].0.ends_with("unreachable."), "{told:?}"); // A download that isn't the one installed. let mut app = app_fetching(|_| Ok(b"x".to_vec())); @@ -1263,7 +1267,7 @@ mod tests { let damaged = Saved { data: Some("!not base64!".into()), url: None, ..record("damaged", "0") }; let told = start(&mut app, vec![damaged]); assert_eq!(told.len(), 1, "{told:?}"); - assert!(told[0].0.starts_with("The plugin damaged wasn't loaded.") && told[0].0.contains("damaged:"), "{told:?}"); + assert!(told[0].0.starts_with("The plugin damaged wasn't loaded (open Plugins") && told[0].0.contains("damaged:"), "{told:?}"); // Loaded: nothing to tell, nothing listed. struct Cleanup(#[allow(dead_code)] tempfile::TempDir); @@ -1303,7 +1307,8 @@ mod tests { let listed: Vec<&str> = app.plugins.unloaded.iter().map(|u| u.saved.id.as_str()).collect(); assert_eq!(listed, ["damaged"]); let told: Vec<&str> = app.session.notices_since(mark).iter().map(|n| n.text.as_str()).collect(); - assert!(told.len() == 1 && told[0].starts_with("The plugin damaged wasn't loaded."), "{told:?}"); + assert!(told.len() == 1 && told[0].starts_with("The plugin damaged wasn't loaded (open Plugins"), "{told:?}"); + assert!(!told[0].contains(".."), "a reason's own full stop isn't doubled: {told:?}"); assert!(rp::get("restore-new").is_some()); } diff --git a/crates/ui-egui/src/readjob.rs b/crates/ui-egui/src/readjob.rs index 8bc14236f..97f2b2b5d 100644 --- a/crates/ui-egui/src/readjob.rs +++ b/crates/ui-egui/src/readjob.rs @@ -339,7 +339,7 @@ pub fn land(app: &mut RustGisApp) -> bool { [what] => app.session.notify(crate::i18n::fmt("{what} was left: {why}", &[("what", what), ("why", &reason)]), false), [first, ..] => app.session.notify( crate::i18n::fmt( - "{n} files or layers were left, the first {what}: {why}", + "{n} reads were left, the first {what}: {why}", &[("n", &of.len().to_string()), ("what", first), ("why", &reason)], ), false, @@ -457,9 +457,7 @@ mod tests { app.run("project.new", Value::Null).unwrap(); landed(&mut app, 60); assert!(app.session.map().layers.is_empty()); - assert!( - app.session.notices.iter().any(|n| n.text == "2 files or layers were left, the first first.csv: another project was opened") - ); + assert!(app.session.notices.iter().any(|n| n.text == "2 reads were left, the first first.csv: another project was opened")); // Closing the app cancels a read under way and waits for it. crate::layerchooser::add_file(&mut app, &first).unwrap(); diff --git a/crates/ui-egui/src/sessionlog.rs b/crates/ui-egui/src/sessionlog.rs index 1da4562c7..c01fad259 100644 --- a/crates/ui-egui/src/sessionlog.rs +++ b/crates/ui-egui/src/sessionlog.rs @@ -219,7 +219,8 @@ fn save(app: &mut RustGisApp) { if app.download_bytes("session-log.csv", bytes) && let Some(said) = app.session.log.dropped_said() { - app.session.notify(format!("session-log.csv: not in it: {said}"), false); + app.session + .notify(crate::i18n::fmt("{name}: not in it: {said}", &[("name", "session-log.csv"), ("said", &said)]), false); } } Err(e) => app.session.notify(e.to_string(), true), diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 525e4dbfa..631fcbb5e 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -440,20 +440,18 @@ impl Theme { pub fn parse(fallback_name: &str, text: &str) -> Result { // Notepad saves UTF-8 with a byte-order mark, which serde refuses at the first byte. let text = text.trim_start_matches('\u{feff}'); + // The schema is read first, so a newer file's new keys are refused by its number, not as unknown. + let loose: serde_json::Value = serde_json::from_str(text).map_err(|e| e.to_string())?; + if let Some(found) = loose.get("schema").and_then(serde_json::Value::as_u64).filter(|&n| n != u64::from(SCHEMA)) { + let named = loose.get("name").and_then(serde_json::Value::as_str).unwrap_or_default(); + let name = shown_name(named, fallback_name, &mut Vec::new()); + return Err(schema_refused(&name, &found.to_string())); + } let raw: Raw = serde_json::from_str(text).map_err(|e| e.to_string())?; let mut notes = Vec::new(); - let mut name = clean_name(&raw.name, &mut notes); - if name.is_empty() { - name = clean_name(fallback_name, &mut notes); - } - if name.is_empty() { - name = "Theme".to_string(); - } + let name = shown_name(&raw.name, fallback_name, &mut notes); if raw.schema != SCHEMA { - return Err(i18n::fmt( - "{name}: theme schema {found} is not {schema}", - &[("name", name.as_str()), ("found", &raw.schema.to_string()), ("schema", &SCHEMA.to_string())], - )); + return Err(schema_refused(&name, &raw.schema.to_string())); } let extends = raw.extends.is_some(); let base = match raw.extends.as_deref() { @@ -470,7 +468,7 @@ impl Theme { }; let unknown: Vec<&str> = raw.colours.keys().map(String::as_str).filter(|k| !TOKENS.contains(k)).collect(); if !unknown.is_empty() { - let names = unknown.join(", "); + let names = unknown.into_iter().map(clip).collect::>().join(", "); return Err(i18n::fmt("{name}: unknown {names}", &[("name", name.as_str()), ("names", &names)])); } let missing: Vec<&str> = match extends { @@ -750,18 +748,62 @@ pub fn contrast(a: Color32, b: Color32) -> f32 { (a.max(b) + 0.05) / (a.min(b) + 0.05) } +/// The file's name, else the name of its file, else "Theme". +fn shown_name(name: &str, fallback_name: &str, notes: &mut Vec) -> String { + let mut shown = clean_name(name, notes); + if shown.is_empty() { + shown = clean_name(fallback_name, notes); + } + if shown.is_empty() { + shown = "Theme".to_string(); + } + shown +} + +/// The refusal of a file written for another theme schema. +fn schema_refused(name: &str, found: &str) -> String { + i18n::fmt("{name}: theme schema {found} is not {schema}", &[("name", name), ("found", found), ("schema", &SCHEMA.to_string())]) +} + /// A name as the menu shows it, with a note for everything it had to drop or cut. fn clean_name(name: &str, notes: &mut Vec) -> String { - let trimmed = name.trim(); - let printable = trimmed.chars().filter(|c| !c.is_control()).count(); - let controls = trimmed.chars().count() - printable; - let kept: String = trimmed.chars().filter(|c| !c.is_control()).take(MAX_NAME).collect(); + // Spaces and control characters at either end are removed; those inside are shown as spaces. + let trimmed = name.trim_matches(|c: char| c.is_whitespace() || c.is_control()); + let controls = trimmed.chars().filter(|c| c.is_control()).count(); + // Spaces at the ends are trimmed quietly, as before; other control characters there are told. + let hard = |s: &str| s.chars().filter(|c| c.is_control() && !c.is_whitespace()).count(); + let removed = hard(name).saturating_sub(hard(trimmed)); + // A run of spaces holding a control character becomes one space, so "Mine\nTwo" reads "Mine Two". + let mut spaced = String::with_capacity(trimmed.len()); + let mut run = String::new(); + for c in trimmed.chars() { + if c.is_whitespace() || c.is_control() { + run.push(c); + continue; + } + if run.chars().any(char::is_control) { + spaced.push(' '); + } else { + spaced.push_str(&run); + } + run.clear(); + spaced.push(c); + } + let printable = spaced.chars().count(); + let kept: String = spaced.chars().take(MAX_NAME).collect(); let kept = kept.trim(); if controls > 0 { notes.push(if controls == 1 { - i18n::fmt("{name}: 1 control character was dropped from the name", &[("name", kept)]) + i18n::fmt("{name}: 1 control character in the name was shown as a space", &[("name", kept)]) } else { - i18n::fmt("{name}: {n} control characters were dropped from the name", &[("name", kept), ("n", &controls.to_string())]) + i18n::fmt("{name}: {n} control characters in the name were shown as spaces", &[("name", kept), ("n", &controls.to_string())]) + }); + } + if removed > 0 { + notes.push(if removed == 1 { + i18n::fmt("{name}: 1 control character at an end of the name was removed", &[("name", kept)]) + } else { + i18n::fmt("{name}: {n} control characters at the ends of the name were removed", &[("name", kept), ("n", &removed.to_string())]) }); } if printable > MAX_NAME { @@ -922,8 +964,8 @@ fn write_theme_text(path: &std::path::Path, text: &str, permissions: Option String { - let mut out: String = text.chars().take(32).collect(); +pub(crate) fn clip(text: &str) -> String { + let mut out: String = text.chars().take(32).map(|c| if c.is_control() { ' ' } else { c }).collect(); if out.chars().count() < text.chars().count() { out.push('…'); } @@ -1271,6 +1313,15 @@ mod tests { assert!(err.contains("schema 9 is not 1"), "{err}"); } + #[test] + fn a_newer_file_with_new_keys_is_refused_by_its_schema() { + let mut json: serde_json::Value = serde_json::from_str(&light().to_json().expect("json")).expect("json"); + json["schema"] = 2.into(); + json["shadows"] = serde_json::json!({"depth": 3}); + let err = Theme::parse("Mine", &json.to_string()).expect_err("a newer schema is refused"); + assert!(err.contains("theme schema 2 is not 1"), "the number is named, not the new key: {err}"); + } + #[test] fn a_file_round_trips_with_its_alpha() { let theme = light(); @@ -1600,6 +1651,7 @@ mod tests { fn a_bad_value_is_echoed_short() { assert_eq!(clip("abc"), "abc", "a short value is kept as it is"); assert!(clip(&"x".repeat(500)).chars().count() <= 33, "a long value is cut to one line"); + assert_eq!(clip("a\nb\tc"), "a b c", "a control character would break the line"); } #[test] @@ -1634,6 +1686,10 @@ mod tests { assert_eq!(theme.metrics.heading, 17.0, "what the file does not set stays standard"); assert_eq!(theme.notes.len(), 1, "{:?}", theme.notes); assert!(theme.notes[0].contains("5 sizes"), "every held value is counted: {:?}", theme.notes); + + // JSON has no infinity, and a size past f32 is refused when the file is read, not reset. + json["metrics"]["heading"] = serde_json::json!(1e39); + assert!(Theme::parse("Mine", &json.to_string()).is_err()); } /// Radius clamping and rounding are counted, while an exact supported radius stays quiet. @@ -1670,7 +1726,16 @@ mod tests { let mut json: serde_json::Value = serde_json::from_str(&light().to_json().expect("json")).expect("json"); json["name"] = serde_json::json!("Mine\nSecond line\twith a tab"); let theme = Theme::parse("Mine", &json.to_string()).expect("it parses"); - assert_eq!(theme.name, "MineSecond linewith a tab", "a newline would break the button"); + assert_eq!(theme.name, "Mine Second line with a tab", "a newline would break the button"); + assert_eq!(theme.notes.len(), 1, "{:?}", theme.notes); + assert!(theme.notes[0].contains("2 control characters"), "{:?}", theme.notes); + + // One at an end is removed, and the note says so rather than "shown as a space". + json["name"] = serde_json::json!("Mine \u{7}"); + let theme = Theme::parse("Mine", &json.to_string()).expect("it parses"); + assert_eq!(theme.name, "Mine"); + assert_eq!(theme.notes.len(), 1, "{:?}", theme.notes); + assert!(theme.notes[0].contains("1 control character at an end of the name was removed"), "{:?}", theme.notes); json["name"] = serde_json::json!("N".repeat(200)); let theme = Theme::parse("Mine", &json.to_string()).expect("it parses"); diff --git a/packaging/windows/rustgis.nsi b/packaging/windows/rustgis.nsi index 9478043d4..cadfc7155 100644 --- a/packaging/windows/rustgis.nsi +++ b/packaging/windows/rustgis.nsi @@ -64,8 +64,9 @@ VIAddVersionKey "LegalCopyright" "Copyright the RustGIS contributors. Apache-2.0 ; replaced while it runs, so a silent install waits for each one it replaces (rustgis.exe, and ; rustgis-cli.exe a terminal may be running) to be released, for up to 30 seconds each. Still ; running then, nothing is installed (exit code 2): a silent install would skip the locked files -; and still record the new version. RustGIS's next start says the update didn't arrive. Run by -; hand, the installer doesn't wait: replacing a running program asks to retry, as before. +; and still record the new version. RustGIS's next start says the update didn't arrive. Run +; interactively (without /S), the installer doesn't wait: replacing a running program asks to +; retry, as before. ; `ID` makes the labels of each insertion unique. !macro WaitForFree FILE ID IfSilent 0 free_${ID}