Skip to content

Commit

Permalink
fix(macro): improve error message clarity
Browse files Browse the repository at this point in the history
  • Loading branch information
jtroo committed Jun 15, 2024
1 parent 9b6ad0e commit a2098f5
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 6 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion parser/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@ kanata-keyberon = { path = "../keyberon" }
bytemuck = "1.15.0"
bitflags = "2.5.0"

[dev-dependencies]
simplelog = "0.12.0"

[features]
cmd = []
interception_driver = []
gui = []
lsp = []
lsp = []
18 changes: 13 additions & 5 deletions parser/src/cfg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2083,21 +2083,29 @@ fn parse_macro_item_impl<'a>(
Ok((events, &acs[1..]))
}
Ok(Action::Custom(custom)) => Ok((vec![SequenceEvent::Custom(custom)], &acs[1..])),
_ => {
Ok(_) => bail_expr!(&acs[0], "{MACRO_ERR}"),
Err(e) => {
if let Some(submacro) = acs[0].list(s.vars()) {
// If it's just a list that's not parsable as a usable action, try parsing the
// content.
let mut submacro_remainder = submacro;
let mut all_events = vec![];
while !submacro_remainder.is_empty() {
let mut events;
(events, submacro_remainder) = parse_macro_item(submacro_remainder, s)?;
(events, submacro_remainder) =
parse_macro_item(submacro_remainder, s).map_err(|_e| e.clone())?;
all_events.append(&mut events);
}
return Ok((all_events, &acs[1..]));
}

let (held_mods, unparsed_str) = parse_mods_held_for_submacro(&acs[0], s)?;
let (held_mods, unparsed_str) =
parse_mods_held_for_submacro(&acs[0], s).map_err(|mut err| {
if err.msg == MACRO_ERR {
err.msg = format!("{}\n{MACRO_ERR}", &e.msg);
}
err
})?;
let mut all_events = vec![];

// First, press all of the modifiers
Expand All @@ -2113,12 +2121,12 @@ fn parse_macro_item_impl<'a>(
// Ensure that the unparsed text is empty since otherwise it means there is
// invalid text there
if !unparsed_str.is_empty() {
bail_expr!(&acs[0], "{MACRO_ERR}")
bail_expr!(&acs[0], "{}\n{MACRO_ERR}", &e.msg)
}
// Check for a follow-up list
rem_start = 2;
if acs.len() < 2 {
bail_expr!(&acs[0], "{MACRO_ERR}")
bail_expr!(&acs[0], "{}\n{MACRO_ERR}", &e.msg)
}
acs[1]
.list(s.vars())
Expand Down
24 changes: 24 additions & 0 deletions parser/src/cfg/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,32 @@ use std::sync::{Mutex, MutexGuard};

mod ambiguous;
mod environment;
mod macros;

static CFG_PARSE_LOCK: Mutex<()> = Mutex::new(());

fn init_log() {
use simplelog::*;
use std::sync::OnceLock;
static LOG_INIT: OnceLock<()> = OnceLock::new();
LOG_INIT.get_or_init(|| {
let mut log_cfg = ConfigBuilder::new();
if let Err(e) = log_cfg.set_time_offset_to_local() {
eprintln!("WARNING: could not set log TZ to local: {e:?}");
};
log_cfg.set_time_format_rfc3339();
CombinedLogger::init(vec![TermLogger::new(
// Note: set to a different level to see logs in tests.
// Also, not all tests call init_log so you might have to add the call there too.
LevelFilter::Error,
log_cfg.build(),
TerminalMode::Stderr,
ColorChoice::AlwaysAnsi,
)])
.expect("logger can init");
});
}

fn lock<T>(lk: &Mutex<T>) -> MutexGuard<T> {
match lk.lock() {
Ok(guard) => guard,
Expand All @@ -18,6 +41,7 @@ fn lock<T>(lk: &Mutex<T>) -> MutexGuard<T> {
}

fn parse_cfg(cfg: &str) -> Result<IntermediateCfg> {
init_log();
let _lk = lock(&CFG_PARSE_LOCK);
let mut s = ParserState::default();
parse_cfg_raw_string(
Expand Down
30 changes: 30 additions & 0 deletions parser/src/cfg/tests/macros.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use super::*;

#[test]
fn unsupported_action_in_macro_triggers_error() {
let source = r#"
(defsrc)
(deflayer base)
(defalias a (macro (multi a b c))) "#;
parse_cfg(source)
.map(|_| ())
.map_err(|e| log::info!("{:?}", miette::Error::from(e)))
.expect_err("errors");
}

#[test]
fn incorrectly_configured_supported_action_in_macro_triggers_useful_error() {
let source = r#"
(defsrc)
(deflayer base)
(defalias a (macro (on-press press-vkey does-not-exist))) "#;
parse_cfg(source)
.map(|_| ())
.map_err(|e| {
let e = miette::Error::from(e);
let msg = format!("{:?}", e);
log::info!("{msg}");
assert!(msg.contains("unknown virtual key name: does-not-exist"));
})
.expect_err("errors");
}

0 comments on commit a2098f5

Please sign in to comment.