From 695cc04e06904254db1531de41570d70a33d43b4 Mon Sep 17 00:00:00 2001 From: ruchirguitar <113586074+ruchirguitar@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:28:25 +0530 Subject: [PATCH] Fix #5236: Edit Original With keeps its Other choice in the dialog and paths with spaces The Other Application choice (before a path is typed) is a `__` field of the Preferences dialog instead of egui memory, so a dialog opened again or reset starts at System Default. The path field shows the stored value untrimmed, so spaces can be typed. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/prefs_dialog.rs | 19 ++++++++++++------- crates/ui-egui/src/prefs_dialog/tests.rs | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/prefs_dialog.rs b/crates/ui-egui/src/prefs_dialog.rs index 03ee49be9..97be22146 100644 --- a/crates/ui-egui/src/prefs_dialog.rs +++ b/crates/ui-egui/src/prefs_dialog.rs @@ -242,6 +242,7 @@ pub fn show(app: &mut VectorcraftApp, ctx: &egui::Context) { for (k, v) in Prefs::default().to_json().as_object().cloned().unwrap_or_default() { d.fields.insert(k, v); } + d.fields.remove(CHOSE_OTHER_APP); } if d.fields.remove(CHOOSE_APP).is_some() { choose_app(app, &mut d); @@ -419,11 +420,13 @@ fn choice_row(ui: &mut egui::Ui, d: &mut Dialog, key: &str, label: &str, opts: & /// Other Application, which shows a field for the application's path. fn edit_original_row(ui: &mut egui::Ui, d: &mut Dialog, key: &str, label: &str, v: &Value) { const OPTIONS: [&str; 3] = ["System Default", "PhotoCraft", "Other Application"]; - let cur = v.as_str().unwrap_or("").trim().to_string(); + // The field shows the value as stored, spaces included (the host trims it when it opens). + let raw = v.as_str().unwrap_or("").to_string(); + let cur = raw.trim().to_string(); let photocraft = cur.eq_ignore_ascii_case("photocraft"); - // Other Application with no path typed yet is still System Default: remember the choice. - let chose_other = egui::Id::unique("prefs-edit-original-other"); - let mut other = !photocraft && (!cur.is_empty() || ui.ctx().data(|m| m.get_temp::(chose_other)).unwrap_or(false)); + // Other Application with no path typed yet is still System Default: the dialog remembers the + // choice in a field of its own, which a new dialog and Reset Preferences start without. + let mut other = !photocraft && (!cur.is_empty() || d.bool(CHOSE_OTHER_APP)); let at = if photocraft { 1 } else if other { @@ -434,12 +437,12 @@ fn edit_original_row(ui: &mut egui::Ui, d: &mut Dialog, key: &str, label: &str, labeled(ui, label, |ui| { if let Some(i) = widgets::dropdown(ui, key, OPTIONS[at], &OPTIONS, 260.0) { other = i == 2; - ui.ctx().data_mut(|m| m.insert_temp(chose_other, other)); + d.fields.insert(CHOSE_OTHER_APP.into(), json!(other)); let new = match i { 0 => String::new(), 1 => "photocraft".to_string(), _ if photocraft => String::new(), - _ => cur.clone(), + _ => raw.clone(), }; d.fields.insert(key.into(), json!(new)); } @@ -447,7 +450,7 @@ fn edit_original_row(ui: &mut egui::Ui, d: &mut Dialog, key: &str, label: &str, if other { let can_pick = d.bool(CAN_PICK_APP); labeled(ui, "Application Path", |ui| { - let mut s = if photocraft { String::new() } else { cur.clone() }; + let mut s = if photocraft { String::new() } else { raw.clone() }; let width = if can_pick { 186.0 } else { 260.0 }; let field = egui::TextEdit::singleline(&mut s).align(egui::Align2::LEFT_CENTER).desired_width(width); let hint = if cfg!(target_os = "macos") { "/Applications/Example.app" } else { "" }; @@ -466,6 +469,8 @@ fn edit_original_row(ui: &mut egui::Ui, d: &mut Dialog, key: &str, label: &str, /// dialog, and that the button was clicked this frame. const CAN_PICK_APP: &str = "__canPickApp"; const CHOOSE_APP: &str = "__chooseApp"; +/// UI-only dialog field: Other Application was chosen, though no path is typed yet. +const CHOSE_OTHER_APP: &str = "__choseOtherApp"; /// The open dialog Edit Original With's Choose… shows: applications (`.app` bundles in /// Applications on macOS, `.exe` files on Windows, any file elsewhere). diff --git a/crates/ui-egui/src/prefs_dialog/tests.rs b/crates/ui-egui/src/prefs_dialog/tests.rs index 32dee4b4b..143adc6e6 100644 --- a/crates/ui-egui/src/prefs_dialog/tests.rs +++ b/crates/ui-egui/src/prefs_dialog/tests.rs @@ -418,3 +418,25 @@ fn edit_original_with_takes_a_chosen_application() { confirm(&mut a).unwrap(); assert_eq!(a.session.prefs.edit_original_app, "/Applications/Editor.app"); } + +/// #5327 review: choosing Other Application before a path is typed is remembered by the dialog +/// itself, so a dialog opened again (or reset) starts at System Default, and a stored path keeps +/// its spaces. +#[test] +fn edit_original_with_other_is_the_dialogs_own_choice() { + let mut a = VectorcraftApp::new(Session::new(), Default::default()); + open(&mut a, Some("File Handling")); + assert!(!a.ui.dialog.as_ref().unwrap().bool(CHOSE_OTHER_APP), "a new dialog starts without the choice"); + let shown = |a: &mut VectorcraftApp| crate::tests_labels::painted_text(a, |a, ui| show(a, ui.ctx())).contains("Application Path"); + assert!(!shown(&mut a), "System Default shows no path field"); + a.ui.dialog.as_mut().unwrap().fields.insert(CHOSE_OTHER_APP.into(), json!(true)); + assert!(shown(&mut a), "Other Application with no path yet shows the field"); + a.ui.dialog = None; + open(&mut a, Some("File Handling")); + assert!(!shown(&mut a), "opened again: System Default"); + // A path with spaces is kept as typed. + let path = "/Applications/Visual Studio Code.app"; + a.ui.dialog.as_mut().unwrap().fields.insert("editOriginalApp".into(), json!(path)); + confirm(&mut a).unwrap(); + assert_eq!(a.session.prefs.edit_original_app, path); +}