From ad79cbcf3152ace45f1defdecc2bcdebf24694ba Mon Sep 17 00:00:00 2001 From: aiongg Date: Tue, 20 Oct 2026 15:29:42 +0000 Subject: [PATCH] keymap: translate shortcut parse errors and give translators whole sentences with placeholders --- crates/keymap/src/ui/keyboard.rs | 2 +- crates/keymap/src/ui/review.rs | 5 +-- crates/keymap/src/ui/rows.rs | 8 ++--- crates/keymap/src/ui/state.rs | 29 ++++++++++++---- crates/keymap/src/ui/summary.rs | 2 +- crates/keymap/src/ui/text.rs | 26 ++++++++++++-- crates/keymap/src/ui/view.rs | 59 ++++++++++++-------------------- crates/keymap/tests/ui_dialog.rs | 8 +++++ 8 files changed, 86 insertions(+), 53 deletions(-) diff --git a/crates/keymap/src/ui/keyboard.rs b/crates/keymap/src/ui/keyboard.rs index 32e92ad6..0d0bc2a5 100644 --- a/crates/keymap/src/ui/keyboard.rs +++ b/crates/keymap/src/ui/keyboard.rs @@ -366,7 +366,7 @@ impl Draw<'_, '_> { /// The holder's label, with its scope when that is not Application. fn holder_text(&self, h: &KeyHolder) -> String { let label = self.label_of(&h.holder.command); - if h.scope.is_app() { label } else { format!("{label} ({})", self.scope_name(&h.scope)) } + if h.scope.is_app() { label } else { fill(self.ui("{label} ({scope})"), &[("label", &label), ("scope", &self.scope_name(&h.scope))]) } } pub(super) fn keyboard_pane(&mut self, ui: &mut Ui) { diff --git a/crates/keymap/src/ui/review.rs b/crates/keymap/src/ui/review.rs index 006d9405..61ef3da7 100644 --- a/crates/keymap/src/ui/review.rs +++ b/crates/keymap/src/ui/review.rs @@ -84,7 +84,7 @@ impl Draw<'_, '_> { return; } ui.add_space(4.0); - ui.label(RichText::new(self.vis(&format!("{title} ({})", entries.len()))).strong()); + ui.label(RichText::new(self.vis(&fill(self.ui("{title} ({n})"), &[("title", title), ("n", &entries.len().to_string())]))).strong()); for e in entries { self.entry(ui, e); } @@ -181,7 +181,8 @@ impl Draw<'_, '_> { if !report.suggestions.is_empty() { ui.add_space(4.0); - ui.label(RichText::new(self.vis(&format!("{} ({})", self.ui("Suggestions"), report.suggestions.len()))).strong()); + let title = fill(self.ui("Suggestions ({n})"), &[("n", &report.suggestions.len().to_string())]); + ui.label(RichText::new(self.vis(&title)).strong()); for (si, s) in report.suggestions.iter().enumerate() { ui.horizontal_wrapped(|ui| { self.entry( diff --git a/crates/keymap/src/ui/rows.rs b/crates/keymap/src/ui/rows.rs index 5bc3e0c2..2f85cf31 100644 --- a/crates/keymap/src/ui/rows.rs +++ b/crates/keymap/src/ui/rows.rs @@ -7,7 +7,7 @@ use crate::chord::{Platform, Trigger}; use crate::command::CommandRef; use crate::keymap::{Bindable, Binding, ConflictKind, Keymap, Kind, Overrides}; use crate::ui::Options; -use crate::ui::text::{Text, trigger_text}; +use crate::ui::text::{Text, trigger_text, visual_around}; /// One entry of the Show menu. #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -375,9 +375,9 @@ fn build(keymap: &Keymap, options: &Options, text: &dyn Text, view: &View) -> Bu .map(|x| { let mut t = trigger_text(text, &x.trigger, platform); if x.scope_or(&b.scope) != &b.scope { - let scope = format!("({})", text.segment(x.scope_or(&b.scope).as_str())); - // Display order; the chord itself is never reordered. - t = if text.rtl() { format!("{} {t}", text.visual(&scope)) } else { format!("{t} {scope}") }; + let scope = text.segment(x.scope_or(&b.scope).as_str()); + let s = craft_i18n::fmt(&text.ui("{chord} ({scope})"), &[("chord", &t), ("scope", &scope)]); + t = visual_around(text, &s, &t); } Chip { binding: x.clone(), text: t, changed: !defaults.contains(x) } }) diff --git a/crates/keymap/src/ui/state.rs b/crates/keymap/src/ui/state.rs index c53a12d2..701ba546 100644 --- a/crates/keymap/src/ui/state.rs +++ b/crates/keymap/src/ui/state.rs @@ -3,11 +3,11 @@ use std::collections::BTreeSet; -use crate::chord::{Chord, Key, Trigger}; +use crate::chord::{Chord, Key, ParseError, Trigger}; use crate::command::CommandRef; use crate::editor::{self, DOUBLE_PRESS, EditSession, Recorded, Recorder}; use crate::import::{self, ForeignSet, ImportOptions, ImportPlan, ImportReport}; -use crate::keymap::{AssignError, Assigned, Binding, Holder, Keymap, MAX_BINDINGS, Overrides, Policy}; +use crate::keymap::{AssignError, Assigned, Binding, BindingError, Holder, Keymap, MAX_BINDINGS, Overrides, Policy}; use crate::profiles::{Builtins, ProfileError, Profiles, set_name}; use crate::scope::Scope; use crate::ui::rows::{AreaEntry, AreaId, Built, Filter, RowCache, View}; @@ -38,7 +38,7 @@ impl Env<'_> { for h in holders { let label = self.label(&h.command); let s = match &h.binding.scope { - Some(scope) => format!("{label} ({})", self.text.segment(scope.as_str())), + Some(scope) => self.ui("{label} ({scope})", &[("label", &label), ("scope", &self.text.segment(scope.as_str()))]), None => label, }; if !out.contains(&s) { @@ -501,9 +501,9 @@ impl DialogState { } /// Typed entry: the text as [`Binding::parse`] reads it, committed like a recorded chord. - /// `Err` is the parse error for under the field. + /// `Err` is the parse error for under the field, translated. pub(crate) fn type_binding(&mut self, env: &mut Env, command: &CommandRef, text: &str) -> Result<(), String> { - let binding = Binding::parse(text).map_err(|e| e.to_string())?; + let binding = Binding::parse(text).map_err(|e| binding_error(env, &e))?; self.select_row(command); if let Some(scope) = binding.scope.clone() { self.context = Some(scope); @@ -1097,6 +1097,23 @@ fn assign_error(env: &Env, e: &AssignError) -> String { } } +/// Why a binding text does not parse, translated. +pub(crate) fn binding_error(env: &Env, e: &BindingError) -> String { + match e { + BindingError::TooLong => env.ui("the shortcut is too long", &[]), + BindingError::Scope => env.ui("the scope name is not valid", &[]), + BindingError::Trigger(e) => match e { + ParseError::Empty => env.ui("empty shortcut", &[]), + ParseError::TooLong => env.ui("the shortcut is too long", &[]), + ParseError::MissingKey => env.ui("the shortcut has no key", &[]), + ParseError::UnknownKey(k) => env.ui("unknown key {key}", &[("key", k)]), + ParseError::UnknownModifier(m) => env.ui("unknown modifier {key}", &[("key", m)]), + ParseError::DuplicateModifier(m) => env.ui("modifier {key} given twice", &[("key", m)]), + ParseError::Sequence => env.ui("key sequences are not supported", &[]), + }, + } +} + fn profile_error(env: &Env, e: &ProfileError) -> String { match e { ProfileError::BadName => env.ui("The name is empty, too long or has control characters", &[]), @@ -1327,7 +1344,7 @@ mod tests { assert_eq!(st.type_binding(&mut fx.env(), &"file.save".into(), "F7 F7"), Ok(())); st.assign_field(&mut fx.env()); assert_eq!(fx.keys("file.save"), ["Cmd+S", "F7 F7"]); - assert!(st.type_binding(&mut fx.env(), &"file.save".into(), "Cmd+Nope").is_err()); + assert_eq!(st.type_binding(&mut fx.env(), &"file.save".into(), "Cmd+Nope"), Err("unknown key Nope".into())); } #[test] diff --git a/crates/keymap/src/ui/summary.rs b/crates/keymap/src/ui/summary.rs index 8b4248ca..01603962 100644 --- a/crates/keymap/src/ui/summary.rs +++ b/crates/keymap/src/ui/summary.rs @@ -21,7 +21,7 @@ fn binding_text(km: &Keymap, text: &dyn Text, b: &Binding, home: &Scope) -> Stri match &b.scope { Some(s) if s != home => { let scope = if s.is_app() { text.ui("Application").into_owned() } else { text.segment(s.as_str()).into_owned() }; - format!("{chord} ({scope})") + craft_i18n::fmt(&text.ui("{chord} ({scope})"), &[("chord", &chord), ("scope", &scope)]) } _ => chord, } diff --git a/crates/keymap/src/ui/text.rs b/crates/keymap/src/ui/text.rs index 2550e21e..d248c5c5 100644 --- a/crates/keymap/src/ui/text.rs +++ b/crates/keymap/src/ui/text.rs @@ -104,6 +104,15 @@ pub fn trigger_text(text: &dyn Text, trigger: &Trigger, platform: Platform) -> S if trigger.double { format!("{one} {one}") } else { one } } +/// `s` in display order with `chord`, text inside it, left as it is: chord text never goes +/// through [`Text::visual`]. Right to left, the parts before and after the chord change sides. +pub(crate) fn visual_around(text: &dyn Text, s: &str, chord: &str) -> String { + let Some(i) = s.find(chord).filter(|_| !chord.is_empty()) else { return text.visual(s) }; + let (before, rest) = s.split_at(i); + let (before, after) = (text.visual(before), text.visual(&rest[chord.len()..])); + if text.rtl() { format!("{after}{chord}{before}") } else { format!("{before}{chord}{after}") } +} + /// Plural forms the dialog looks up, as `one|other`, the key of craft-i18n's `@plural` rows. pub const PLURALS: &[(&str, &str)] = &[ ("{n} shortcut is assigned more than once", "{n} shortcuts are assigned more than once"), @@ -149,6 +158,8 @@ pub const STRINGS: &[&str] = &[ "Conflicts", "No matching commands", "(cycles)", + "{label} (cycles)", + "{label} (Changed)", // Table and detail pane. "Command", "Commands", @@ -156,9 +167,13 @@ pub const STRINGS: &[&str] = &[ "Select a command to see and change its shortcuts", "Shortcut", "Default", + "{chord} (Default)", + "{chord} ({scope})", + "{label} ({scope})", "Application", "Current shortcuts:", "New shortcut:", + "New shortcut: {chord}", "press keys", "{chord} (press again for a double press)", "Type…", @@ -166,7 +181,6 @@ pub const STRINGS: &[&str] = &[ "Assign", "Use Default", "Clear", - "Remove", "Default: {chord}", "None", "Currently assigned to:", @@ -178,6 +192,13 @@ pub const STRINGS: &[&str] = &[ "{label}: {chord}", "{label}: press keys", "Now used by {label}", + "{label}: {chord} (Now used by {holder})", + // Typed shortcuts and saved texts that do not parse. + "the shortcut has no key", + "unknown modifier {key}", + "modifier {key} given twice", + "key sequences are not supported", + "the scope name is not valid", // On-screen keyboard. "Keyboard", "Keyboard: US", @@ -263,7 +284,8 @@ pub const STRINGS: &[&str] = &[ "Not matched", "Unreadable", "Approximated", - "Suggestions", + "{title} ({n})", + "Suggestions ({n})", "numeric keypad key read as main key", "Matched without change", "Command for {item}", diff --git a/crates/keymap/src/ui/view.rs b/crates/keymap/src/ui/view.rs index 373bbb82..155ee2e9 100644 --- a/crates/keymap/src/ui/view.rs +++ b/crates/keymap/src/ui/view.rs @@ -24,9 +24,9 @@ use crate::keymap::{Binding, MAX_BINDINGS, Problem}; use crate::scope::Scope; use crate::ui::capture; use crate::ui::rows::{AreaId, Filter, FixedLine, Line, Row}; -use crate::ui::state::{Answer, DialogState, Env, Prompt, Recording, Target}; +use crate::ui::state::{Answer, DialogState, Env, Prompt, Recording, Target, binding_error}; use crate::ui::style::{self, DialogStyle}; -use crate::ui::text::trigger_text; +use crate::ui::text::{trigger_text, visual_around}; #[path = "keyboard.rs"] pub(crate) mod keyboard; @@ -165,13 +165,7 @@ impl Draw<'_, '_> { /// `s` in display order with `chord`, text inside it, left as it is: chord text never goes /// through [`Text::visual`](crate::ui::Text::visual). fn vis_chord(&self, s: &str, chord: &str) -> String { - match s.find(chord).filter(|_| !chord.is_empty()) { - None => self.vis(s), - Some(i) => { - let (before, rest) = s.split_at(i); - self.joined(vec![self.vis(before), chord.to_string(), self.vis(&rest[chord.len()..])]) - } - } + visual_around(self.env.text, s, chord) } /// `text` after a ⚠, in display order. @@ -192,20 +186,11 @@ impl Draw<'_, '_> { if s.is_app() { self.ui("Application") } else { self.env.text.segment(s.as_str()).into_owned() } } - /// [`binding_text`](Self::binding_text) in display order. - fn binding_shown(&self, b: &Binding, home: Option<&Scope>) -> String { - let chord = self.chord(&b.trigger); - match &b.scope { - Some(s) if Some(s) != home => self.joined(vec![chord, " ".into(), self.vis(&format!("({})", self.scope_name(s)))]), - _ => chord, - } - } - - /// A binding as text, with ` (Scope)` when its scope is not the command's home scope. + /// A binding as text, with its scope when that is not the command's home scope. fn binding_text(&self, b: &Binding, home: Option<&Scope>) -> String { let chord = self.chord(&b.trigger); match &b.scope { - Some(s) if Some(s) != home => format!("{chord} ({})", self.scope_name(s)), + Some(s) if Some(s) != home => fill(self.ui("{chord} ({scope})"), &[("chord", &chord), ("scope", &self.scope_name(s))]), _ => chord, } } @@ -832,10 +817,10 @@ impl Draw<'_, '_> { let resp = ui.interact(rect, self.base.with(("row", &r.key)), Sense::CLICK); let mut a11y = r.label.clone(); if r.cycles { - a11y = format!("{a11y} {}", self.ui("(cycles)")); + a11y = fill(self.ui("{label} (cycles)"), &[("label", &a11y)]); } if r.changed { - a11y = format!("{a11y} ({})", self.ui("Changed")); + a11y = fill(self.ui("{label} (Changed)"), &[("label", &a11y)]); } resp.widget_info(|| WidgetInfo::selected(WidgetType::SelectableLabel, true, selected, &a11y)); ui.ctx().accesskit_node_builder(resp.id, |node| node.set_selected(selected)); @@ -983,18 +968,20 @@ impl Draw<'_, '_> { let (keys, _) = self.one_line(ui.painter(), &[(f.keys.clone(), fmt)], shortcut_w); ui.painter().galley(pos2(m.x(rect.max.x - shortcut_w - ICON - 4.0, keys.size().x), rect.center().y - keys.size().y / 2.0), keys, color); let resp = ui.interact(rect, self.base.with(("fixed", &f.label, &f.keys)), Sense::hover()); - let label = fill(self.ui("{label}: {chord}"), &[("label", &f.label), ("chord", &f.keys)]); let mut tip = Vec::new(); if cut { tip.push(self.vis(&f.label)); } match &f.now_used_by { Some(by) => { - let used = fill(self.ui("Now used by {label}"), &[("label", by)]); - resp.widget_info(|| WidgetInfo::labeled(WidgetType::Label, true, format!("{label} ({used})"))); - tip.push(self.vis(&used)); + let label = fill(self.ui("{label}: {chord} (Now used by {holder})"), &[("label", &f.label), ("chord", &f.keys), ("holder", by)]); + resp.widget_info(|| WidgetInfo::labeled(WidgetType::Label, true, &label)); + tip.push(self.vis(&fill(self.ui("Now used by {label}"), &[("label", by)]))); + } + None => { + let label = fill(self.ui("{label}: {chord}"), &[("label", &f.label), ("chord", &f.keys)]); + resp.widget_info(|| WidgetInfo::labeled(WidgetType::Label, true, &label)); } - None => resp.widget_info(|| WidgetInfo::labeled(WidgetType::Label, true, &label)), } if !tip.is_empty() { resp.on_hover_text(tip.join("\n")); @@ -1040,12 +1027,8 @@ impl Draw<'_, '_> { let text = self.binding_text(binding, home); ui.horizontal(|ui| { ui.add_space(8.0); - let shown = if defaults.contains(binding) { - self.joined(vec![self.binding_shown(binding, home), " ".into(), self.vis(&format!("({})", self.ui("Default")))]) - } else { - self.binding_shown(binding, home) - }; - ui.label(shown); + let shown = if defaults.contains(binding) { fill(self.ui("{chord} (Default)"), &[("chord", &text)]) } else { text.clone() }; + ui.label(self.vis_chord(&shown, &self.chord(&binding.trigger))); if editable { let a11y = fill(self.ui("Remove {chord} from {label}"), &[("chord", &text), ("label", &label)]); let r = self.icon_button(ui, &a11y, false, paint_cross).on_hover_text(self.vis(&a11y)); @@ -1117,7 +1100,7 @@ impl Draw<'_, '_> { } } if let Some(e) = &self.state.type_error { - ui.add(Label::new(RichText::new(format!("⚠ {e}")).color(self.style.warning)).wrap()); + ui.add(Label::new(RichText::new(self.warned(self.vis(e))).color(self.style.warning)).wrap()); } return; } @@ -1130,7 +1113,8 @@ impl Draw<'_, '_> { let size = vec2(FIELD_W, ui.spacing().interact_size.y); let (rect, _) = ui.allocate_exact_size(size, Sense::hover()); let resp = ui.interact(rect, field_id, Sense::click()); - let a11y = format!("{title} {}", if shown.is_empty() { self.ui("None") } else { shown.clone() }); + let value = if shown.is_empty() { self.ui("None") } else { shown.clone() }; + let a11y = fill(self.ui("New shortcut: {chord}"), &[("chord", &value)]); resp.widget_info(|| WidgetInfo::labeled(WidgetType::Button, true, &a11y)); let v = ui.visuals(); let stroke = if recording || resp.has_focus() { v.selection.stroke } else { v.widgets.inactive.bg_stroke }; @@ -1201,7 +1185,7 @@ impl Draw<'_, '_> { let label = self.label_of(&h.command); let home = self.env.keymap.bindable(&h.command).map(|b| b.scope.clone()); let text = match &h.binding.scope { - Some(s) if Some(s) != home.as_ref() => format!("{label} ({})", self.scope_name(s)), + Some(s) if Some(s) != home.as_ref() => fill(self.ui("{label} ({scope})"), &[("label", &label), ("scope", &self.scope_name(s))]), _ => label, }; ui.horizontal(|ui| { @@ -1361,7 +1345,8 @@ impl Draw<'_, '_> { match p { Problem::UnknownCommand { key } => fill(self.ui("{key}: no command has this name"), &[("key", key)]), Problem::BadBinding { key, text, error } => { - fill(self.ui("{key}: “{text}” is not a shortcut ({error})"), &[("key", key), ("text", text), ("error", &error.to_string())]) + let error = binding_error(self.env, error); + fill(self.ui("{key}: “{text}” is not a shortcut ({error})"), &[("key", key), ("text", text), ("error", &error)]) } Problem::Reserved { key, text } => fill(self.ui("{key}: {text} is reserved by the system"), &[("key", key), ("text", text)]), Problem::UnknownScope { key, text } => fill(self.ui("{key}: the scope in “{text}” is unknown"), &[("key", key), ("text", text)]), diff --git a/crates/keymap/tests/ui_dialog.rs b/crates/keymap/tests/ui_dialog.rs index aed56d40..64500ac2 100644 --- a/crates/keymap/tests/ui_dialog.rs +++ b/crates/keymap/tests/ui_dialog.rs @@ -549,9 +549,17 @@ fn typed_entry_parses_and_shows_errors() { h.get_by_label("Save").click(); h.run(); click(&mut h, "Type…"); + h.get_by_role_and_label(Role::TextInput, "New shortcut:").type_text("Cmd+Nope"); + h.run(); + press(&mut h, Modifiers::NONE, EguiKey::Enter); + assert!(has(&h, "⚠ unknown key Nope")); + h.get_by_role_and_label(Role::TextInput, "New shortcut:").focus(); + h.run(); + press(&mut h, Modifiers::COMMAND, EguiKey::A); h.get_by_role_and_label(Role::TextInput, "New shortcut:").type_text("Cmd+Shift+K"); h.run(); press(&mut h, Modifiers::NONE, EguiKey::Enter); + assert!(!has(&h, "⚠ unknown key Nope")); click(&mut h, "Assign"); assert_eq!(h.state().bindings("file.save"), ["Cmd+S", "Cmd+Shift+K"]); }