From e2d0c328fac1119675c9a236086c63555bca5127 Mon Sep 17 00:00:00 2001 From: Julian Orth Date: Thu, 23 Jan 2025 12:07:29 +0100 Subject: [PATCH] xkb: by default, don't format multiple actions per level --- compile-tests/src/main.rs | 7 ++-- kbvm-cli/src/compile_rmlvo.rs | 2 +- kbvm-cli/src/compile_xkb.rs | 2 +- kbvm-cli/src/output/ansi.rs | 2 +- kbvm-cli/src/output/json.rs | 2 +- kbvm/release-notes.md | 29 ++++++++++++++++ kbvm/src/xkb/format.rs | 12 ++++++- kbvm/src/xkb/keymap.rs | 1 + kbvm/src/xkb/keymap/format.rs | 12 +++++++ kbvm/src/xkb/keymap/format/map6.xkb | 16 +++++++++ kbvm/src/xkb/keymap/format/map7.xkb | 51 +++++++++++++++++++++++++++++ kbvm/src/xkb/keymap/format/map8.xkb | 51 +++++++++++++++++++++++++++++ kbvm/src/xkb/keymap/format/tests.rs | 15 +++++++++ 13 files changed, 195 insertions(+), 7 deletions(-) create mode 100644 kbvm/release-notes.md create mode 100644 kbvm/src/xkb/keymap/format/map6.xkb create mode 100644 kbvm/src/xkb/keymap/format/map7.xkb create mode 100644 kbvm/src/xkb/keymap/format/map8.xkb diff --git a/compile-tests/src/main.rs b/compile-tests/src/main.rs index cd9cff79..f9fc5deb 100644 --- a/compile-tests/src/main.rs +++ b/compile-tests/src/main.rs @@ -129,7 +129,7 @@ fn test_kccgst(mut diagnostics: &mut Vec, case: &Path) -> Result<(), // let builder = map_actual.to_builder().build_state_machine(); // println!("{:#?}", builder); - let map = format!("{:#}\n", map_actual.format()); + let map = format!("{}\n", map_actual.format().multiple_actions_per_level(true)); let expected_path = case.join("expected.xkb"); let expected = match std::fs::read(&expected_path) { @@ -165,7 +165,10 @@ fn test_kccgst(mut diagnostics: &mut Vec, case: &Path) -> Result<(), }); } - let round_trip = format!("{:#}\n", map_expected.format()); + let round_trip = format!( + "{}\n", + map_expected.format().multiple_actions_per_level(true), + ); if round_trip.as_bytes() != expected { let rt = case.join("round_trip.xkb"); std::fs::write(&rt, &round_trip).map_err(ResultError::WriteRoundTripFailed)?; diff --git a/kbvm-cli/src/compile_rmlvo.rs b/kbvm-cli/src/compile_rmlvo.rs index 467a1431..e078880e 100644 --- a/kbvm-cli/src/compile_rmlvo.rs +++ b/kbvm-cli/src/compile_rmlvo.rs @@ -24,5 +24,5 @@ pub fn main(args: CompileRmlvoArgs) { groups.as_deref(), options.as_deref(), ); - println!("{:#}", expanded.format()); + println!("{}", expanded.format().multiple_actions_per_level(true)); } diff --git a/kbvm-cli/src/compile_xkb.rs b/kbvm-cli/src/compile_xkb.rs index dcca469d..6d97220f 100644 --- a/kbvm-cli/src/compile_xkb.rs +++ b/kbvm-cli/src/compile_xkb.rs @@ -24,7 +24,7 @@ pub fn main(args: CompileXkbArgs) { let expanded = context.keymap_from_bytes(WriteToLog, Some(path.as_ref()), &source); match expanded { Ok(map) => { - println!("{:#}", map.format()); + println!("{}", map.format().multiple_actions_per_level(true)); } Err(_) => { log::error!("could not compile keymap"); diff --git a/kbvm-cli/src/output/ansi.rs b/kbvm-cli/src/output/ansi.rs index ffc6353e..d70394c1 100644 --- a/kbvm-cli/src/output/ansi.rs +++ b/kbvm-cli/src/output/ansi.rs @@ -71,7 +71,7 @@ impl Output for Ansi { "{} {} {:#}", self.now(), "keymap".color_time(self.theme), - keymap.format(), + keymap.format().multiple_actions_per_level(true), ); } diff --git a/kbvm-cli/src/output/json.rs b/kbvm-cli/src/output/json.rs index c562069a..97556cf0 100644 --- a/kbvm-cli/src/output/json.rs +++ b/kbvm-cli/src/output/json.rs @@ -96,7 +96,7 @@ impl Json { impl Output for Json { fn keymap(&mut self, keymap: &Keymap) { self.msg(Type::Keymap { - map: format!("{:#}", keymap.format()), + map: format!("{}", keymap.format().multiple_actions_per_level(true)), }); } diff --git a/kbvm/release-notes.md b/kbvm/release-notes.md new file mode 100644 index 00000000..39638ce9 --- /dev/null +++ b/kbvm/release-notes.md @@ -0,0 +1,29 @@ +# Unreleased + +- Levels with multiple actions are no longer formatted by default. For example, + + ```xkb + xkb_symbols { + key { + [ + { + SetMods(mods = Mod2), + SetGroup(group = +1), + } + ] + }; + }; + ``` + + will be formatted as + + ```xkb + xkb_symbols { + key { [ NoAction() ] }; + }; + ``` + + This is because xkbcommon and Xwayland will reject the entire keymap if there are + multiple actions per level. + + This can be reverted with the `multiple_actions_per_level` flag. diff --git a/kbvm/src/xkb/format.rs b/kbvm/src/xkb/format.rs index 300ceaf1..771d7e7a 100644 --- a/kbvm/src/xkb/format.rs +++ b/kbvm/src/xkb/format.rs @@ -41,6 +41,7 @@ where nesting: 0, multi_line: f.alternate(), lookup_only: false, + omit_multiple_actions: false, long_keys: None, newline: match f.alternate() { true => "\n", @@ -56,6 +57,7 @@ struct Writer<'a, 'b> { nesting: usize, multi_line: bool, lookup_only: bool, + omit_multiple_actions: bool, long_keys: Option, String>>, newline: &'static str, f: &'a mut Formatter<'b>, @@ -177,6 +179,7 @@ impl Display for keymap::Formatter<'_> { nesting: 0, multi_line: !self.single_line, lookup_only: self.lookup_only, + omit_multiple_actions: !self.multiple_actions_per_level, long_keys, newline: match self.single_line { false => "\n", @@ -700,6 +703,7 @@ impl Format for Keys<'_> { for (offset, group) in key.groups.iter().enumerate() { if let Some(group) = group { let idx = offset + 1; + #[expect(clippy::too_many_arguments)] fn write_levels( needs_newline: &mut bool, idx: usize, @@ -708,6 +712,7 @@ impl Format for Keys<'_> { name: &str, absent: &str, field: impl Fn(&KeyLevel) -> &SmallVec<[T; 1]>, + is_actions: bool, ) -> fmt::Result { if group.levels.iter().all(|l| field(l).is_empty()) { return Ok(()); @@ -716,7 +721,10 @@ impl Format for Keys<'_> { f.write_nesting()?; write!(f.f, "{name}[Group{idx}] = [")?; f.write_inline_list(&group.levels, |f, l| { - let list = field(l); + let mut list = &**field(l); + if is_actions && f.omit_multiple_actions && list.len() > 1 { + list = &[]; + } if list.is_empty() { f.write(absent)?; } else if list.len() == 1 { @@ -740,6 +748,7 @@ impl Format for Keys<'_> { "symbols", "NoSymbol", |l| &l.symbols, + false, )?; if !f.lookup_only { write_levels( @@ -750,6 +759,7 @@ impl Format for Keys<'_> { "actions", "NoAction()", |l| &l.actions, + true, )?; } } diff --git a/kbvm/src/xkb/keymap.rs b/kbvm/src/xkb/keymap.rs index 4b1e9c57..6e0e0d1a 100644 --- a/kbvm/src/xkb/keymap.rs +++ b/kbvm/src/xkb/keymap.rs @@ -724,6 +724,7 @@ impl Keymap { keymap: self, single_line: false, lookup_only: false, + multiple_actions_per_level: false, rename_long_keys: false, } } diff --git a/kbvm/src/xkb/keymap/format.rs b/kbvm/src/xkb/keymap/format.rs index 78f136d8..34d61644 100644 --- a/kbvm/src/xkb/keymap/format.rs +++ b/kbvm/src/xkb/keymap/format.rs @@ -20,6 +20,7 @@ pub struct Formatter<'a> { pub(crate) keymap: &'a Keymap, pub(crate) single_line: bool, pub(crate) lookup_only: bool, + pub(crate) multiple_actions_per_level: bool, pub(crate) rename_long_keys: bool, } @@ -53,6 +54,17 @@ impl Formatter<'_> { self.rename_long_keys = val; self } + + /// Enables or disables formatting of levels containing more than one action. + /// + /// By default, such actions are not formatted. + /// + /// This should almost always be disabled. Xwayland and libxkbcommon will fail to + /// parse keymaps that contain levels with multiple actions. + pub fn multiple_actions_per_level(mut self, val: bool) -> Self { + self.multiple_actions_per_level = val; + self + } } impl Debug for Formatter<'_> { diff --git a/kbvm/src/xkb/keymap/format/map6.xkb b/kbvm/src/xkb/keymap/format/map6.xkb new file mode 100644 index 00000000..af421d87 --- /dev/null +++ b/kbvm/src/xkb/keymap/format/map6.xkb @@ -0,0 +1,16 @@ +xkb_keymap { + xkb_keycodes { + = 1; + = 2; + }; + xkb_symbols { + key { + [ { a, b } ], + [ { SetMods(mods = Mod2), SetMods(mods = Mod3) } ], + }; + key { + [ a ], + [ SetMods(mods = Mod2), SetMods(mods = Mod3) ], + }; + }; +}; diff --git a/kbvm/src/xkb/keymap/format/map7.xkb b/kbvm/src/xkb/keymap/format/map7.xkb new file mode 100644 index 00000000..5ef8fbae --- /dev/null +++ b/kbvm/src/xkb/keymap/format/map7.xkb @@ -0,0 +1,51 @@ +xkb_keymap { + xkb_keycodes { + minimum = 8; + maximum = 255; + + indicator 1 = "DUMMY"; + + = 1; + = 2; + }; + + xkb_types { + virtual_modifiers Dummy; + + type "ONE_LEVEL" { + modifiers = None; + level_name[Level1] = "Any"; + map[None] = Level1; + }; + + type "TWO_LEVEL" { + modifiers = Shift; + level_name[Level1] = "Base"; + level_name[Level2] = "Shift"; + map[Shift] = Level2; + }; + }; + + xkb_compat { + interpret VoidSymbol { + repeat = false; + }; + }; + + xkb_symbols { + key.repeat = true; + + key { + repeat = false, + type[Group1] = "ONE_LEVEL", + symbols[Group1] = [ { a, b } ], + actions[Group1] = [ { SetMods(modifiers = Mod2), SetMods(modifiers = Mod3) } ] + }; + key { + repeat = false, + type[Group1] = "TWO_LEVEL", + symbols[Group1] = [ a, NoSymbol ], + actions[Group1] = [ SetMods(modifiers = Mod2), SetMods(modifiers = Mod3) ] + }; + }; +}; diff --git a/kbvm/src/xkb/keymap/format/map8.xkb b/kbvm/src/xkb/keymap/format/map8.xkb new file mode 100644 index 00000000..7a74806c --- /dev/null +++ b/kbvm/src/xkb/keymap/format/map8.xkb @@ -0,0 +1,51 @@ +xkb_keymap { + xkb_keycodes { + minimum = 8; + maximum = 255; + + indicator 1 = "DUMMY"; + + = 1; + = 2; + }; + + xkb_types { + virtual_modifiers Dummy; + + type "ONE_LEVEL" { + modifiers = None; + level_name[Level1] = "Any"; + map[None] = Level1; + }; + + type "TWO_LEVEL" { + modifiers = Shift; + level_name[Level1] = "Base"; + level_name[Level2] = "Shift"; + map[Shift] = Level2; + }; + }; + + xkb_compat { + interpret VoidSymbol { + repeat = false; + }; + }; + + xkb_symbols { + key.repeat = true; + + key { + repeat = false, + type[Group1] = "ONE_LEVEL", + symbols[Group1] = [ { a, b } ], + actions[Group1] = [ NoAction() ] + }; + key { + repeat = false, + type[Group1] = "TWO_LEVEL", + symbols[Group1] = [ a, NoSymbol ], + actions[Group1] = [ SetMods(modifiers = Mod2), SetMods(modifiers = Mod3) ] + }; + }; +}; diff --git a/kbvm/src/xkb/keymap/format/tests.rs b/kbvm/src/xkb/keymap/format/tests.rs index 06843dc2..99985ffd 100644 --- a/kbvm/src/xkb/keymap/format/tests.rs +++ b/kbvm/src/xkb/keymap/format/tests.rs @@ -24,6 +24,7 @@ fn single_line() { let expected = include_str!("map3.xkb"); assert_eq!(actual, expected); } + #[test] fn lookup_only() { let context = Context::default(); @@ -34,3 +35,17 @@ fn lookup_only() { let expected = include_str!("map5.xkb"); assert_eq!(actual, expected); } + +#[test] +fn multiple_actions() { + let context = Context::default(); + let map = context + .keymap_from_bytes(WriteToStderr, None, include_str!("map6.xkb")) + .unwrap(); + let with_actions = format!("{}\n", map.format().multiple_actions_per_level(true)); + let expected = include_str!("map7.xkb"); + assert_eq!(with_actions, expected); + let without_actions = format!("{}\n", map.format()); + let expected = include_str!("map8.xkb"); + assert_eq!(without_actions, expected); +}