From 7776832ec11ee7d4e62cfd2a6ad7735f323ab5bc Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Sat, 3 Nov 2018 00:03:40 +0100 Subject: parser: Parse `Action::Map`s at the same time as everything else Was having a hell of a time trying to return a `Result` from `MapAction::parse()` in 75d52e385fa66d6e151c9baa2cf22c2223c39ff0. After struggling with it, I started to think if there was a different way to go about this. Previously I had tried and failed to parse map actions in the main parser. If I could get that working, then this whole `MapAction::parse()` method would be obsolete. No need to muss with `Result`s and borrowed strings at all. When I originally wrote the parser code, I thought in terms of: "How do I get the parsed result of the `map()` parser into the `action()` parser so that it can conditionally parse one way for the `map` keyword, and another for the `cmd` keyword?". In hindsight, this was incredibly idiotic, but I guess I was too close to the code and too deep in the details. Obviously the answer to parsing differently depending on `map` or `cmd` is not to get the parsed result of `map()` into `action()`. These are parser combinators. _Of course_ we can just make two map definition parsers, one for `map` and another for `cmd`. Can't believe I never thought of that last time. Who knows how much time I wasted trying and failing, then writing a sub-optimal solution. Oh well, I guess I should just be thankful that a niggling logging problem (in `MapAction::parse()`) that turned into a maddening journey to a giant brick wall in search of a `Result` return value with correct borrows finally enlightened me to the fact that I was doing the wrong thing all along. * Comment out double-parsing code related to `MapAction::parse()` because we want to parse in a single go. No more two-pass parsing. * Split the `map_kind()` parser into two separate parsers for `map` and `cmd`. Similarly, split the `map()` parser into `map_map()` and `map_cmd()` for the same reason. Our new parser tree looks like this: map(): map definition line -> map_map(): `map` definitions -> map_kind_map(): `map` -> map_cmd(): `cmd` definitions -> map_kind_cmd(): `cmd` * Up the `recursion_limit` to 256 from 128 because I was getting stack size errors randomly when I was updating the tests for this code change. * Update `action_character()` to not parse `\n` newlines. Otherwise the newline gets included as a real `Character` in the `KeyboardKeyWithModifiers` vector. * The `map_collection_fails_without_terminating_newline()` test fails as a result of the `action_character()` parser change. Previously it relied on `map` lines parsing to `Action::String`s, which fail if there's no terminating newline. Now that `map`s parse to `Action::Map`s, the newline isn't checked by the map line parser (`map_map()`). Because the newline doesn't matter for `map` definitions, the parser would return `Ok`, failing this test even though the behaviour was correct. Change the map line to a `cmd` to have the test check the behaviour it was previously testing. * Update the tests to use parsed `Action::Map` instead of `Action::String`s where appropriate. Used this program to convert the strings into the correct source code: # action_string_to_action_map.py #!/usr/bin/env python3 import sys action_string = sys.argv[1] action = '' action += 'action: Action::Map(vec![\n' for c in action_string: action += ' KeyboardKeyWithModifiers::new(\n' action += " KeyboardKey::Character(Character::new('{}')),\n".format(c) action += ' vec![],\n' action += ' ),\n' action += ']),' print(action) --- src/ffi.rs | 2 +- src/lib.rs | 2 +- src/parser.rs | 372 +++++++++++++++++++++++++++++++++++++++++++++------------- 3 files changed, 294 insertions(+), 82 deletions(-) (limited to 'src') diff --git a/src/ffi.rs b/src/ffi.rs index f8a95c2..e0cd0af 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -74,7 +74,7 @@ pub extern "C" fn dome_key_state_load_map_group(ptr: *mut State) { state.map_group = match MapGroup::parse(&state.mappings_str) { Ok(mut map_group) => { - map_group.parse_actions(); + // map_group.parse_actions(); Some(map_group) }, Err(e) => { diff --git a/src/lib.rs b/src/lib.rs index 20f4fb6..1843e65 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -#![recursion_limit="128"] +#![recursion_limit="256"] extern crate autopilot; extern crate chrono; diff --git a/src/parser.rs b/src/parser.rs index f943ab3..90004ed 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -141,71 +141,71 @@ pub struct MapAction { } impl MapAction { - pub fn parse( - &mut self, - ) -> Result<(), CombineErrors> { - use std::mem; - match self.kind { - MapKind::Map => { - // match self.action { - // Action::String(ref s) => { - // let input = State::new(s.as_str()); - // - // // match action_map() - // // .easy_parse(input) - // // .map(|t| t.0) - // // { - // // Ok(a) => Some(a), - // // Err(e) => { - // // error!("{}", e); - // // - // // None - // // }, - // // } - // let parsed_action = action_map() - // .easy_parse(input) - // .map(|t| t.0)?; - // - // }, - // _ => (), - // }; - - // let yo = self.action; - // let parsed_action = self.action.parse()?; - let yo = self.action.clone(); - let parsed_action = match yo { - Action::String(s) => { - let input = State::new(s.as_str()); - - action_map() - .easy_parse(input) - .map(|t| t.0) - .map(|action| Some(action)) - }, - _ => Ok(None), - }?; - - if let Some(action) = parsed_action { - // self.action = action; - mem::replace(&mut self.action, action); - } - - // self.action = { - // let parsed_action = self.action.parse()?; - // - // match parsed_action { - // Some(a) => a, - // None => self.action, - // } - // }; - }, - - // Commands don't get parsed. They remain `Action::String`s. - MapKind::Command => (), - }; - - Ok(()) - } + // pub fn parse( + // &mut self, + // ) -> Result<(), CombineErrors> { + // use std::mem; + // match self.kind { + // MapKind::Map => { + // // match self.action { + // // Action::String(ref s) => { + // // let input = State::new(s.as_str()); + // // + // // // match action_map() + // // // .easy_parse(input) + // // // .map(|t| t.0) + // // // { + // // // Ok(a) => Some(a), + // // // Err(e) => { + // // // error!("{}", e); + // // // + // // // None + // // // }, + // // // } + // // let parsed_action = action_map() + // // .easy_parse(input) + // // .map(|t| t.0)?; + // // + // // }, + // // _ => (), + // // }; + // + // // let yo = self.action; + // // let parsed_action = self.action.parse()?; + // let yo = self.action.clone(); + // let parsed_action = match yo { + // Action::String(s) => { + // let input = State::new(s.as_str()); + // + // action_map() + // .easy_parse(input) + // .map(|t| t.0) + // .map(|action| Some(action)) + // }, + // _ => Ok(None), + // }?; + // + // if let Some(action) = parsed_action { + // // self.action = action; + // mem::replace(&mut self.action, action); + // } + // + // // self.action = { + // // let parsed_action = self.action.parse()?; + // // + // // match parsed_action { + // // Some(a) => a, + // // None => self.action, + // // } + // // }; + // }, + // + // // Commands don't get parsed. They remain `Action::String`s. + // MapKind::Command => (), + // }; + // + // Ok(()) + // } } #[derive(Debug, PartialEq)] @@ -245,12 +245,12 @@ impl MapGroup { pub fn parse_actions(&mut self) { for map_action in self.maps.values_mut() { - map_action.parse(); + // map_action.parse(); } for mode in self.modes.values_mut() { for map_action in mode.values_mut() { - map_action.parse(); + // map_action.parse(); } } } @@ -330,6 +330,22 @@ where ) } +fn map_kind_map() -> impl Parser +where + I: Stream, + I::Error: ParseError, +{ + string("map").map(|_| MapKind::Map) +} + +fn map_kind_cmd() -> impl Parser +where + I: Stream, + I::Error: ParseError, +{ + string("cmd").map(|_| MapKind::Command) +} + fn headphone_button() -> impl Parser where I: Stream, @@ -393,7 +409,7 @@ where I::Error: ParseError, { or( - satisfy(|c| c != '<' && c != '\\'), + satisfy(|c| c != '<' && c != '\\' && c != '\n'), action_escape() ) } @@ -594,13 +610,33 @@ where skip_many1(space().or(tab())) } -fn map() -> impl Parser +fn map_map() -> impl Parser +where + I: Stream, + I::Error: ParseError, +{ + ( + map_kind_map(), + whitespace_separator(), + trigger(), + whitespace_separator(), + action_map() + ).map(|(kind, _, trigger, _, action)| + Map { + trigger: trigger, + action: action, + kind: kind, + } + ) +} + +fn map_cmd() -> impl Parser where I: Stream, I::Error: ParseError, { ( - map_kind(), + map_kind_cmd(), whitespace_separator(), trigger(), whitespace_separator(), @@ -614,6 +650,17 @@ where ) } +fn map() -> impl Parser +where + I: Stream, + I::Error: ParseError, +{ + or( + map_map(), + map_cmd(), + ) +} + fn maps() -> impl Parser where I: Stream, @@ -1143,7 +1190,24 @@ mod tests { "; let expected = Map { trigger: vec![HeadphoneButton::Play, HeadphoneButton::Down], - action: Action::String("test".to_owned()), + action: Action::Map(vec![ + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('t')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('e')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('s')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('t')), + vec![], + ), + ]), kind: MapKind::Map, }; let result = map().parse(text).map(|t| t.0); @@ -1160,7 +1224,24 @@ cmd echo test expected.insert( vec![HeadphoneButton::Play, HeadphoneButton::Down], MapAction { - action: Action::String("test".to_owned()), + action: Action::Map(vec![ + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('t')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('e')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('s')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('t')), + vec![], + ), + ]), kind: MapKind::Map, } ); @@ -1177,11 +1258,12 @@ cmd echo test } #[test] - fn map_collection_fails_without_terminating_newline() { + fn map_collection_fails_without_terminating_newline_after_cmd() { let text = "map works -map fails"; +cmd fails"; let result = map_collection().easy_parse(State::new(text)).map(|t| t.0); + // TODO: Why is this here? let mut expected = HashMap::new(); expected.insert( vec![HeadphoneButton::Play], @@ -1223,14 +1305,92 @@ cmd /usr/bin/say 'hello' expected.insert( vec![HeadphoneButton::Up, HeadphoneButton::Down], MapAction { - action: Action::String("test".to_owned()), + action: Action::Map(vec![ + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('t')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('e')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('s')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('t')), + vec![], + ), + ]), kind: MapKind::Map, }, ); expected.insert( vec![HeadphoneButton::Play], MapAction { - action: Action::String("salt and pepper".to_owned()), + action: Action::Map(vec![ + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('s')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('a')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('l')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('t')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new(' ')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('a')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('n')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('d')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new(' ')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('p')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('e')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('p')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('p')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('e')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('r')), + vec![], + ), + ]), kind: MapKind::Map, }, ); @@ -1268,7 +1428,44 @@ cmd /usr/bin/say 'hello' expected.maps.insert( vec![HeadphoneButton::Down], MapAction { - action: Action::String("insert {}".to_owned()), + action: Action::Map(vec![ + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('i')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('n')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('s')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('e')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('r')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('t')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new(' ')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('{')), + vec![], + ), + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('}')), + vec![], + ), + ]), kind: MapKind::Map, }, ); @@ -1317,7 +1514,12 @@ map k }), Definition::Map(Map { trigger: vec![HeadphoneButton::Play], - action: Action::String("m".to_owned()), + action: Action::Map(vec![ + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('m')), + vec![], + ), + ]), kind: MapKind::Map, }), Definition::Mode(Mode { @@ -1326,7 +1528,12 @@ map k }), Definition::Map(Map { trigger: vec![HeadphoneButton::Down], - action: Action::String("k".to_owned()), + action: Action::Map(vec![ + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('k')), + vec![], + ), + ]), kind: MapKind::Map, }), ]; @@ -1383,7 +1590,12 @@ cmd /usr/bin/say hello mode_maps.insert( vec![HeadphoneButton::Play], MapAction { - action: Action::String("p".to_owned()), + action: Action::Map(vec![ + KeyboardKeyWithModifiers::new( + KeyboardKey::Character(Character::new('p')), + vec![], + ), + ]), kind: MapKind::Map, }, ); -- cgit v1.2.3