add error_and_panic helper for logerr

Cargo.lock

@@ -257,6 +257,15 @@ version = "0.6.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "702fc72eb24e5a1e48ce58027a675bc24edd52096d5397d4aea7c6dd9eca0bd1"
 
+[[package]]
+name = "clipboard-win"
+version = "5.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "12f9a0700e0127ba15d1d52dd742097f821cd9c65939303a44d970465040a297"
+dependencies = [
+ "error-code",
+]
+
 [[package]]
 name = "cnx"
 version = "0.3.0"
@@ -309,6 +318,12 @@ version = "1.10.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "11157ac094ffbdde99aa67b23417ebdd801842852b500e395a45a9c0aac03e4a"
 
+[[package]]
+name = "endian-type"
+version = "0.1.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c34f04666d835ff5d62e058c3995147c06f42fe86ff053337632bca83e42702d"
+
 [[package]]
 name = "env_logger"
 version = "0.10.2"
@@ -338,6 +353,23 @@ dependencies = [
  "windows-sys 0.52.0",
 ]
 
+[[package]]
+name = "error-code"
+version = "3.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a0474425d51df81997e2f90a21591180b38eccf27292d755f3e30750225c175b"
+
+[[package]]
+name = "fd-lock"
+version = "4.0.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7e5768da2206272c81ef0b5e951a41862938a6070da63bcea197899942d3b947"
+dependencies = [
+ "cfg-if",
+ "rustix",
+ "windows-sys 0.52.0",
+]
+
 [[package]]
 name = "futures"
 version = "0.3.30"
@@ -553,6 +585,7 @@ dependencies = [
  "paste",
  "pretty_assertions",
  "pretty_env_logger",
+ "rustyline",
  "serde",
  "serde_json",
  "strum",
@@ -685,6 +718,26 @@ dependencies = [
  "windows-sys 0.48.0",
 ]
 
+[[package]]
+name = "nibble_vec"
+version = "0.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "77a5d83df9f36fe23f0c3648c6bbb8b0298bb5f1939c8f2704431371f4b84d43"
+dependencies = [
+ "smallvec",
+]
+
+[[package]]
+name = "nix"
+version = "0.27.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2eb04e9c688eff1c89d72b407f168cf79bb9e867a9d3323ed6c01519eb9cc053"
+dependencies = [
+ "bitflags 2.4.2",
+ "cfg-if",
+ "libc",
+]
+
 [[package]]
 name = "num-traits"
 version = "0.2.18"
@@ -877,6 +930,16 @@ dependencies = [
  "proc-macro2",
 ]
 
+[[package]]
+name = "radix_trie"
+version = "0.2.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c069c179fcdc6a2fe24d8d18305cf085fdbd4f922c041943e203685d6a1c58fd"
+dependencies = [
+ "endian-type",
+ "nibble_vec",
+]
+
 [[package]]
 name = "regex"
 version = "1.10.3"
@@ -931,6 +994,28 @@ version = "1.0.14"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "7ffc183a10b4478d04cbbbfc96d0873219d962dd5accaff2ffbd4ceb7df837f4"
 
+[[package]]
+name = "rustyline"
+version = "13.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "02a2d683a4ac90aeef5b1013933f6d977bd37d51ff3f4dad829d4931a7e6be86"
+dependencies = [
+ "bitflags 2.4.2",
+ "cfg-if",
+ "clipboard-win",
+ "fd-lock",
+ "home",
+ "libc",
+ "log",
+ "memchr",
+ "nix",
+ "radix_trie",
+ "unicode-segmentation",
+ "unicode-width",
+ "utf8parse",
+ "winapi",
+]
+
 [[package]]
 name = "ryu"
 version = "1.0.17"
@@ -1010,18 +1095,18 @@ checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623"
 
 [[package]]
 name = "strum"
-version = "0.25.0"
+version = "0.26.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "290d54ea6f91c969195bdbcd7442c8c2a2ba87da8bf60a7ee86a235d4bc1e125"
+checksum = "723b93e8addf9aa965ebe2d11da6d7540fa2283fcea14b3371ff055f7ba13f5f"
 dependencies = [
  "strum_macros",
 ]
 
 [[package]]
 name = "strum_macros"
-version = "0.25.3"
+version = "0.26.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "23dc1fa9ac9c169a78ba62f0b841814b7abae11bdd047b9c58f893439e309ea0"
+checksum = "7a3417fc93d76740d974a01654a09777cb500428cc874ca9f45edfe0c4d4cd18"
 dependencies = [
  "heck",
  "proc-macro2",
@@ -1189,6 +1274,18 @@ version = "1.0.12"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b"
 
+[[package]]
+name = "unicode-segmentation"
+version = "1.11.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d4c87d22b6e3f4a18d4d40ef354e97c90fcb14dd91d7dc0aa9d8a1172ebf7202"
+
+[[package]]
+name = "unicode-width"
+version = "0.1.11"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e51733f11c9c4f72aa0c160008246859e340b00807569a0da0e7a1079b27ba85"
+
 [[package]]
 name = "utf8parse"
 version = "0.2.1"

Cargo.toml

@@ -11,13 +11,14 @@ clap = { version = "4.4.18", features = ["derive", "cargo"] }
 paste = "1.0.14"
 serde = { version = "1.0.195", features = ["derive"] }
 serde_json = { version = "1.0.113" }
-strum = { version = "0.25.0", features = ["derive"] }
+strum = { version = "0.26", features = ["derive"] }
 thiserror = "1.0.57"
 toml = "0.8.8"
 which = "6.0.0"
 log = "0.4"
 pretty_env_logger = "0.5"
 cnx = { git = "https://github.com/mjkillough/cnx.git", rev = "7845d99baa296901171c083db61588a62f9a8b34" }
+rustyline = "13.0.0"
 
 [dev-dependencies]
 pretty_assertions = "1.4.0"

rust-toolchain.toml

@@ -0,0 +1,2 @@
+[toolchain]
+channel = "stable"

src/add/mod.rs

@@ -0,0 +1,81 @@
+use clap::Subcommand;
+use rustyline::{error::ReadlineError, DefaultEditor};
+use thiserror::Error;
+
+use crate::hlwm::{
+    command::{CommandParseError, HlwmCommand},
+    key::Key,
+    parser::{FromCommandArgs, ParseError},
+};
+
+#[derive(Subcommand, Debug, Clone)]
+pub enum Add {
+    /// Add a new keybind to the existing hlctl config
+    #[clap(arg_required_else_help = true)]
+    Keybind {
+        #[arg(short, long)]
+        interactive: bool,
+    },
+}
+
+#[derive(Debug, Clone, Error)]
+pub enum Error {
+    #[error("readline error: [{0}]")]
+    ReadlineError(String),
+    #[error("parse error: [{0}]")]
+    KeyParseError(#[from] ParseError),
+    #[error("could not parse command: [{0}]")]
+    CommandParseError(#[from] CommandParseError),
+}
+
+impl From<ReadlineError> for Error {
+    fn from(value: ReadlineError) -> Self {
+        Self::ReadlineError(value.to_string())
+    }
+}
+
+impl Add {
+    pub fn run(self) -> Result<(), Error> {
+        match self {
+            Add::Keybind { interactive } => {
+                if interactive {
+                    keybind_interactive()
+                } else {
+                    todo!()
+                }
+            }
+        }
+    }
+}
+
+fn keybind_interactive() -> Result<(), Error> {
+    let mut read = DefaultEditor::new()?;
+    let keys = read
+        .readline("Keys (Separated by spaces, tabs, +, or -): ")?
+        .trim()
+        .split([' ', '\t', '+', '-'])
+        .map(|l| l.trim())
+        .filter(|l| !l.is_empty())
+        .map(|k| Key::from_str_case_insensitive(k))
+        .collect::<Result<Vec<_>, _>>()?;
+
+    let command = {
+        let read = read.readline("Command (Arguments separated by tabs or spaces): ")?;
+        let mut args = read
+            .trim()
+            .split([' ', '\t'])
+            .map(|a| a.trim())
+            .filter(|a| !a.is_empty())
+            .map(|a| a.to_string());
+        match args.next() {
+            Some(cmd) => HlwmCommand::from_command_args(&cmd, args)?,
+            None => {
+                println!("No command provided");
+                std::process::exit(1);
+            }
+        }
+    };
+
+    println!("{keys:?} -> {command:?}");
+    Ok(())
+}

src/config.rs

@@ -17,22 +17,23 @@ use strum::IntoEnumIterator;
 use thiserror::Error;
 
 use crate::{
-    environ::{self, ActiveKeybinds, EnvironError},
+    environ::{self, ActiveKeybinds},
     hlwm::{
         self,
-        attribute::Attribute,
+        attribute::{Attribute, AttributeType},
         color::Color,
         command::{CommandError, HlwmCommand},
         hook::Hook,
         key::{Key, KeyUnbind, Keybind, MouseButton, Mousebind, MousebindAction},
+        parser::{ArgParser, FromCommandArgs, FromStrings, ParseError},
         rule::{Condition, Consequence, FloatPlacement, Rule, Unrule},
         setting::{FrameLayout, Setting, ShowFrameDecoration, SmartFrameSurroundings},
         theme::ThemeAttr,
         window::Window,
-        Align, Client, Direction, Index, Operator, Separator, StringParseError, ToggleBool,
+        Align, Client, Direction, Index, Operator, Separator, ToggleBool,
     },
     logerr::UnwrapLog,
-    rule, window_types,
+    rule, split, window_types,
 };
 
 #[derive(Debug, Error)]
@@ -51,14 +52,13 @@ pub enum ConfigError {
     CommandError(#[from] CommandError),
     #[error("non-utf8 string error: {0}")]
     Utf8StringError(#[from] FromUtf8Error),
-    #[error("failed parsing value from string: {0}")]
+    #[error("failed parsing value: {0}")]
-    StringParseError(#[from] StringParseError),
+    ParseError(#[from] ParseError),
-    #[error("getting from environment error: [{0}]")]
-    EnvironError(#[from] EnvironError),
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct Config {
+    pub use_panel: bool,
     pub font: String,
     pub font_bold: String,
     /// Pango font for use where pango fonts are
@@ -98,35 +98,36 @@ impl Display for SetAttribute {
     }
 }
 
-impl FromStr for SetAttribute {
+impl FromStrings for SetAttribute {
-    type Err = StringParseError;
+    fn from_strings<I: Iterator<Item = String>>(s: I) -> Result<Self, ParseError> {
-
+        let s = s.collect::<Vec<_>>();
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let original_str = s.join(" ");
-        let mut parts = s.split(')');
+        let mut parser = ArgParser::from_strings(s.into_iter());
-        let type_string = parts.next().map(|p| p.strip_prefix('(')).flatten().ok_or(
+        let x = parser.must_string("set_attribute(type/name)")?;
-            StringParseError::RequiredArgMissing("attribute type".into()),
+        let (attr_ty, remainder) = split::parens(&x).ok_or(ParseError::InvalidValue {
-        )?;
+            value: original_str,
-        let mut parts = parts
+            expected: "does not have a valid type string (type)",
-            .next()
+        })?;
-            .ok_or(StringParseError::RequiredArgMissing(
+        if remainder.contains('=') {
-                "attribute path/value".into(),
+            let mut parser = ArgParser::from_strings(remainder.split('=').map(|c| c.to_string()));
-            ))?
+            return Ok(Self {
-            .split('=')
+                path: parser.must_string("set_attribute(path)")?,
-            .map(|v| v.trim());
+                value: Attribute::new(
-        let path = parts
+                    AttributeType::from_str(&attr_ty)?,
-            .next()
+                    &parser.must_string("set_attribute(value)")?,
-            .ok_or(StringParseError::RequiredArgMissing(
+                )?,
-                "attribute path".into(),
+            });
-            ))?
-            .to_string();
-        let value_string = parts.collect::<Vec<_>>().join("=");
-        if value_string.is_empty() {
-            return Err(StringParseError::RequiredArgMissing(
-                "attribute value".into(),
-            ));
         }
-        let value = Attribute::new(type_string, &value_string)?;
+        if parser.must_string("set_attribute(equals)")? != "=" {
-        Ok(Self { path, value })
+            return Err(ParseError::ValueMissing);
+        }
+        Ok(Self {
+            path: remainder,
+            value: Attribute::new(
+                AttributeType::from_str(&attr_ty)?,
+                &parser.must_string("set_attribute(value)")?,
+            )?,
+        })
     }
 }
 
@@ -151,13 +152,16 @@ impl<'de> Deserialize<'de> for SetAttribute {
             }
         }
         let str_val: String = Deserialize::deserialize(deserializer)?;
-        Ok(Self::from_str(&str_val).map_err(|_| {
+        Ok(
-            serde::de::Error::invalid_value(serde::de::Unexpected::Str(&str_val), &ExpectedKey)
+            Self::from_strings(split::tab_or_space(&str_val).into_iter()).map_err(|_| {
-        })?)
+                serde::de::Error::invalid_value(serde::de::Unexpected::Str(&str_val), &ExpectedKey)
+            })?,
+        )
     }
 }
 
 impl Config {
+    const USE_PANEL: &'static str = "settings.my_hlctl_panel_enabled";
     const FONT: &'static str = "theme.my_font";
     const FONT_BOLD: &'static str = "theme.my_font_bold";
     const FONT_PANGO: &'static str = "theme.my_font_pango";
@@ -216,6 +220,7 @@ impl Config {
     fn attrs_set(&self) -> Result<Vec<HlwmCommand>, ConfigError> {
         info!("loading attr settings command set");
         Ok([
+            (Self::USE_PANEL, Some(self.use_panel.to_string())),
             (Self::FONT, Some(self.font.clone())),
             (Self::FONT_BOLD, Some(self.font_bold.clone())),
             (Self::FONT_PANGO, Some(self.font_pango.clone())),
@@ -291,8 +296,7 @@ impl Config {
                     HlwmCommand::Spawn {
                         executable: s.name,
                         args: s.arguments,
-                    }
+                    },
-                    .to_try(),
                 ]
             })
             .flatten()
@@ -389,6 +393,9 @@ impl Config {
         let default = Config::default();
         let mod_key = setting(Self::MOD_KEY, Key::from_str, default.mod_key);
         Config {
+            use_panel: client
+                .get_from_str_attr(Self::USE_PANEL.to_string())
+                .unwrap_or_log(default.use_panel),
             font: client
                 .get_str_attr(Self::FONT.to_string())
                 .unwrap_or_log(default.font),
@@ -416,12 +423,13 @@ impl Config {
                                 .into_iter()
                                 .find(|f| (*f).eq(&attr))
                                 .unwrap();
-                            ThemeAttr::from_raw_parts(
+                            ThemeAttr::from_command_args(
                                 &attr_path,
-                                &client
+                                [client
                                     .get_attr(attr_path.clone())
                                     .map(|t| t.to_string())
-                                    .unwrap_or_log(default.to_string()),
+                                    .unwrap_or_log(default.to_string())]
+                                .into_iter(),
                             )
                             .unwrap_or_log(default.clone())
                         })
@@ -432,14 +440,13 @@ impl Config {
                 .unwrap_or_log(default.keybinds),
             tags: Self::active_tags(mod_key).unwrap_or_log(default.tags),
             rules: (|| -> Result<Vec<Rule>, ConfigError> {
-                Ok(
+                Ok(HlwmCommand::ListRules
-                    String::from_utf8(client.execute(HlwmCommand::ListRules)?.stdout)?
+                    .execute_str()?
-                        .split('\n')
+                    .split('\n')
-                        .map(|l| l.trim())
+                    .map(|l| l.trim())
-                        .filter(|l| !l.is_empty())
+                    .filter(|l| !l.is_empty())
-                        .map(|line| Rule::from_str(line))
+                    .map(|line| Rule::from_strings(split::tab_or_space(line).into_iter()))
-                        .collect::<Result<_, _>>()?,
+                    .collect::<Result<_, _>>()?)
-                )
             })()
             .unwrap_or_log(default.rules),
             settings: (|| -> Result<Vec<_>, CommandError> {
@@ -582,6 +589,7 @@ impl Default for Config {
         let text_color = Color::from_hex("#898989").expect("default text color");
 
         Self {
+            use_panel: true,
             mod_key,
             font: String::from("-*-fixed-medium-*-*-*-12-*-*-*-*-*-*-*"),
             font_bold: font_bold.clone(),
@@ -719,16 +727,10 @@ impl Default for Config {
                     HlwmCommand::JumpTo(Window::Urgent),
                 ),
             ],
-            services: vec![
+            services: vec![Service {
-                Service {
+                name: String::from("fcitx5"),
-                    name: String::from("fcitx5"),
+                arguments: vec![],
-                    arguments: vec![],
+            }],
-                },
-                Service {
-                    name: String::from("hlctl"),
-                    arguments: vec![String::from("panel")],
-                },
-            ],
             tags: [
                 Tag::standard(".1", Key::Char('1')),
                 Tag::standard(".2", Key::Char('2')),
@@ -960,6 +962,9 @@ mod test {
             value: Attribute::Color(Color::X11(X11Color::NavajoWhite3)),
         });
         let cfg_serialized = cfg.serialize().unwrap();
+        println!("--> Config <--");
+        println!("{cfg_serialized}");
+        println!("--> Config <--");
         let parsed_cfg = Config::deserialize(&cfg_serialized).unwrap();
 
         assert_eq!(cfg, parsed_cfg,)

src/environ.rs

@@ -1,43 +1,27 @@
-use std::{str::FromStr, string::FromUtf8Error};
-
 use log::debug;
-use thiserror::Error;
 
 use crate::{
     hlwm::{
         command::{CommandError, HlwmCommand},
-        key::{KeyParseError, Keybind},
+        key::Keybind,
-        Client,
+        parser::FromStrings,
     },
     split,
 };
 
-#[derive(Clone, Debug, Error)]
-pub enum EnvironError {
-    #[error("keybind parsing error: [{0}]")]
-    KeyParseError(#[from] KeyParseError),
-    #[error("command execution error: [{0}]")]
-    CommandError(#[from] CommandError),
-}
-
-impl From<FromUtf8Error> for EnvironError {
-    fn from(value: FromUtf8Error) -> Self {
-        CommandError::UtfError(value).into()
-    }
-}
-
 pub enum ActiveKeybinds {
     All,
     OmitNamedTagBinds,
     OnlyNamedTagBinds,
 }
 
-pub fn active_keybinds(ty: ActiveKeybinds) -> Result<Vec<Keybind>, EnvironError> {
+pub fn active_keybinds(ty: ActiveKeybinds) -> Result<Vec<Keybind>, CommandError> {
     let (use_tag, move_tag) = (
         HlwmCommand::UseTag(String::new()).to_string(),
         HlwmCommand::MoveTag(String::new()).to_string(),
     );
-    String::from_utf8(Client::new().execute(HlwmCommand::ListKeybinds)?.stdout)?
+    HlwmCommand::ListKeybinds
+        .execute_str()?
         .split("\n")
         .map(|l| l.trim())
         .filter(|l| !l.is_empty())
@@ -58,10 +42,12 @@ pub fn active_keybinds(ty: ActiveKeybinds) -> Result<Vec<Keybind>, EnvironError>
             }
         })
         .map(|row: &str| {
-            Ok(Keybind::from_str(row).map_err(|err| {
+            Ok(
-                debug!("row: [{row}], error: [{err}]");
+                Keybind::from_strings(split::tab_or_space(row).into_iter()).map_err(|err| {
-                err
+                    debug!("row: [{row}], error: [{err}]");
-            })?)
+                    err
+                })?,
+            )
         })
-        .collect::<Result<Vec<_>, EnvironError>>()
+        .collect::<Result<Vec<_>, CommandError>>()
 }

src/hlwm/and_or_command.rs

@@ -0,0 +1,117 @@
+use crate::hlwm::parser::ParseError;
+
+use super::{
+    command::HlwmCommand,
+    parser::{ArgParser, FromCommandArgs, FromStringsHint},
+    Separator,
+};
+
+pub struct AndOrCommands(Vec<HlwmCommand>);
+
+impl AndOrCommands {
+    pub fn commands(self) -> Vec<HlwmCommand> {
+        self.0
+    }
+}
+
+impl FromStringsHint<Separator> for AndOrCommands {
+    fn from_strings_hint<I: Iterator<Item = String>>(
+        s: I,
+        hint: Separator,
+    ) -> Result<Self, ParseError> {
+        Ok(Self(
+            split_by_separator(s, hint)
+                .into_iter()
+                .map(|cmd_args| -> Result<HlwmCommand, ParseError> {
+                    let mut parser = ArgParser::from_strings(cmd_args.into_iter());
+                    HlwmCommand::from_command_args(
+                        &parser.must_string("hlwmcommand(command)")?,
+                        parser.collect::<Vec<String>>().into_iter(),
+                    )
+                })
+                .collect::<Result<Vec<HlwmCommand>, ParseError>>()?,
+        ))
+    }
+}
+
+fn split_by_separator<I: Iterator<Item = String>>(s: I, separator: Separator) -> Vec<Vec<String>> {
+    let separator = separator.to_string();
+    let mut commands = Vec::new();
+    let mut working = Vec::new();
+    for itm in s {
+        if itm.len() == 1 && itm == separator {
+            if !working.is_empty() {
+                commands.push(working);
+                working = Vec::new();
+            }
+            continue;
+        }
+        working.push(itm);
+    }
+    if !working.is_empty() {
+        commands.push(working);
+    }
+    commands
+}
+
+#[cfg(test)]
+mod test {
+    use crate::hlwm::Separator;
+
+    use super::split_by_separator;
+    use pretty_assertions::assert_eq;
+
+    #[test]
+    fn test_split_by_separator() {
+        let cases = [(
+            [
+                "or",
+                ",",
+                "and",
+                ".",
+                "compare",
+                "tags.focus.curframe_wcount",
+                "=",
+                "2",
+                ".",
+                "cycle_layout",
+                "+1",
+                "vertical",
+                "horizontal",
+                "max",
+                "vertical",
+                "grid",
+                ",",
+                "cycle_layout",
+                "+1",
+            ]
+            .into_iter(),
+            Separator::Comma,
+            vec![
+                vec!["or"],
+                vec![
+                    "and",
+                    ".",
+                    "compare",
+                    "tags.focus.curframe_wcount",
+                    "=",
+                    "2",
+                    ".",
+                    "cycle_layout",
+                    "+1",
+                    "vertical",
+                    "horizontal",
+                    "max",
+                    "vertical",
+                    "grid",
+                ],
+                vec!["cycle_layout", "+1"],
+            ],
+        )];
+        for (input, separator, expected) in cases {
+            let input = input.map(|i| i.to_string());
+            let actual = split_by_separator(input, separator);
+            assert_eq!(expected, actual);
+        }
+    }
+}

src/hlwm/attribute.rs

@@ -1,26 +1,23 @@
-use std::{num::ParseIntError, str::FromStr};
+use std::str::FromStr;
 
 use serde::{Deserialize, Serialize};
-use strum::IntoEnumIterator;
 use thiserror::Error;
 
-use crate::hlwm::hex::ParseHex;
+use crate::hlwm::{command::HlwmCommand, hex::ParseHex};
 
 use super::{
-    color::{self, Color, X11Color},
+    color::{Color, X11Color},
+    command::CommandError,
     hex::HexError,
+    hlwmbool,
     octal::{OctalError, ParseOctal},
-    StringParseError,
+    parser::{FromStringsHint, ParseError, ToOption},
 };
 
 #[derive(Debug, Clone, Error)]
 pub enum AttributeError {
-    #[error("error parsing integer value: {0:?}")]
+    #[error("error parsing value: {0}")]
-    ParseIntError(#[from] ParseIntError),
+    ParseError(#[from] ParseError),
-    #[error("error parsing bool value: [{0}]")]
-    ParseBoolError(String),
-    #[error("error parsing color value: {0}")]
-    ParseColorError(#[from] color::ParseError),
     #[error("unknown attribute type [{0}]")]
     UnknownType(String),
     #[error("not a valid rectangle: [{0}]")]
@@ -31,6 +28,15 @@ pub enum AttributeError {
     OctalError(#[from] OctalError),
 }
 
+impl From<AttributeError> for ParseError {
+    fn from(value: AttributeError) -> Self {
+        match value {
+            AttributeError::UnknownType(t) => ParseError::InvalidCommand(t),
+            _ => ParseError::PrimitiveError(value.to_string()),
+        }
+    }
+}
+
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
 pub enum AttributeOption {
     Bool(Option<bool>),
@@ -42,6 +48,13 @@ pub enum AttributeOption {
     WindowID(Option<u32>),
 }
 
+impl FromStringsHint<&str> for AttributeOption {
+    fn from_strings_hint<I: Iterator<Item = String>>(s: I, hint: &str) -> Result<Self, ParseError> {
+        let s = s.collect::<Vec<_>>().to_option().map(|t| t.join(" "));
+        Ok(Self::new(AttributeType::from_str(hint)?, s.as_ref())?)
+    }
+}
+
 impl Default for AttributeOption {
     fn default() -> Self {
         Self::Int(None)
@@ -49,19 +62,21 @@ impl Default for AttributeOption {
 }
 
 impl AttributeOption {
-    pub fn new(type_string: &str, value_string: &Option<String>) -> Result<Self, AttributeError> {
+    pub fn new(
+        attr_type: AttributeType,
+        value_string: Option<&String>,
+    ) -> Result<Self, AttributeError> {
         if let Some(val) = value_string {
-            return Ok(Attribute::new(type_string, val)?.into());
+            return Ok(Attribute::new(attr_type, val)?.into());
         }
-        match type_string {
+        match attr_type {
-            "int" => Ok(Self::Int(None)),
+            AttributeType::Int => Ok(Self::Int(None)),
-            "bool" => Ok(Self::Bool(None)),
+            AttributeType::Bool => Ok(Self::Bool(None)),
-            "uint" => Ok(Self::Uint(None)),
+            AttributeType::Uint => Ok(Self::Uint(None)),
-            "color" => Ok(Self::Color(None)),
+            AttributeType::Color => Ok(Self::Color(None)),
-            "windowid" => Ok(Self::WindowID(None)),
+            AttributeType::WindowID => Ok(Self::WindowID(None)),
-            "rectangle" => Ok(Self::Rectangle(None)),
+            AttributeType::Rectangle => Ok(Self::Rectangle(None)),
-            "string" | "names" | "regex" | "font" => Ok(Self::String(None)),
+            AttributeType::String => Ok(Self::String(None)),
-            _ => Err(AttributeError::UnknownType(type_string.to_string())),
         }
     }
 
@@ -169,75 +184,57 @@ impl From<(u32, u32)> for Attribute {
     }
 }
 
+impl FromStringsHint<&str> for Attribute {
+    fn from_strings_hint<I: Iterator<Item = String>>(s: I, hint: &str) -> Result<Self, ParseError> {
+        let value = s.collect::<Vec<_>>().join(" ");
+        Ok(Self::new(
+            AttributeType::get_type_or_guess(hint, &value),
+            &value,
+        )?)
+    }
+}
+
 impl Attribute {
-    pub fn new(type_string: &str, value_string: &str) -> Result<Self, AttributeError> {
+    pub fn new(attr_type: AttributeType, value_string: &str) -> Result<Self, AttributeError> {
-        match type_string {
+        match attr_type {
-            "bool" => match value_string {
+            AttributeType::Bool => Ok(Self::Bool(hlwmbool::from_hlwm_string(value_string)?)),
-                "on" | "true" => Ok(Attribute::Bool(true)),
+            AttributeType::Color => Ok(Attribute::Color(Color::from_str(value_string)?)),
-                "off" | "false" => Ok(Attribute::Bool(false)),
+            AttributeType::Int => Ok(Attribute::Int(
-                _ => Err(AttributeError::ParseBoolError(type_string.to_string())),
+                value_string.parse().map_err(|err| ParseError::from(err))?,
-            },
+            )),
-            "color" => Ok(Attribute::Color(Color::from_str(value_string)?)),
+            AttributeType::String => Ok(Attribute::String(value_string.to_string())),
-            "int" => Ok(Attribute::Int(value_string.parse()?)),
+            AttributeType::Uint => Ok(Attribute::Uint(
-            "string" | "names" | "regex" | "font" => {
+                value_string.parse().map_err(|err| ParseError::from(err))?,
-                Ok(Attribute::String(value_string.to_string()))
+            )),
-            }
+            AttributeType::Rectangle => {
-            "uint" => Ok(Attribute::Uint(value_string.parse()?)),
-            "rectangle" => {
                 let parts = value_string.split('x').collect::<Vec<_>>();
                 if parts.len() != 2 {
                     return Err(AttributeError::NotRectangle(value_string.to_string()));
                 }
                 Ok(Attribute::Rectangle {
-                    x: parts.get(0).unwrap().parse()?,
+                    x: parts
-                    y: parts.get(1).unwrap().parse()?,
+                        .get(0)
+                        .unwrap()
+                        .parse()
+                        .map_err(|err| ParseError::from(err))?,
+                    y: parts
+                        .get(1)
+                        .unwrap()
+                        .parse()
+                        .map_err(|err| ParseError::from(err))?,
                 })
             }
-            "windowid" => {
+            AttributeType::WindowID => {
                 if let Some(hex) = value_string.strip_prefix("0x") {
                     Ok(Attribute::WindowID(hex.parse_hex()?))
                 } else if let Some(octal) = value_string.strip_prefix("0") {
                     Ok(Attribute::WindowID(octal.parse_octal()?))
                 } else {
-                    Ok(Attribute::WindowID(value_string.parse()?))
+                    Ok(Attribute::WindowID(
+                        value_string.parse().map_err(|err| ParseError::from(err))?,
+                    ))
                 }
             }
-            _ => Err(AttributeError::UnknownType(type_string.to_string())),
-        }
-    }
-
-    fn guess_from_value(value: &str) -> Option<Attribute> {
-        match value {
-            "on" | "true" => Some(Self::Bool(true)),
-            "off" | "false" => Some(Self::Bool(false)),
-            _ => {
-                // Match for all colors first
-                if let Some(color) = X11Color::iter()
-                    .into_iter()
-                    .find(|c| c.to_string() == value)
-                {
-                    return Some(Attribute::Color(Color::X11(color)));
-                }
-
-                // Match for primitive types or color string
-                let mut chars = value.chars();
-                match chars.next().unwrap() {
-                    '+' => Self::guess_from_value(&chars.collect::<String>()),
-                    '0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' | '-' => {
-                        i32::from_str(value).ok().map(|i| Attribute::Int(i))
-                    }
-                    '#' => Color::from_hex(value).ok().map(|c| Attribute::Color(c)),
-                    _ => None,
-                }
-            }
-        }
-    }
-
-    pub fn guess_type(value: &str) -> Self {
-        if value.is_empty() {
-            Self::String(String::new())
-        } else {
-            Self::guess_from_value(value).unwrap_or(Attribute::String(value.to_string()))
         }
     }
 
@@ -281,11 +278,71 @@ impl Default for AttributeType {
 }
 
 impl FromStr for AttributeType {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        Self::iter()
+        match s {
-            .find(|t| t.to_string() == s)
+            "bool" => Ok(Self::Bool),
-            .ok_or(StringParseError::UnknownValue)
+            "color" => Ok(Self::Color),
+            "int" => Ok(Self::Int),
+            "string" | "names" | "regex" | "font" => Ok(Self::String),
+            "uint" => Ok(Self::Uint),
+            "rectangle" => Ok(Self::Rectangle),
+            "windowid" => Ok(Self::WindowID),
+            _ => Err(ParseError::InvalidCommand(s.to_string())),
+        }
+    }
+}
+
+impl AttributeType {
+    /// Gets the type for the given `path` from herbstluftwm
+    pub fn get_type(path: &str) -> Result<AttributeType, CommandError> {
+        Ok(Self::from_str(
+            &String::from_utf8(HlwmCommand::AttrType(path.to_string()).execute()?.stdout)?
+                .split('\n')
+                .map(|l| l.trim())
+                .filter(|l| !l.is_empty())
+                .next()
+                .ok_or(CommandError::Empty)?,
+        )?)
+    }
+
+    /// Tries to get the type for the given `path`, but defaults to [AttributeType::String]
+    /// if that fails in any way
+    pub fn get_type_or_string(path: &str) -> AttributeType {
+        Self::get_type(path).unwrap_or(AttributeType::String)
+    }
+
+    /// Tries to get the type for the given `path`, and if that fails
+    /// tries to guess the type using [Self::guess]
+    pub fn get_type_or_guess(path: &str, value: &str) -> Self {
+        Self::get_type(path).unwrap_or(Self::guess(value))
+    }
+
+    pub fn guess(value: &str) -> Self {
+        if value.is_empty() {
+            return Self::String;
+        }
+        match value {
+            "on" | "true" | "off" | "false" => Self::Bool,
+            _ => {
+                // Match for all colors first
+                if X11Color::iter().into_iter().any(|c| c.to_string() == value) {
+                    return Self::Color;
+                }
+                // Is it a valid color string?
+                if Color::from_hex(value).is_ok() {
+                    return Self::Color;
+                }
+
+                // Match for primitive types or color string
+                let mut chars = value.chars();
+                match chars.next().unwrap() {
+                    '+' => Self::guess(&chars.collect::<String>()),
+                    '0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' | '-' => Self::Int,
+                    _ => Self::String,
+                }
+            }
+        }
     }
 }

src/hlwm/color/mod.rs

@@ -1,9 +1,8 @@
 use std::str::FromStr;
 
 use serde::{de::Unexpected, Deserialize, Serialize};
-use thiserror::Error;
 
-use super::hex::{HexError, ParseHex};
+use super::{hex::ParseHex, parser::ParseError};
 
 mod x11;
 pub use x11::X11Color;
@@ -51,14 +50,6 @@ impl Default for Color {
     }
 }
 
-#[derive(Debug, Clone, Error)]
-pub enum ParseError {
-    #[error("length must be either 6 characters or 7 with '#' prefix")]
-    InvalidLength,
-    #[error("invalid hex value: [{0}]")]
-    HexError(#[from] HexError),
-}
-
 impl FromStr for Color {
     type Err = ParseError;
 
@@ -76,7 +67,10 @@ impl Color {
     pub fn from_hex(hex: &str) -> Result<Self, ParseError> {
         let expected_len = if hex.starts_with('#') { 7 } else { 6 };
         if hex.len() != expected_len {
-            return Err(ParseError::InvalidLength);
+            return Err(ParseError::InvalidValue {
+                value: hex.to_string(),
+                expected: "a 6 digit hex value",
+            });
         }
         let hex = hex.strip_prefix('#').unwrap_or(hex);
 

src/hlwm/color/x11.rs

@@ -2,7 +2,7 @@ use std::str::FromStr;
 
 use serde::{Deserialize, Serialize};
 
-use crate::hlwm::StringParseError;
+use crate::hlwm::parser::ParseError;
 
 macro_rules! color_impl {
 	() => {};
@@ -50,13 +50,13 @@ macro_rules! color_impl {
 }
 
 impl FromStr for X11Color {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         X11Color::ALL
             .into_iter()
             .find(|c| c.color_names().into_iter().any(|n| n == s))
-            .ok_or(StringParseError::UnknownValue)
+            .ok_or(ParseError::InvalidCommand(s.to_string()))
     }
 }
 

src/hlwm/command.rs

@@ -1,22 +1,35 @@
-use std::{io, process::ExitStatus, str::FromStr, string::FromUtf8Error};
+use std::{
+    borrow::BorrowMut,
+    io,
+    process::{self, ExitStatus},
+    str::FromStr,
+    string::FromUtf8Error,
+};
 use strum::IntoEnumIterator;
 
 use serde::{de::Expected, Deserialize, Serialize};
 use thiserror::Error;
 
-use crate::{gen_parse, hlwm::Client, split};
+use crate::{
+    hlwm::{
+        parser::{either::Either, ParseError},
+        Client,
+    },
+    split,
+};
 
 use super::{
-    attribute::{Attribute, AttributeError, AttributeOption},
+    and_or_command::AndOrCommands,
+    attribute::{Attribute, AttributeError, AttributeOption, AttributeType},
     hlwmbool::ToggleBool,
     hook::Hook,
     key::{KeyUnbind, Keybind, Mousebind},
     pad::Pad,
+    parser::{self, ArgParser, Flip, FromCommandArgs, FromStrings, ToOption},
     rule::{Rule, Unrule},
     setting::{FrameLayout, Setting, SettingName},
     window::Window,
-    Align, Direction, Index, Monitor, Operator, Separator, StringParseError, TagSelect,
+    Align, Direction, Index, Monitor, Operator, Separator, TagSelect, ToCommandString,
-    ToCommandString,
 };
 
 #[derive(Debug, Clone, strum::Display, strum::EnumIter, PartialEq)]
@@ -196,6 +209,7 @@ pub enum HlwmCommand {
         filter_name: Option<String>,
         identifier: String,
         object: String,
+        command: Box<HlwmCommand>,
     },
     #[strum(serialize = "sprintf")]
     Sprintf(Vec<String>),
@@ -205,7 +219,7 @@ pub enum HlwmCommand {
 }
 
 impl FromStr for Box<HlwmCommand> {
-    type Err = CommandParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         Ok(Box::new(HlwmCommand::from_str(s)?))
@@ -227,42 +241,33 @@ impl<'de> Deserialize<'de> for HlwmCommand {
         D: serde::Deserializer<'de>,
     {
         pub enum Expect {
-            NotEmpty,
+            ParseError(ParseError),
         }
         impl Expected for Expect {
             fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
                 match self {
-                    Expect::NotEmpty => write!(f, "value not being empty"),
+                    Expect::ParseError(err) => f.write_str(&err.to_string()),
                 }
             }
         }
 
         let str_val: String = Deserialize::deserialize(deserializer)?;
-
+        ArgParser::from_strings(split::tab_or_space(&str_val).into_iter())
-        let parts = split::tab_or_space(&str_val);
+            .collect_command("hlwm_command")
-        if parts.is_empty() {
+            .map_err(|err| {
-            return Err(serde::de::Error::invalid_length(0, &Expect::NotEmpty));
+                serde::de::Error::invalid_value(
-        }
+                    serde::de::Unexpected::Str(&str_val),
-        let mut parts = parts.into_iter();
+                    &Expect::ParseError(err),
-        let command = parts.next().unwrap();
+                )
-        let args = parts.collect();
+            })
-
-        Ok(Self::from_raw_parts(&command, args).map_err(|err| {
-            serde::de::Error::invalid_value(serde::de::Unexpected::Str(&str_val), &err)
-        })?)
     }
 }
 
 impl FromStr for HlwmCommand {
-    type Err = CommandParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let mut parts = s.split("\t");
+        ArgParser::from_strings(split::tab_or_space(s).into_iter()).collect_command("hlwm_command")
-        let command = parts
-            .next()
-            .ok_or(CommandParseError::UnknownCommand(s.to_string()))?;
-        let args = parts.map(String::from).collect();
-        HlwmCommand::from_raw_parts(command, args)
     }
 }
 
@@ -272,14 +277,8 @@ impl Default for HlwmCommand {
     }
 }
 
-#[derive(Debug, Error)]
+#[derive(Debug, Clone, Error)]
 pub enum CommandParseError {
-    #[error("unknown command [{0}]")]
-    UnknownCommand(String),
-    #[error("bad argument for command [{command}]")]
-    BadArgument { command: String },
-    #[error("missing required argument")]
-    MissingArgument,
     #[error("invalid argument count [{0}] at [{1}]")]
     InvalidArgumentCount(usize, String),
     #[error("error parsing attribute: [{0}]")]
@@ -287,9 +286,15 @@ pub enum CommandParseError {
     #[error("command execution error: [{0}]")]
     CommandError(#[from] CommandError),
     #[error("string utf8 error")]
-    StringUtf8Error(#[from] FromUtf8Error),
+    StringUtf8Error(String),
-    #[error("parsing string value error: [{0}]")]
+    #[error("parsing error: [{0}]")]
-    StringParseError(#[from] StringParseError),
+    StringParseError(#[from] ParseError),
+}
+
+impl From<FromUtf8Error> for CommandParseError {
+    fn from(value: FromUtf8Error) -> Self {
+        Self::StringUtf8Error(value.to_string())
+    }
 }
 
 impl serde::de::Expected for CommandParseError {
@@ -298,14 +303,6 @@ impl serde::de::Expected for CommandParseError {
     }
 }
 
-fn trim_quotes(itm: String) -> String {
-    if itm.starts_with('"') && itm.ends_with('"') {
-        itm.trim_matches('"').to_string()
-    } else {
-        itm
-    }
-}
-
 impl HlwmCommand {
     pub fn silent(self) -> HlwmCommand {
         HlwmCommand::Silent(Box::new(self))
@@ -315,14 +312,33 @@ impl HlwmCommand {
         HlwmCommand::Try(Box::new(self))
     }
 
-    pub fn from_raw_parts(command: &str, args: Vec<String>) -> Result<Self, CommandParseError> {
+    pub fn execute(self) -> Result<process::Output, CommandError> {
-        let command = HlwmCommand::iter()
+        Client::new().execute(self)
-            .find(|cmd| cmd.to_string() == command)
+    }
-            .ok_or(CommandParseError::UnknownCommand(command.to_string()))?;
 
-        gen_parse!(command, args);
+    pub fn execute_str(self) -> Result<String, CommandError> {
+        Ok(String::from_utf8(self.execute()?.stdout)?)
+    }
+}
 
-        let parsed_command = match command {
+impl FromCommandArgs for HlwmCommand {
+    fn from_command_args<S: Into<String>, I: Iterator<Item = S>>(
+        cmd_name: &str,
+        args: I,
+    ) -> Result<Self, super::parser::ParseError> {
+        let mut command = HlwmCommand::iter()
+            .find(|cmd| cmd.to_string() == cmd_name)
+            .ok_or(parser::ParseError::InvalidCommand(cmd_name.to_string()))?;
+        // Since HlwmCommand will often be parsed by its constituent commands (such as keybind)
+        // just passing in `args.map(|s| s.into())` results in the typechecker overflowing its
+        // recursion limit. So, to get around this, HlwmCommand will collect whatever type I
+        // is into a `Vec<String>`, which makes this loop not infinite.
+        //
+        // There really should be a better error for this. I'm lucky I figured it out fast.
+        let mut parser =
+            ArgParser::from_strings(args.map(|s| s.into()).collect::<Vec<_>>().into_iter());
+
+        match command.borrow_mut() {
             HlwmCommand::Quit
             | HlwmCommand::Lock
             | HlwmCommand::Cycle
@@ -336,252 +352,199 @@ impl HlwmCommand {
             | HlwmCommand::True
             | HlwmCommand::Mouseunbind
             | HlwmCommand::ListMonitors
-            | HlwmCommand::ListKeybinds => Ok::<_, CommandParseError>(command.clone()),
+            | HlwmCommand::ListKeybinds => (),
-            HlwmCommand::Echo(_) => Ok(Self::Echo(args)),
+            HlwmCommand::Echo(arg) => *arg = vec![parser.collect::<Vec<_>>().join(" ")],
-            HlwmCommand::Close { window: _ } => parse!(window: [Option<FromStr>] => Close),
+            HlwmCommand::Close { window } => {
-            HlwmCommand::Spawn {
+                *window = parser.optional_next_from_str("close(window)")?
-                executable: _,
-                args: _,
-            } => {
-                parse!(executable: String, args: [Vec<String>] => Spawn).map(|spawn| match spawn {
-                    HlwmCommand::Spawn { executable, args } => HlwmCommand::Spawn {
-                        executable: trim_quotes(executable),
-                        args: args.into_iter().map(trim_quotes).collect(),
-                    },
-                    _ => unreachable!(),
-                })
             }
-            HlwmCommand::GetAttr(_) => parse!(String => GetAttr),
+            HlwmCommand::Spawn { executable, args } => {
-            HlwmCommand::SetAttr {
+                *executable = parser.must_string("spawn(executable)")?;
-                path: _,
+                *args = parser.collect();
-                new_value: _,
-            } => {
-                let mut args = args.into_iter();
-                let path = args.next().ok_or(CommandParseError::BadArgument {
-                    command: command.to_string(),
-                })?;
-                Ok(HlwmCommand::SetAttr {
-                    path: path.clone(),
-                    new_value: {
-                        Attribute::new(
-                            &String::from_utf8(
-                                Client::new()
-                                    .execute(HlwmCommand::AttrType(path.clone()))?
-                                    .stdout,
-                            )?
-                            .split('\n')
-                            .next()
-                            .ok_or(CommandParseError::CommandError(CommandError::Empty))?,
-                            &args.collect::<Vec<_>>().join(" "),
-                        )?
-                    },
-                })
             }
-            HlwmCommand::Attr {
+            HlwmCommand::GetAttr(attr) => *attr = parser.must_string("get_attr")?,
-                path: _,
+            HlwmCommand::SetAttr { path, new_value } => {
-                new_value: _,
+                *path = parser.must_string("set_attr(path)")?;
-            } => {
+                *new_value =
-                let mut args = args.into_iter();
+                    parser.collect_from_strings_hint(path.as_str(), "set_attr(new_value)")?;
-                Ok(HlwmCommand::Attr {
-                    path: args.next().ok_or(CommandParseError::BadArgument {
-                        command: command.to_string(),
-                    })?,
-                    new_value: {
-                        let args = args.collect::<Vec<_>>();
-                        if args.is_empty() {
-                            None
-                        } else {
-                            Some(Attribute::guess_type(&args.join("\t")))
-                        }
-                    },
-                })
             }
-            HlwmCommand::NewAttr { path: _, attr: _ } => {
+            HlwmCommand::Attr { path, new_value } => {
-                let mut args = args.into_iter();
+                *path = parser.must_string("attr(path)")?;
-                let attr_type = args.next().ok_or(CommandParseError::BadArgument {
+                *new_value = parser
-                    command: command.to_string(),
+                    .collect::<Vec<_>>()
-                })?;
+                    .to_option()
-                let path = args.next().ok_or(CommandParseError::BadArgument {
+                    .map(|t| {
-                    command: command.to_string(),
+                        let value = t.join(" ");
-                })?;
+                        Attribute::new(AttributeType::get_type_or_guess(&path, &value), &value)
-                Ok(HlwmCommand::NewAttr {
-                    path,
-                    attr: {
-                        let attr = args.collect::<Vec<String>>();
-                        let attr = if attr.len() == 0 {
-                            None
-                        } else {
-                            Some(attr.join("\t"))
-                        };
-                        AttributeOption::new(&attr_type, &attr)?
-                    },
-                })
-            }
-            HlwmCommand::AttrType(_) => parse!(String => AttrType),
-            HlwmCommand::RemoveAttr(_) => parse!(String => RemoveAttr),
-            HlwmCommand::Set(_) => parse!(FromStrAll => Set),
-            HlwmCommand::EmitHook(_) => parse!(FromStr => EmitHook),
-            HlwmCommand::Keybind(_) => parse!(FromStrAll => Keybind),
-            HlwmCommand::Keyunbind(_) => parse!(FromStr => Keyunbind),
-            HlwmCommand::Mousebind(_) => parse!(FromStrAll => Mousebind),
-            HlwmCommand::JumpTo(_) => parse!(FromStr => JumpTo),
-            HlwmCommand::AddTag(_) => parse!(String => AddTag),
-            HlwmCommand::MergeTag { tag: _, target: _ } => {
-                parse!(tag: String, target: [Option<FromStr>] => MergeTag)
-            }
-            HlwmCommand::Focus(_) => parse!(FromStr => Focus),
-            HlwmCommand::Shift(_) => parse!(FromStr => Shift),
-            HlwmCommand::Split(_) => parse!(FromStrAll => Split),
-            HlwmCommand::Fullscreen(_) => parse!(FromStr => Fullscreen),
-            HlwmCommand::CycleLayout {
-                delta: _,
-                layouts: _,
-            } => {
-                if args.is_empty() {
-                    Ok(HlwmCommand::CycleLayout {
-                        delta: None,
-                        layouts: vec![],
                     })
-                } else {
+                    .flip()?;
-                    let first = args.first().ok_or(CommandParseError::BadArgument {
+            }
-                        command: command.to_string(),
+            HlwmCommand::NewAttr { path, attr } => {
-                    })?;
+                let attr_type: AttributeType = parser.next_from_str("new_attr(attr_type)")?;
-                    match FrameLayout::from_str(first) {
+                *path = parser.must_string("new_attr(path)")?;
-                        Ok(_) => {
+                let value = parser.collect::<Vec<_>>().to_option().map(|v| v.join(" "));
-                            // only frame layouts
+                *attr = AttributeOption::new(attr_type, value.as_ref())?;
-                            Ok(HlwmCommand::CycleLayout {
+            }
-                                delta: None,
+            HlwmCommand::AttrType(path) | HlwmCommand::RemoveAttr(path) => {
-                                layouts: args
+                *path = parser.must_string(cmd_name)?
-                                    .into_iter()
+            }
-                                    .map(|i| FrameLayout::from_str(&i))
+            HlwmCommand::Set(setting) => *setting = parser.collect_command(cmd_name)?,
-                                    .collect::<Result<_, _>>()
+            HlwmCommand::EmitHook(hook) => *hook = parser.collect_command(cmd_name)?,
-                                    .map_err(|_| CommandParseError::BadArgument {
+            HlwmCommand::Keybind(keybind) => *keybind = parser.collect_from_strings(cmd_name)?,
-                                        command: command.to_string(),
+            HlwmCommand::Keyunbind(keyunbind) => *keyunbind = parser.next_from_str(cmd_name)?,
-                                    })?,
+            HlwmCommand::Mousebind(mouseunbind) => {
-                            })
+                *mouseunbind = parser.collect_from_strings(cmd_name)?
+            }
+            HlwmCommand::JumpTo(win) => *win = parser.next_from_str(cmd_name)?,
+            HlwmCommand::AddTag(tag) => *tag = parser.must_string(cmd_name)?,
+            HlwmCommand::MergeTag { tag, target } => {
+                *tag = parser.must_string("merge_tag(tag)")?;
+                *target = parser.optional_next_from_str("merge_tag(target)")?;
+            }
+            HlwmCommand::Focus(dir) | HlwmCommand::Shift(dir) => {
+                *dir = parser.next_from_str(cmd_name)?
+            }
+            HlwmCommand::Split(align) => *align = parser.collect_from_strings(cmd_name)?,
+            HlwmCommand::Fullscreen(set) => *set = parser.next_from_str(cmd_name)?,
+            HlwmCommand::CycleLayout { delta, layouts } => {
+                let cycle_res = parser.try_first(
+                    |s| Index::<i32>::from_str(s),
+                    |s| Ok(FrameLayout::from_str(s)?),
+                    cmd_name,
+                );
+                match cycle_res {
+                    Ok(res) => match res {
+                        Either::Left(idx) => {
+                            *delta = Some(idx);
+                            *layouts = parser.collect_from_str("cycle_layout(layouts)")?;
                         }
-                        Err(_) => {
+                        Either::Right(layout) => {
-                            // Has index
+                            *delta = None;
-                            let mut args = args.into_iter();
+                            *layouts = [layout]
-                            Ok(HlwmCommand::CycleLayout {
+                                .into_iter()
-                                delta: Some(args.next().unwrap().parse().map_err(|_| {
+                                .chain(parser.collect_from_str("cycle_layout(layouts)")?)
-                                    CommandParseError::BadArgument {
+                                .collect();
-                                        command: command.to_string(),
+                        }
-                                    }
+                    },
-                                })?),
+                    Err(err) => {
-                                layouts: args
+                        if let ParseError::Empty = err {
-                                    .map(|i| FrameLayout::from_str(&i))
+                            *delta = None;
-                                    .collect::<Result<_, _>>()
+                            *layouts = vec![];
-                                    .map_err(|_| CommandParseError::BadArgument {
+                        } else {
-                                        command: command.to_string(),
+                            return Err(err);
-                                    })?,
-                            })
                         }
                     }
                 }
             }
             HlwmCommand::Resize {
-                direction: _,
+                direction,
-                fraction_delta: _,
+                fraction_delta,
-            } => parse!(direction: FromStr, fraction_delta: [Option<FromStr>] => Resize),
+            } => {
+                *direction = parser.next_from_str("resize(direction)")?;
+                *fraction_delta = parser.optional_next_from_str("resize(fraction_delta)")?;
+            }
             HlwmCommand::Or {
-                separator: _,
+                separator,
-                commands: _,
+                commands,
-            } => parse!(And_Or => Or),
+            }
-            HlwmCommand::And {
+            | HlwmCommand::And {
-                separator: _,
+                separator,
-                commands: _,
+                commands,
-            } => parse!(And_Or => And),
+            } => {
+                *separator = parser.next_from_str(&format!("{cmd_name}(separator)"))?;
+                let commands_wrap: AndOrCommands = parser
+                    .collect_from_strings_hint(*separator, &format!("{cmd_name}(commands)"))?;
+                *commands = commands_wrap.commands();
+            }
             HlwmCommand::Compare {
-                attribute: _,
+                attribute,
-                operator: _,
+                operator,
-                value: _,
+                value,
-            } => parse!(attribute: String, operator: FromStr, value: String => Compare),
-            HlwmCommand::TagStatus { monitor: _ } => {
-                parse!(monitor: [Option<FromStr>] => TagStatus)
-            }
-            HlwmCommand::Rule(_) => parse!(FromStrAll => Rule),
-            HlwmCommand::Get(_) => parse!(FromStr => Get),
-            HlwmCommand::MoveIndex {
-                index: _,
-                skip_visible: _,
             } => {
-                parse!(skip_visible: [Flag("--skip-visible")], index: FromStr => MoveIndex)
+                *attribute = parser.must_string("compare(attribute)")?;
+                *operator = parser.next_from_str("compare(operator)")?;
+                *value = parser.must_string("compare(value)")?;
             }
+            HlwmCommand::TagStatus { monitor } => {
+                *monitor = parser.optional_next_from_str(cmd_name)?;
+            }
+            HlwmCommand::Rule(rule) => {
+                *rule = parser.collect_from_strings(cmd_name)?;
+            }
+            HlwmCommand::Get(set) => *set = parser.next_from_str(cmd_name)?,
             HlwmCommand::UseIndex {
-                index: _,
+                index,
-                skip_visible: _,
+                skip_visible,
-            } => {
+            }
-                parse!(skip_visible: [Flag("--skip-visible")], index: FromStr => UseIndex)
+            | HlwmCommand::MoveIndex {
+                index,
+                skip_visible,
+            } => {
+                let (args, skip) = parser.collect_strings_with_flag("--skip-visible");
+                *skip_visible = skip;
+                *index = ArgParser::from_strings(args.into_iter()).next_from_str(cmd_name)?;
+            }
+            HlwmCommand::MoveTag(tag) | HlwmCommand::UseTag(tag) => {
+                *tag = parser.must_string(cmd_name)?
+            }
+            HlwmCommand::Try(hlcmd) | HlwmCommand::Silent(hlcmd) => {
+                *hlcmd = Box::new(parser.collect_command(cmd_name)?);
             }
-            HlwmCommand::UseTag(_) => parse!(String => UseTag),
-            HlwmCommand::MoveTag(_) => parse!(String => MoveTag),
-            HlwmCommand::Try(_) => parse!(FromStrAll => Try),
-            HlwmCommand::Silent(_) => parse!(FromStrAll => Silent),
             HlwmCommand::MonitorRect {
-                monitor: _,
+                monitor,
-                without_pad: _,
+                without_pad,
             } => {
-                parse!(without_pad: [Flag("-p")], monitor: [Option<FromStr>] => MonitorRect)
+                let (args, pad) = parser.collect_strings_with_flag("-p");
+                *without_pad = pad;
+                *monitor = args.first().map(|s| u32::from_str(s)).flip()?;
             }
-            HlwmCommand::Pad { monitor: _, pad: _ } => {
+            HlwmCommand::Pad { monitor, pad } => {
-                parse!(monitor: FromStr, pad: FromStrAll => Pad)
+                *monitor = parser.next_from_str("pad(monitor)")?;
+                *pad = parser.collect_from_strings("pad(pad)")?;
             }
             HlwmCommand::Substitute {
-                identifier: _,
+                identifier,
-                attribute_path: _,
+                attribute_path,
-                command: _,
+                command,
             } => {
-                parse!(identifier: String, attribute_path: String, command: FromStrAll => Substitute)
+                *identifier = parser.must_string("substitute(identifier)")?;
+                *attribute_path = parser.must_string("substitute(attribute_path)")?;
+                *command = Box::new(parser.collect_command("substitute(command)")?);
             }
             HlwmCommand::ForEach {
-                unique: _,
+                unique,
-                recursive: _,
+                recursive,
-                filter_name: _,
+                filter_name,
-                identifier: _,
+                identifier,
-                object: _,
+                object,
+                command,
             } => {
-                let (params, args): (Vec<String>, Vec<String>) =
+                // Note: ForEach is still likely bugged due to this method of parsing the parts, as foreach may be nested
-                    args.into_iter().partition(|a| a.starts_with("--"));
+                let (normal, flags): (Vec<_>, Vec<_>) = parser.inner().partition(|c| {
-
+                    !c.starts_with("--unique")
-                if args.len() < 2 {
+                        && !c.starts_with("--filter-name")
-                    return Err(CommandParseError::BadArgument {
+                        && !c.starts_with("--recursive")
-                        command: command.to_string(),
+                });
-                    });
+                let mut normal = ArgParser::from_strings(normal.into_iter());
-                }
+                for flag in flags {
-                let mut args = args.into_iter();
+                    match flag.as_str() {
-                let (identifier, object) =
+                        "--unique" => *unique = true,
-                    (args.next().unwrap(), args.collect::<Vec<_>>().join("\t"));
+                        "--recursive" => *recursive = true,
-                let mut unique = false;
-                let mut recursive = false;
-                let mut filter_name: Option<String> = None;
-                for param in params {
-                    match param.as_str() {
-                        "--unique" => unique = true,
-                        "--recursive" => recursive = true,
                         other => {
                             if let Some(name) = other.strip_prefix("--filter-name=") {
-                                filter_name = Some(name.to_string());
+                                *filter_name = Some(name.to_string());
                             }
                         }
                     }
                 }
-
+                *identifier = normal.must_string("for_each(identifier)")?;
-                Ok(HlwmCommand::ForEach {
+                *object = normal.must_string("for_each(object)")?;
-                    unique,
+                *command = Box::new(normal.collect_command("for_each(command)")?);
-                    recursive,
-                    filter_name,
-                    identifier,
-                    object,
-                })
             }
-            HlwmCommand::Sprintf(_) => parse!([Vec<String>] => Sprintf),
+            HlwmCommand::Sprintf(s) => *s = parser.collect(),
-            HlwmCommand::Unrule(_) => parse!(FromStr => Unrule),
+            HlwmCommand::Unrule(unrule) => *unrule = parser.next_from_str(cmd_name)?,
-        }?;
+        };
 
-        assert_eq!(command.to_string(), parsed_command.to_string());
+        Ok(command)
-
-        Ok(parsed_command)
     }
 }
 
@@ -611,10 +574,12 @@ pub enum CommandError {
     UtfError(#[from] FromUtf8Error),
     #[error("attribute error: {0:?}")]
     AttributeError(#[from] AttributeError),
-    #[error("string parse error: {0}")]
+    #[error("parse error: {0}")]
-    StringParseError(#[from] StringParseError),
+    ParseError(#[from] ParseError),
     #[error("unexpected empty result")]
     Empty,
+    #[error("invalid value")]
+    Invalid,
 }
 
 impl From<io::Error> for CommandError {
@@ -803,8 +768,9 @@ impl ToCommandString for HlwmCommand {
                 filter_name,
                 identifier,
                 object,
+                command,
             } => {
-                let mut parts = Vec::with_capacity(6);
+                let mut parts = Vec::with_capacity(7);
                 parts.push(self.to_string());
                 parts.push(identifier.to_string());
                 parts.push(object.to_string());
@@ -817,6 +783,7 @@ impl ToCommandString for HlwmCommand {
                 if *recursive {
                     parts.push("--recursive".into());
                 }
+                parts.push(command.to_command_string());
                 parts.join("\t")
             }
             HlwmCommand::Sprintf(args) => [self.to_string()]
@@ -833,6 +800,40 @@ impl ToCommandString for HlwmCommand {
     }
 }
 
+#[derive(Default)]
+struct ForEach {
+    unique: bool,
+    recursive: bool,
+    filter_name: Option<String>,
+    identifier: String,
+    object: String,
+    command: HlwmCommand,
+}
+
+impl FromStrings for ForEach {
+    fn from_strings<I: Iterator<Item = String>>(s: I) -> Result<Self, ParseError> {
+        let mut for_each = Self::default();
+        let (normal, flags): (Vec<_>, Vec<_>) = s.partition(|c| !c.starts_with("--"));
+        let mut normal = ArgParser::from_strings(normal.into_iter());
+        for flag in flags {
+            match flag.as_str() {
+                "--unique" => for_each.unique = true,
+                "--recursive" => for_each.recursive = true,
+                other => {
+                    if let Some(filter_name) = other.strip_prefix("--filter-name=") {
+                        for_each.filter_name = Some(filter_name.to_string());
+                    }
+                }
+            }
+        }
+        for_each.identifier = normal.must_string("for_each(identifier)")?;
+        for_each.object = normal.must_string("for_each(object)")?;
+        for_each.command = normal.collect_command("for_each(command)")?;
+
+        Ok(for_each)
+    }
+}
+
 #[cfg(test)]
 mod test {
 
@@ -843,7 +844,7 @@ mod test {
         command::{FrameLayout, HlwmCommand, Index, Operator, Separator},
         hlwmbool::ToggleBool,
         hook::Hook,
-        key::{Key, KeyUnbind, MouseButton, Mousebind, MousebindAction},
+        key::{Key, KeyUnbind, Keybind, MouseButton, Mousebind, MousebindAction},
         pad::Pad,
         rule::{Condition, Consequence, Rule, RuleOperator},
         setting::{Setting, SettingName},
@@ -937,7 +938,10 @@ mod test {
                     "emit_hook\treload".into(),
                 ),
                 HlwmCommand::Keybind(_) => (
-                    HlwmCommand::Keybind("Mod4+1\treload".parse().unwrap()),
+                    HlwmCommand::Keybind(Keybind::new(
+                        [Key::Mod4Super, Key::Char('1')],
+                        HlwmCommand::Reload,
+                    )),
                     "keybind\tMod4+1\treload".into(),
                 ),
                 HlwmCommand::Keyunbind(_) => (
@@ -1122,6 +1126,7 @@ mod test {
                     filter_name: _,
                     identifier: _,
                     object: _,
+                    command: _,
                 } => (
                     HlwmCommand::ForEach {
                         unique: true,
@@ -1129,8 +1134,10 @@ mod test {
                         filter_name: Some(".+".into()),
                         identifier: "CLIENT".into(),
                         object: "clients.".into(),
+                        command: Box::new(HlwmCommand::Cycle),
                     },
-                    "foreach\tCLIENT\tclients.\t--filter-name=.+\t--unique\t--recursive".into(),
+                    "foreach\tCLIENT\tclients.\t--filter-name=.+\t--unique\t--recursive\tcycle"
+                        .into(),
                 ),
                 HlwmCommand::Sprintf(_) => (
                     HlwmCommand::Sprintf(

src/hlwm/hlwmbool.rs

@@ -2,14 +2,17 @@ use std::{fmt::Display, str::FromStr};
 
 use serde::{Deserialize, Serialize};
 
-use super::StringParseError;
+use super::parser::ParseError;
 
 #[inline(always)]
-pub fn from_hlwm_string(s: &str) -> Result<bool, StringParseError> {
+pub fn from_hlwm_string(s: &str) -> Result<bool, ParseError> {
     match s {
         "on" | "true" => Ok(true),
         "off" | "false" => Ok(false),
-        _ => Err(StringParseError::BoolError(s.to_string())),
+        _ => Err(ParseError::PrimitiveError(format!(
+            "value [{s}] is not a {}",
+            std::any::type_name::<bool>()
+        ))),
     }
 }
 
@@ -26,7 +29,7 @@ impl From<bool> for ToggleBool {
 }
 
 impl FromStr for ToggleBool {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         if let Ok(b) = from_hlwm_string(s) {
@@ -34,7 +37,10 @@ impl FromStr for ToggleBool {
         } else if s == "toggle" {
             Ok(Self::Toggle)
         } else {
-            Err(StringParseError::UnknownValue)
+            Err(ParseError::PrimitiveError(format!(
+                "value [{s}] is not a {}",
+                std::any::type_name::<ToggleBool>()
+            )))
         }
     }
 }

src/hlwm/hook.rs

@@ -1,11 +1,13 @@
-use std::str::FromStr;
+use std::{borrow::BorrowMut, str::FromStr};
 
 use serde::{Deserialize, Serialize};
 use strum::IntoEnumIterator;
 
-use crate::gen_parse;
+use super::{
-
+    parser::{ArgParser, FromCommandArgs, ParseError},
-use super::{command::CommandParseError, window::Window, Monitor, TagSelect, ToCommandString};
+    window::Window,
+    Monitor, TagSelect, ToCommandString,
+};
 
 #[derive(Debug, Clone, Serialize, Deserialize, strum::Display, strum::EnumIter, PartialEq)]
 #[strum(serialize_all = "snake_case")]
@@ -59,55 +61,55 @@ pub enum Hook {
     Reload,
 }
 
-impl Hook {
+impl FromCommandArgs for Hook {
-    fn from_raw_parts(command: &str, args: Vec<String>) -> Result<Self, CommandParseError> {
+    fn from_command_args<S: Into<String>, I: IntoIterator<Item = S>>(
-        let command = Self::iter()
+        cmd: &str,
-            .find(|cmd| cmd.to_string() == command)
+        args: I,
-            .ok_or(CommandParseError::UnknownCommand(command.to_string()))?;
+    ) -> Result<Self, super::parser::ParseError> {
-        gen_parse!(command, args);
+        let mut command = Self::iter()
+            .find(|c: &Hook| c.to_string() == cmd)
+            .ok_or(ParseError::InvalidCommand(cmd.to_string()))?;
 
-        match command {
+        let x = args.into_iter().map(|i| i.into()).collect::<Vec<String>>();
-            Hook::AttributeChanged {
+        let mut parser = ArgParser::from_strings(x.into_iter());
-                path: _,
+
-                old: _,
+        match command.borrow_mut() {
-                new: _,
+            Hook::QuitPanel | Hook::Reload | Hook::TagFlags => (),
-            } => parse!(path: String, old: String, new: String => AttributeChanged),
+            Hook::AttributeChanged { path, old, new } => {
-            Hook::Fullscreen { on: _, window: _ } => {
+                *path = parser.must_string("attribute_changed(path)")?;
-                parse!(on: FromStr, window: FromStr => Fullscreen)
+                *old = parser.must_string("attribute_changed(old)")?;
+                *new = parser.must_string("attribute_changed(new)")?;
             }
-            Hook::TagChanged { tag: _, monitor: _ } => {
+            Hook::Fullscreen { on, window } => {
-                parse!(tag: String, monitor: FromStr => TagChanged)
+                *on = parser.next_from_str("fullscreen(on)")?;
+                *window = parser.next_from_str("fullscreen(window)")?;
+            }
+            Hook::TagChanged { tag, monitor } => {
+                *tag = parser.must_string("tag_changed(tag)")?;
+                *monitor = parser.next_from_str("tag_changed(monitor)")?;
+            }
+            Hook::FocusChanged { window, title } | Hook::WindowTitleChanged { window, title } => {
+                *window = parser.next_from_str(&format!("{cmd}(window)"))?;
+                *title = parser.must_string(&format!("{cmd}(title)"))?;
+            }
+            Hook::TagAdded(tag) => {
+                *tag = TagSelect::from_str(&parser.must_string(cmd)?).expect("infallible")
+            }
+            Hook::TagRenamed { old, new } => {
+                *old = parser.must_string("tag_renamed(old)")?;
+                *new = parser.must_string("tag_renamed(new)")?;
+            }
+            Hook::Urgent { on, window } => {
+                *on = parser.next_from_str("urgent(on)")?;
+                *window = parser.next_from_str("urgent(window)")?;
+            }
+            Hook::Rule { hook, window } => {
+                *hook = parser.must_string("rule(hook)")?;
+                *window = parser.next_from_str("rule(window)")?;
             }
-            Hook::FocusChanged {
-                window: _,
-                title: _,
-            } => parse!(window: FromStr, title: String => FocusChanged),
-            Hook::WindowTitleChanged {
-                window: _,
-                title: _,
-            } => parse!(window: FromStr, title: String => WindowTitleChanged),
-            Hook::TagFlags => Ok(Hook::TagFlags),
-            Hook::TagAdded(_) => parse!(FromStr => TagAdded),
-            Hook::TagRenamed { old: _, new: _ } => parse!(old: String, new: String => TagRenamed),
-            Hook::Urgent { on: _, window: _ } => parse!(on: Bool, window: FromStr => Urgent),
-            Hook::Rule { hook: _, window: _ } => parse!(hook: String, window: FromStr => Rule),
-            Hook::QuitPanel => Ok(Hook::QuitPanel),
-            Hook::Reload => Ok(Hook::Reload),
         }
-    }
-}
 
-impl FromStr for Hook {
+        Ok(command)
-    type Err = CommandParseError;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let mut split = s.split("\t");
-        Hook::from_raw_parts(
-            split
-                .next()
-                .ok_or(CommandParseError::UnknownCommand(format!("hook {s}")))?,
-            split.map(String::from).collect(),
-        )
     }
 }
 

src/hlwm/key.rs

@@ -1,14 +1,14 @@
-use std::{fmt::Display, str::FromStr};
+use std::{borrow::BorrowMut, fmt::Display, str::FromStr};
 
 use serde::{Deserialize, Serialize};
 use strum::IntoEnumIterator;
-use thiserror::Error;
 
-use crate::split;
+use crate::{logerr::UnwrapLog, split};
 
 use super::{
-    command::{CommandParseError, HlwmCommand},
+    command::HlwmCommand,
-    StringParseError, ToCommandString,
+    parser::{ArgParser, FromCommandArgs, FromStrings, ParseError},
+    ToCommandString,
 };
 
 #[derive(Debug, Clone, Copy, PartialEq, strum::EnumIter)]
@@ -43,6 +43,21 @@ pub enum Key {
     Mouse(MouseButton),
 }
 
+fn title_case(s: &str) -> String {
+    let mut chars = s.chars();
+    let mut out_chars = Vec::with_capacity(s.len());
+
+    if let Some(first) = chars.next() {
+        out_chars.push(first.to_ascii_uppercase());
+    }
+
+    while let Some(c) = chars.next() {
+        out_chars.push(c.to_ascii_lowercase());
+    }
+
+    out_chars.into_iter().collect()
+}
+
 impl Key {
     pub fn is_standard(&self) -> bool {
         match self {
@@ -62,6 +77,63 @@ impl Key {
             _ => false,
         }
     }
+
+    pub fn parse_keybind_keys(s: &str) -> Result<Vec<Key>, ParseError> {
+        s.split(['-', '+'])
+            .map(Key::from_str)
+            .collect::<Result<Vec<_>, _>>()
+    }
+    pub fn from_str_case_insensitive(s: &str) -> Result<Self, ParseError> {
+        Key::from_str(&title_case(s))
+    }
+
+    /// Returns the name for this key as it should appear in the config
+    pub fn config_name(&self) -> String {
+        self.aliases()
+            .first()
+            .expect_log("all keys should have at least one alias")
+            .to_string()
+    }
+
+    /// Returns a list of aliases for the key
+    pub fn aliases(&self) -> Vec<String> {
+        match self {
+            Key::Mod1Alt => vec!["Alt".into(), "Mod1".into()],
+            Key::Mod4Super => vec!["Super".into(), "Mod4".into()],
+            Key::Return => vec!["Return".into(), "Enter".into()],
+            Key::Shift => vec!["Shift".into()],
+            Key::Tab => vec!["Tab".into()],
+            Key::Left => vec!["Left".into()],
+            Key::Right => vec!["Right".into()],
+            Key::Up => vec!["Up".into()],
+            Key::Down => vec!["Down".into()],
+            Key::Space => vec!["Space".into()],
+            Key::Control => vec!["Ctrl".into(), "Ctl".into(), "Control".into()],
+            Key::Backtick => vec!["Backtick".into(), "Grave".into()],
+            Key::F1 => vec!["F1".into()],
+            Key::F2 => vec!["F2".into()],
+            Key::F3 => vec!["F3".into()],
+            Key::F4 => vec!["F4".into()],
+            Key::F5 => vec!["F5".into()],
+            Key::F6 => vec!["F6".into()],
+            Key::F7 => vec!["F7".into()],
+            Key::F8 => vec!["F8".into()],
+            Key::F9 => vec!["F9".into()],
+            Key::F10 => vec!["F10".into()],
+            Key::F11 => vec!["F11".into()],
+            Key::F12 => vec!["F12".into()],
+            Key::Home => vec!["Home".into()],
+            Key::Delete => vec!["Delete".into(), "Del".into()],
+            Key::Char(c) => vec![c.to_ascii_lowercase().to_string()],
+            Key::Mouse(m) => vec![m.to_string()],
+        }
+    }
+}
+
+impl Display for Key {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(&self.aliases().first().unwrap())
+    }
 }
 
 impl From<MouseButton> for Key {
@@ -81,7 +153,7 @@ impl Serialize for Key {
     where
         S: serde::Serializer,
     {
-        serializer.serialize_str(&self.to_string())
+        serializer.serialize_str(&self.config_name())
     }
 }
 
@@ -97,14 +169,14 @@ impl<'de> Deserialize<'de> for Key {
             }
         }
         let str_val: String = Deserialize::deserialize(deserializer)?;
-        Ok(Self::from_str(&str_val).map_err(|_| {
+        Ok(Self::from_str_case_insensitive(&str_val).map_err(|_| {
             serde::de::Error::invalid_value(serde::de::Unexpected::Str(&str_val), &ExpectedKey)
         })?)
     }
 }
 
 impl FromStr for Key {
-    type Err = KeyParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
@@ -112,16 +184,21 @@ impl FromStr for Key {
                 Ok(Self::Mouse(MouseButton::from_str(s)?))
             }
             _ => {
-                if let Some(key) = Self::iter().into_iter().find(|key| key.to_string() == s) {
+                if let Some(key) = Self::iter().into_iter().find(|key| match key {
-                    match key {
+                    Key::Char(_) | Key::Mouse(_) => false,
-                        Key::Char(_) | Key::Mouse(_) => (),
+                    _ => key.aliases().contains(&s.to_string()),
-                        _ => return Ok(key),
+                }) {
-                    }
+                    return Ok(key);
                 }
                 if s.len() == 1 {
-                    Ok(Self::Char(s.chars().next().unwrap()))
+                    Ok(Self::Char(
+                        s.chars().next().unwrap_log().to_ascii_lowercase(),
+                    ))
                 } else {
-                    Err(KeyParseError::ExpectedCharKey(s.to_string()))
+                    Err(ParseError::InvalidValue {
+                        value: s.to_string(),
+                        expected: "a valid key",
+                    })
                 }
             }
         }
@@ -144,7 +221,7 @@ impl Default for MouseButton {
 }
 
 impl FromStr for MouseButton {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
@@ -153,7 +230,7 @@ impl FromStr for MouseButton {
             "Button3" => Ok(Self::Button3),
             "Button4" => Ok(Self::Button4),
             "Button5" => Ok(Self::Button5),
-            _ => Err(StringParseError::UnknownValue),
+            _ => Err(ParseError::InvalidCommand(s.to_string())),
         }
     }
 }
@@ -170,42 +247,6 @@ impl Display for MouseButton {
     }
 }
 
-impl Display for Key {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let data = match self {
-            Key::Return => "Return".to_string(),
-            Key::Shift => "Shift".to_string(),
-            Key::Tab => "Tab".to_string(),
-            Key::Left => "Left".to_string(),
-            Key::Right => "Right".to_string(),
-            Key::Up => "Up".to_string(),
-            Key::Down => "Down".to_string(),
-            Key::Space => "space".to_string(),
-            Key::Control => "Control".to_string(),
-            Key::Backtick => "grave".to_string(),
-            Key::Char(c) => c.to_string(),
-            Key::Mouse(m) => m.to_string(),
-            Key::F1 => "F1".to_string(),
-            Key::F2 => "F2".to_string(),
-            Key::F3 => "F3".to_string(),
-            Key::F4 => "F4".to_string(),
-            Key::F5 => "F5".to_string(),
-            Key::F6 => "F6".to_string(),
-            Key::F7 => "F7".to_string(),
-            Key::F8 => "F8".to_string(),
-            Key::F9 => "F9".to_string(),
-            Key::F10 => "F10".to_string(),
-            Key::F11 => "F11".to_string(),
-            Key::F12 => "F12".to_string(),
-            Key::Home => "Home".to_string(),
-            Key::Delete => "Delete".to_string(),
-            Key::Mod1Alt => "Mod1".to_string(),
-            Key::Mod4Super => "Mod4".to_string(),
-        };
-        f.write_str(&data)
-    }
-}
-
 #[derive(Debug, Clone, strum::Display, strum::EnumIter, PartialEq)]
 #[strum(serialize_all = "snake_case")]
 pub enum MousebindAction {
@@ -242,27 +283,48 @@ impl<'de> Deserialize<'de> for MousebindAction {
     }
 }
 
-impl FromStr for MousebindAction {
+impl FromCommandArgs for MousebindAction {
-    type Err = StringParseError;
+    fn from_command_args<S: Into<String>, I: Iterator<Item = S>>(
+        command: &str,
+        args: I,
+    ) -> Result<Self, ParseError> {
+        let mut action = Self::iter()
+            .find(|i| i.to_string() == command)
+            .ok_or(ParseError::InvalidCommand(command.to_string()))?;
 
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match action.borrow_mut() {
-        let mut parts = split::tab_or_space(s).into_iter();
+            MousebindAction::Move | MousebindAction::Resize | MousebindAction::Zoom => (),
-        let first = parts.next().ok_or(StringParseError::UnknownValue)?;
+            MousebindAction::Call(arg) => {
-        let act = Self::iter()
+                *arg = Box::new(
-            .find(|i| i.to_string() == first)
+                    ArgParser::from_strings(args.map(|a| a.into()))
-            .ok_or(StringParseError::UnknownValue)?;
+                        .collect_command("mousebind_args(command)")?,
-
+                )
-        match act {
-            MousebindAction::Move | MousebindAction::Resize | MousebindAction::Zoom => Ok(act),
-            MousebindAction::Call(_) => {
-                let command = parts.next().ok_or(StringParseError::UnknownValue)?;
-                let args = parts.collect();
-                Ok(MousebindAction::Call(Box::new(
-                    HlwmCommand::from_raw_parts(&command, args)
-                        .map_err(|err| StringParseError::CommandParseError(err.to_string()))?,
-                )))
             }
         }
+
+        Ok(action)
+    }
+}
+
+impl FromStr for MousebindAction {
+    type Err = ParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let mut parser = ArgParser::from_strings(split::tab_or_space(s).into_iter());
+        let action_str = parser.must_string("mousebind_action(action)")?;
+
+        let mut action = Self::iter()
+            .find(|i| i.to_string() == action_str)
+            .ok_or(ParseError::InvalidCommand(s.to_string()))?;
+
+        match action.borrow_mut() {
+            MousebindAction::Move | MousebindAction::Resize | MousebindAction::Zoom => (),
+            MousebindAction::Call(command) => {
+                *command = Box::new(parser.collect_command("mousebind_action(command)")?);
+            }
+        }
+
+        Ok(action)
     }
 }
 
@@ -283,32 +345,6 @@ impl ToCommandString for MousebindAction {
     }
 }
 
-#[derive(Debug, Clone, Error)]
-pub enum KeyParseError {
-    #[error("value too short (expected >= 2 parts, got {0} parts)")]
-    TooShort(usize),
-    #[error("no keys in keybind")]
-    NoKeys,
-    #[error("command parse error: {0}")]
-    CommandParseError(String),
-    #[error("expected char key, got: [{0}]")]
-    ExpectedCharKey(String),
-    #[error("string parse error: [{0}]")]
-    StringParseError(String),
-}
-
-impl From<StringParseError> for KeyParseError {
-    fn from(value: StringParseError) -> Self {
-        KeyParseError::StringParseError(value.to_string())
-    }
-}
-
-impl From<CommandParseError> for KeyParseError {
-    fn from(value: CommandParseError) -> Self {
-        KeyParseError::CommandParseError(value.to_string())
-    }
-}
-
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
 pub struct Keybind {
     pub keys: Vec<Key>,
@@ -323,29 +359,13 @@ impl Default for Keybind {
         }
     }
 }
+impl FromStrings for Keybind {
+    fn from_strings<I: Iterator<Item = String>>(s: I) -> Result<Self, ParseError> {
+        let mut parser = ArgParser::from_strings(s);
 
-impl FromStr for Keybind {
-    type Err = KeyParseError;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let parts = s.split('\t').collect::<Vec<&str>>();
-        if parts.len() < 2 {
-            return Err(KeyParseError::TooShort(s.len()));
-        }
-        let mut parts = parts.into_iter();
-
-        let keys = parts
-            .next()
-            .unwrap()
-            .split("+")
-            .map(|key| Key::from_str(key))
-            .collect::<Result<Vec<Key>, _>>()?;
-
-        let command = parts.next().unwrap();
-        let args: Vec<String> = parts.map(String::from).collect();
         Ok(Self {
-            keys,
+            keys: Key::parse_keybind_keys(&parser.must_string("keybind(keys)")?)?,
-            command: Box::new(HlwmCommand::from_raw_parts(command, args)?),
+            command: Box::new(parser.collect_command("keybind(command)")?),
         })
     }
 }
@@ -383,29 +403,12 @@ pub struct Mousebind {
     pub action: MousebindAction,
 }
 
-impl FromStr for Mousebind {
+impl FromStrings for Mousebind {
-    type Err = StringParseError;
+    fn from_strings<I: Iterator<Item = String>>(s: I) -> Result<Self, ParseError> {
+        let mut parser = ArgParser::from_strings(s);
+        let keys = Key::parse_keybind_keys(&parser.must_string("mousebind(keys)")?)?;
 
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let action: MousebindAction = parser.collect_command("mousebind(action)")?;
-        let mut parts = s.split("\t");
-        let keys = parts
-            .next()
-            .ok_or(StringParseError::UnknownValue)?
-            .split("-")
-            .map(|key| key.parse())
-            .collect::<Result<Vec<Key>, _>>()?;
-        let action = parts.next().ok_or(StringParseError::UnknownValue)?;
-        let mut action = MousebindAction::iter()
-            .into_iter()
-            .find(|act| act.to_string() == action)
-            .ok_or(StringParseError::UnknownValue)?;
-        if let MousebindAction::Call(_) = action {
-            let command = parts.next().ok_or(StringParseError::UnknownValue)?;
-            action = MousebindAction::Call(Box::new(
-                HlwmCommand::from_raw_parts(command, parts.map(String::from).collect())
-                    .map_err(|_| StringParseError::UnknownValue)?,
-            ));
-        }
 
         Ok(Self { keys, action })
     }
@@ -449,16 +452,12 @@ pub enum KeyUnbind {
 }
 
 impl FromStr for KeyUnbind {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
-            "--all" => Ok(Self::All),
+            "--all" | "-F" => Ok(Self::All),
-            _ => Ok(KeyUnbind::Keybind(
+            _ => Ok(Self::Keybind(Key::parse_keybind_keys(s)?)),
-                s.split("-")
-                    .map(|key| key.parse())
-                    .collect::<Result<_, _>>()?,
-            )),
         }
     }
 }
@@ -489,9 +488,10 @@ mod test {
     use serde::{Deserialize, Serialize};
     use strum::IntoEnumIterator;
 
-    use crate::hlwm::command::HlwmCommand;
+    use crate::{hlwm::command::HlwmCommand, logerr::UnwrapLog};
 
     use super::{Key, MousebindAction};
+    use pretty_assertions::assert_eq;
 
     #[derive(Debug, Serialize, Deserialize)]
     pub struct KeyWrapper {
@@ -501,8 +501,8 @@ mod test {
     #[test]
     fn key_serialize_deserialize() {
         for (original_key, wrapper) in Key::iter().map(|key| (key, KeyWrapper { key })) {
-            let wrapper_str = toml::to_string_pretty(&wrapper).unwrap();
+            let wrapper_str = toml::to_string_pretty(&wrapper).expect_log("serialize");
-            let parsed: KeyWrapper = toml::from_str(&wrapper_str).unwrap();
+            let parsed: KeyWrapper = toml::from_str(&wrapper_str).expect_log("deserialize");
             assert_eq!(original_key, parsed.key);
         }
     }
@@ -527,4 +527,60 @@ mod test {
             assert_eq!(original_key, parsed.action);
         }
     }
+
+    fn case_random<S: Into<String>>(s: S) -> Vec<String> {
+        let s = s.into();
+        let counts: usize = s.chars().map(|c| c.is_ascii_alphabetic() as usize).sum();
+        if counts < 2 {
+            return vec![s.to_string()];
+        }
+        let mut variants: Vec<String> = Vec::with_capacity(counts + 2);
+        let mut working = Vec::new();
+
+        for idx in 0..counts {
+            let mut c_idx = 0;
+            for c in s.chars() {
+                if !c.is_ascii_alphabetic() {
+                    working.push(c);
+                    continue;
+                }
+                if c_idx == idx {
+                    working.push(c.to_ascii_uppercase());
+                } else {
+                    working.push(c.to_ascii_lowercase());
+                }
+                c_idx += 1;
+            }
+
+            variants.push(working.into_iter().collect());
+            working = Vec::new();
+        }
+        variants.push(s.to_ascii_lowercase());
+        variants.push(s.to_ascii_uppercase());
+
+        variants
+    }
+
+    #[test]
+    fn key_from_case_insensitive() {
+        let test_cases = Key::iter().map(|key| {
+            let strings = key
+                .aliases()
+                .into_iter()
+                .map(|a| case_random(a))
+                .flatten()
+                .collect::<Vec<_>>();
+            (strings, key)
+        });
+
+        for (strings, key) in test_cases {
+            for case in strings {
+                println!("comparing str: [{case}] with key: [{key}]");
+                let parsed = Key::from_str_case_insensitive(&case).expect(&format!(
+                    "key [{key}] parse from insensitive string [{case}] failed"
+                ));
+                assert_eq!(key, parsed);
+            }
+        }
+    }
 }

src/hlwm/macros.rs

@@ -1,153 +0,0 @@
-#[macro_export]
-macro_rules! gen_parse {
-    ($command:ident, $args:ident) => {
-        macro_rules! parse {
-			(And_Or => $$val:tt) => {
-				{
-					let mut args = $args.into_iter();
-                	let separator: Separator = args
-                	    .next()
-                	    .ok_or(CommandParseError::MissingArgument)?
-                	    .parse()?;
-                	let sep_str = separator.to_string();
-                	let args = args.collect::<Vec<_>>();
-                	let commands = args
-                	    .split(|itm| itm.eq(&sep_str))
-                	    .map(|itm| match itm.len() {
-                	        0 => Err(CommandParseError::MissingArgument),
-                	        1 => HlwmCommand::from_str(itm.first().unwrap()),
-                	        _ => {
-                	            let mut args = itm.into_iter();
-                	            HlwmCommand::from_raw_parts(
-                	                &args.next().unwrap(),
-                	                args.map(|i| i.to_owned()).collect(),
-                	            )
-                	        }
-                	    })
-                	    .collect::<Result<Vec<_>, _>>()?;
-					Ok(Self::$$val{
-						separator,
-						commands,
-					})}
-			};
-			($$arg_type:tt => $$val:tt) => {
-				{
-					#[allow(unused_mut)]
-					let mut args_iter = $args.into_iter();
-					Ok(Self::$$val(parse!(Argument args_iter: $$arg_type)))
-				}
-			};
-            ($$($$arg:tt: $$arg_type:tt),+ => $val:tt) => {
-				{
-					#[allow(unused_mut)]
-					let mut args_iter = $args.into_iter();
-					Ok(Self::$val{
-						$$(
-							$$arg: parse!(Argument args_iter: $$arg_type)
-						),+
-					})
-				}
-			};
-			(Argument $$args:ident: [Flag($pat:literal)]) => {
-				{
-					let mut args = $$args.clone().into_iter();
-                	let mut flag = false;
-                	if args.any(|i| i == $pat) {
-                	    $$args = $$args.filter(|a| a != $pat).collect::<Vec<_>>().into_iter();
-                	    flag = true;
-                	}
-					flag
-				}
-			};
-			(Argument $$args:ident: String) => {
-				$$args
-					.next()
-					.ok_or(CommandParseError::BadArgument {
-						command: $command.to_string(),
-					})?
-			};
-			(Argument $$args:ident: FromStrAll) => {
-				$$args
-					.collect::<Vec<_>>()
-					.join("\t")
-					.parse()
-					.map_err(|_| CommandParseError::BadArgument {
-						command: $command.to_string(),
-					})?
-			};
-			(Argument $$args:ident: FromStr) => {
-				parse!(Argument $$args: String)
-					.parse()
-					.map_err(|_| CommandParseError::BadArgument {
-						command: $command.to_string(),
-					})?
-			};
-			(Argument $$args:ident: [Option<FromStr>]) => {
-				{
-					let args: Vec<_> = $$args.collect();
-					if args.len() == 0 {
-						None
-					} else {
-						let mut args = args.into_iter();
-						Some(parse!(Argument args: String)
-							.parse()
-							.map_err(|_| CommandParseError::BadArgument {
-								command: $command.to_string(),
-							})?)
-					}
-				}
-			};
-			(Argument $$args:ident: [Vec<String>]) => {
-				$$args.collect::<Vec<String>>()
-			};
-			(Argument $$args:ident: Bool) => {
-				match parse!(Argument $$args: String).as_str() {
-					"on" | "true" => true,
-					"off" | "false" => false,
-					_ => return Err(CommandParseError::BadArgument {
-						command: $command.to_string(),
-					}),
-				}
-			};
-			(Argument $$args:ident: [Option<String>]) => {
-				{
-					let args = $$args.collect::<Vec<String>>();
-					if args.len() == 0 {
-						None
-					} else {
-						Some(args.join("\t"))
-					}
-				}
-			};
-			(Argument $$args:ident: [Option<Vec<String>>]) => {
-				{
-					let args = $$args.collect::<Vec<String>>();
-					if args.len() == 0 {
-						None
-					} else {
-						Some(args)
-					}
-				}
-			};
-			(Argument $$args:ident: [Option<FromStr>]) => {
-				{
-					let args = $$args.map(|item| item.parse().map_err(|_| CommandParseError::BadArgument {
-						command: $command.to_string(),
-					})).collect::<Result<Vec<_>, _>>()?;
-					if args.len() == 0 {
-						None
-					} else {
-						Some(args)
-					}
-				}
-			};
-			(Argument $$args:ident: [Vec<FromStr>]) => {
-				{
-					$$args.map(|item| item.parse().map_err(|_| CommandParseError::BadArgument {
-						command: $command.to_string(),
-					})).collect::<Result<Vec<_>, _>>()?
-				}
-			};
-		}
-    };
-}

src/hlwm/mod.rs

@@ -1,26 +1,25 @@
 use std::{
     convert::Infallible,
     fmt::Display,
-    num::{ParseFloatError, ParseIntError},
     process::{self, Stdio},
     str::FromStr,
 };
 
-use log::{debug, error};
+use log::{error, trace};
 use serde::{Deserialize, Serialize};
 use strum::IntoEnumIterator;
-use thiserror::Error;
 
 use crate::cmd;
 
 use self::{
-    attribute::{Attribute, AttributeError},
+    attribute::{Attribute, AttributeType},
     command::{CommandError, HlwmCommand},
-    key::KeyParseError,
+    parser::{ArgParser, FromStrings, ParseError},
     setting::{Setting, SettingName},
     tag::TagStatus,
 };
 
+mod and_or_command;
 pub mod attribute;
 pub mod color;
 pub mod command;
@@ -30,13 +29,12 @@ pub mod hook;
 pub mod key;
 mod octal;
 pub mod pad;
+pub mod parser;
 pub mod rule;
 pub mod setting;
 pub mod tag;
 pub mod theme;
 pub mod window;
-#[macro_use]
-mod macros;
 
 pub use hlwmbool::ToggleBool;
 
@@ -58,7 +56,7 @@ impl Client {
     /// Run the command and wait for it to finish.
     pub fn execute(&self, command: HlwmCommand) -> Result<process::Output, CommandError> {
         let args = command.args();
-        debug!("running command: [{}]", (&args).join(" "),);
+        trace!("running command: [{}]", (&args).join(" "),);
         let output = Self::herbstclient()
             .arg("--no-newline")
             .args(args)
@@ -90,29 +88,37 @@ impl Client {
             .ok_or(CommandError::Empty)
     }
 
+    pub fn get_from_str_attr<E: Into<ParseError>, T: FromStr<Err = E>>(
+        &self,
+        attr: String,
+    ) -> Result<T, CommandError> {
+        Ok(self
+            .get_str_attr(attr)?
+            .parse()
+            .map_err(|err: E| err.into())?)
+    }
+
     pub fn get_attr(&self, attr: String) -> Result<Attribute, CommandError> {
-        let attr_type = self
+        let attr_type = AttributeType::get_type(&attr)?;
-            .query(HlwmCommand::AttrType(attr.clone()))?
-            .first()
-            .cloned()
-            .ok_or(CommandError::Empty)?;
         let attr_val = self
             .query(HlwmCommand::GetAttr(attr))?
             .first()
             .cloned()
             .ok_or(CommandError::Empty)?;
-        Ok(Attribute::new(&attr_type, &attr_val)?)
+        Ok(Attribute::new(attr_type, &attr_val)?)
     }
 
     pub fn get_setting(&self, setting: SettingName) -> Result<Setting, CommandError> {
-        Ok(Setting::from_str(&format!(
+        let setting_value = String::from_utf8(self.execute(HlwmCommand::Get(setting))?.stdout)?;
-            "{setting}\t{}",
+        Ok(
-            String::from_utf8(self.execute(HlwmCommand::Get(setting))?.stdout,)?
+            Setting::from_str(&format!("{setting}\t{}", setting_value)).map_err(|err| {
-        ))
+                error!("failed getting setting [{setting}]: {err}");
-        .map_err(|err| {
+                ParseError::InvalidValue {
-            error!("failed getting setting [{setting}]: {err}");
+                    value: setting_value,
-            StringParseError::UnknownValue
+                    expected: "setting value (get_setting)",
-        })?)
+                }
+            })?,
+        )
     }
 
     pub fn query(&self, command: HlwmCommand) -> Result<Vec<String>, CommandError> {
@@ -145,28 +151,6 @@ pub trait ToCommandString {
     fn to_command_string(&self) -> String;
 }
 
-#[derive(Debug, Clone, Error)]
-pub enum StringParseError {
-    #[error("unknown value")]
-    UnknownValue,
-    #[error("failed parsing float: [{0}]")]
-    FloatError(#[from] ParseFloatError),
-    #[error("invalid bool value: [{0}]")]
-    BoolError(String),
-    #[error("failed parsing int: [{0}]")]
-    IntError(#[from] ParseIntError),
-    #[error("command parse error")]
-    CommandParseError(String),
-    #[error("invalid length for part [{1}]: [{0}]")]
-    InvalidLength(usize, &'static str),
-    #[error("required arguments missing: [{0}]")]
-    RequiredArgMissing(String),
-    #[error("attribute error: [{0}]")]
-    AttributeError(#[from] AttributeError),
-    #[error("key parse error: [{0}]")]
-    KeyParseError(#[from] KeyParseError),
-}
-
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub enum TagSelect {
     Index(i32),
@@ -213,22 +197,31 @@ impl<N> FromStr for Index<N>
 where
     N: PartialEq + FromStr,
 {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let mut chars = s.chars();
-        let prefix = chars.next().ok_or(StringParseError::UnknownValue)?;
+        let prefix = chars.next().ok_or(ParseError::Empty)?;
         match prefix {
-            '+' => Ok(Self::Relative(
+            '+' => Ok(Self::Relative({
-                N::from_str(&chars.collect::<String>())
+                let s = chars.collect::<String>();
-                    .map_err(|_| StringParseError::UnknownValue)?,
+                N::from_str(&s).map_err(|_| ParseError::InvalidValue {
-            )),
+                    value: s,
-            '-' => Ok(Self::Relative(
+                    expected: std::any::type_name::<N>(),
-                N::from_str(s).map_err(|_| StringParseError::UnknownValue)?,
+                })?
-            )),
+            })),
-            _ => Ok(Self::Absolute(
+            '-' => Ok(Self::Relative(N::from_str(s).map_err(|_| {
-                N::from_str(s).map_err(|_| StringParseError::UnknownValue)?,
+                ParseError::InvalidValue {
-            )),
+                    value: s.to_string(),
+                    expected: std::any::type_name::<N>(),
+                }
+            })?)),
+            _ => Ok(Self::Absolute(N::from_str(s).map_err(|_| {
+                ParseError::InvalidValue {
+                    value: s.to_string(),
+                    expected: std::any::type_name::<N>(),
+                }
+            })?)),
         }
     }
 }
@@ -260,20 +253,20 @@ where
     }
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize, strum::EnumIter, PartialEq)]
+#[derive(Debug, Clone, Copy, Serialize, Deserialize, strum::EnumIter, PartialEq)]
 pub enum Separator {
     Comma,
     Period,
 }
 
 impl FromStr for Separator {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         Self::iter()
             .into_iter()
             .find(|i| i.to_string() == s)
-            .ok_or(StringParseError::UnknownValue)
+            .ok_or(ParseError::InvalidCommand(s.to_string()))
     }
 }
 
@@ -309,13 +302,13 @@ pub enum Operator {
 }
 
 impl FromStr for Operator {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         Self::iter()
             .into_iter()
             .find(|i| i.to_string() == s)
-            .ok_or(StringParseError::UnknownValue)
+            .ok_or(ParseError::InvalidCommand(s.to_string()))
     }
 }
 
@@ -348,13 +341,13 @@ pub enum Direction {
 }
 
 impl FromStr for Direction {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         Self::iter()
             .into_iter()
             .find(|dir| dir.to_string() == s)
-            .ok_or(StringParseError::UnknownValue)
+            .ok_or(ParseError::InvalidCommand(s.to_string()))
     }
 }
 
@@ -375,25 +368,20 @@ pub enum Align {
     Auto,
 }
 
-impl FromStr for Align {
+impl FromStrings for Align {
-    type Err = StringParseError;
+    fn from_strings<I: Iterator<Item = String>>(s: I) -> Result<Self, ParseError> {
+        let mut args = ArgParser::from_strings(s);
+        let alignment = args.must_string("align(align)")?;
+        let fraction = args.optional_next_from_str("align(fraction)")?;
 
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match alignment.as_str() {
-        let mut parts = s.split("\t");
-        let align = parts.next().ok_or(StringParseError::UnknownValue)?;
-        let fraction = match parts.next().map(|f| f64::from_str(f)) {
-            Some(val) => Some(val?),
-            None => None,
-        };
-
-        match align {
             "bottom" | "vertical" | "vert" | "v" => Ok(Self::Bottom(fraction)),
             "left" => Ok(Self::Left(fraction)),
             "right" | "horizontal" | "horiz" | "h" => Ok(Self::Right(fraction)),
             "top" => Ok(Self::Top(fraction)),
             "explode" => Ok(Self::Explode),
             "auto" => Ok(Self::Auto),
-            _ => Err(StringParseError::UnknownValue),
+            _ => Err(ParseError::InvalidCommand(alignment)),
         }
     }
 }

src/hlwm/pad.rs

@@ -2,7 +2,7 @@ use std::{fmt::Display, str::FromStr};
 
 use crate::split;
 
-use super::StringParseError;
+use super::parser::{FromStrings, ParseError};
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum Pad {
@@ -13,13 +13,11 @@ pub enum Pad {
     UpRightDownLeft(u32, u32, u32, u32),
 }
 
-impl FromStr for Pad {
+impl FromStrings for Pad {
-    type Err = StringParseError;
+    fn from_strings<I: Iterator<Item = String>>(s: I) -> Result<Self, ParseError> {
-
+        let parts = s.collect::<Vec<_>>();
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let parts = split::tab_or_space(s);
         match parts.len() {
-            0 => Err(StringParseError::InvalidLength(0, "pad")),
+            0 => Err(ParseError::Empty),
             1 => Ok(Pad::Up(parts[0].parse()?)),
             2 => Ok(Pad::UpRight(parts[0].parse()?, parts[1].parse()?)),
             3 => Ok(Pad::UpRightDown(
@@ -37,6 +35,15 @@ impl FromStr for Pad {
     }
 }
 
+impl FromStr for Pad {
+    type Err = ParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let parts = split::tab_or_space(s);
+        Self::from_strings(parts.into_iter())
+    }
+}
+
 impl Display for Pad {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {

src/hlwm/parser/either.rs

@@ -0,0 +1,4 @@
+pub enum Either<L, R> {
+    Left(L),
+    Right(R),
+}

src/hlwm/parser/mod.rs

@@ -0,0 +1,234 @@
+use std::{
+    convert::Infallible,
+    num::{ParseFloatError, ParseIntError},
+    str::{FromStr, ParseBoolError},
+    string::FromUtf8Error,
+};
+
+use log::error;
+use thiserror::Error;
+
+use self::either::Either;
+
+use super::hex::HexError;
+
+pub mod either;
+mod traits;
+pub use traits::*;
+
+#[derive(Debug, Clone, Error, strum::EnumIs)]
+pub enum ParseError {
+    #[error("no more items left in parser")]
+    Empty,
+    #[error("missing expected value")]
+    ValueMissing,
+    #[error("invalid command: [{0}]")]
+    InvalidCommand(String),
+    #[error("invalid value [{value}], expected [{expected}]")]
+    InvalidValue {
+        value: String,
+        expected: &'static str,
+    },
+    #[error("hex decoding error: [{0}]")]
+    HexError(#[from] HexError),
+    #[error("primitive type parsing error: [{0}]")]
+    PrimitiveError(String),
+}
+
+impl From<Infallible> for ParseError {
+    fn from(_: Infallible) -> Self {
+        // nonsense
+        unreachable!()
+    }
+}
+
+macro_rules! from_primitive {
+	($($err:ty,)*) => {
+		$(
+			impl From<$err> for ParseError {
+				fn from(value: $err) -> Self {
+					ParseError::PrimitiveError(value.to_string())
+				}
+			}
+		)*
+	};
+}
+from_primitive!(
+    ParseBoolError,
+    ParseIntError,
+    FromUtf8Error,
+    ParseFloatError,
+);
+
+impl From<strum::ParseError> for ParseError {
+    fn from(value: strum::ParseError) -> Self {
+        ParseError::InvalidCommand(value.to_string())
+    }
+}
+
+pub struct ArgParser<I, T>
+where
+    I: Iterator<Item = T>,
+{
+    inner: I,
+}
+
+impl<I, T> Clone for ArgParser<I, T>
+where
+    I: Iterator<Item = T> + Clone,
+{
+    fn clone(&self) -> Self {
+        Self {
+            inner: self.inner.clone(),
+        }
+    }
+}
+
+impl<I> ArgParser<I, String>
+where
+    I: Iterator<Item = String>,
+{
+    pub fn from_strings(src: I) -> Self {
+        Self { inner: src }
+    }
+
+    #[inline(always)]
+    pub fn next_string(&mut self) -> Option<String> {
+        self.inner.next()
+    }
+
+    #[inline(always)]
+    pub fn inner(self) -> I {
+        self.inner
+    }
+
+    /// Gets the next string, then tries the `try_this` function on it.
+    /// If that function returns [Err], it runs the `then_this` function
+    /// and returns that result.
+    pub fn try_first<F1, F2, T1, T2, E>(
+        &mut self,
+        try_this: F1,
+        then_this: F2,
+        on_error: &str,
+    ) -> Result<Either<T1, T2>, ParseError>
+    where
+        F1: FnOnce(&String) -> Result<T1, E>,
+        F2: FnOnce(&String) -> Result<T2, E>,
+        E: Into<ParseError>,
+    {
+        let next = self.must_string(on_error)?;
+        if let Ok(t1) = try_this(&next) {
+            return Ok(Either::Left(t1));
+        }
+        match then_this(&next) {
+            Ok(t2) => Ok(Either::Right(t2)),
+            Err(err) => {
+                let err = err.into();
+                error!("{on_error}: {err}");
+                Err(err)
+            }
+        }
+    }
+
+    /// Get the next string, or [ParseError::Empty]
+    #[inline(always)]
+    pub fn must_string(&mut self, on_error: &str) -> Result<String, ParseError> {
+        self.inner
+            .next()
+            .ok_or(ParseError::Empty)
+            .inspect_err(|err| error!("{on_error}: {err}"))
+    }
+
+    pub fn next_from_str<E: Into<ParseError>, T: FromStr<Err = E>>(
+        &mut self,
+        on_error: &str,
+    ) -> Result<T, ParseError> {
+        Ok(self.must_string(on_error)?.parse().map_err(|err: E| {
+            let err = err.into();
+            error!("{on_error}: {err}");
+            err
+        })?)
+    }
+
+    /// Consumes the remainder of the iterator and uses the [FromCommandArgs]
+    /// trait to parse it into the desired type
+    pub fn collect_command<C: FromCommandArgs>(mut self, on_error: &str) -> Result<C, ParseError> {
+        C::from_command_args(&self.must_string(on_error)?, self.inner)
+    }
+
+    /// Consumes the remainder of the iterator and uses the [FromStrings]
+    /// trait to parse it into the desired type
+    pub fn collect_from_strings<T: FromStrings>(self, on_error: &str) -> Result<T, ParseError> {
+        T::from_strings(self.inner).inspect_err(|err| {
+            error!("{on_error}: {err}");
+        })
+    }
+
+    /// Consumes the remainder of the iterator and uses the [FromStringsHint]
+    /// trait to parse it into the desired type.
+    ///
+    /// Takes a `hint` that may aid in the parsing of that string
+    pub fn collect_from_strings_hint<H, T: FromStringsHint<H>>(
+        self,
+        hint: H,
+        on_error: &str,
+    ) -> Result<T, ParseError> {
+        T::from_strings_hint(self.inner, hint).inspect_err(|err| {
+            error!("{on_error}: {err}");
+        })
+    }
+
+    #[inline(always)]
+    pub fn collect<B: FromIterator<String>>(self) -> B {
+        self.inner.collect()
+    }
+
+    /// Get the next [String] from the iterator and tries to parse it with [FromStr]
+    /// if there's any remaining items. Otherwise just returns [None]
+    pub fn optional_next_from_str<E: Into<ParseError>, T: FromStr<Err = E>>(
+        &mut self,
+        on_error: &str,
+    ) -> Result<Option<T>, ParseError> {
+        match self.next_string() {
+            Some(next) => Ok(Some(next.parse().map_err(|err: E| {
+                let err = err.into();
+                error!("{on_error}: {err}");
+                err
+            })?)),
+            None => Ok(None),
+        }
+    }
+
+    /// Consumes the iterator and takes every remaining element, parses them with
+    /// [FromStr] and returns them in a [Vec]
+    pub fn collect_from_str<E: Into<ParseError>, T: FromStr<Err = E>>(
+        self,
+        on_error: &str,
+    ) -> Result<Vec<T>, ParseError> {
+        self.inner
+            .map(|item| T::from_str(&item))
+            .collect::<Result<Vec<_>, _>>()
+            .map_err(|err| {
+                let err = err.into();
+                error!("{on_error}: {err}");
+                err
+            })
+    }
+
+    /// Consumes the iterator and takes every remaining element, checks if any match
+    /// `flag`, and returns the strings in the iterator that don't match `flag`,
+    /// as well as a boolean on whether `flag` was found.
+    pub fn collect_strings_with_flag(self, flag: &str) -> (Vec<String>, bool) {
+        let mut out = Vec::new();
+        let mut found = false;
+        for item in self.inner {
+            if item == flag {
+                found = true;
+                continue;
+            }
+            out.push(item);
+        }
+
+        (out, found)
+    }
+}

src/hlwm/parser/traits.rs

@@ -0,0 +1,67 @@
+use super::ParseError;
+
+pub trait FromStrings: Sized {
+    fn from_strings<I: Iterator<Item = String>>(s: I) -> Result<Self, ParseError>;
+}
+
+/// Same as [FromStrings] but can pass a hint to the function which can help
+/// in some determinations (e.g. passing in the path of the attribute for
+/// parsing a [crate::hlwm::Attribute] so it can use the path to determine the
+/// type of the attribute)
+pub trait FromStringsHint<Hint>: Sized {
+    fn from_strings_hint<I: Iterator<Item = String>>(s: I, hint: Hint) -> Result<Self, ParseError>;
+}
+
+pub trait FromCommandArgs: Sized {
+    fn from_command_args<S: Into<String>, I: Iterator<Item = S>>(
+        command: &str,
+        args: I,
+    ) -> Result<Self, ParseError>;
+}
+
+pub trait ToOption: Sized {
+    fn to_option(self) -> Option<Self>;
+}
+
+impl<T> ToOption for Vec<T> {
+    fn to_option(self) -> Option<Self> {
+        match self.is_empty() {
+            false => Some(self),
+            true => None,
+        }
+    }
+}
+
+impl ToOption for &str {
+    fn to_option(self) -> Option<Self> {
+        match self.is_empty() {
+            false => Some(self),
+            true => None,
+        }
+    }
+}
+
+impl ToOption for String {
+    fn to_option(self) -> Option<Self> {
+        match self.is_empty() {
+            false => Some(self),
+            true => None,
+        }
+    }
+}
+
+pub trait Flip {
+    type Target;
+    fn flip(self) -> Self::Target;
+}
+
+impl<T, E> Flip for Option<Result<T, E>> {
+    type Target = Result<Option<T>, E>;
+
+    fn flip(self) -> Self::Target {
+        match self {
+            Some(r) => r.map(Some),
+            None => Self::Target::Ok(None),
+        }
+    }
+}

src/hlwm/rule.rs

@@ -10,7 +10,12 @@ use strum::IntoEnumIterator;
 
 use crate::split;
 
-use super::{hlwmbool, hook::Hook, StringParseError, ToCommandString};
+use super::{
+    hlwmbool,
+    hook::Hook,
+    parser::{FromCommandArgs, FromStrings, ParseError},
+    ToCommandString,
+};
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct Rule {
@@ -78,23 +83,17 @@ impl Rule {
     }
 }
 
-impl FromStr for Rule {
+impl FromStrings for Rule {
-    type Err = StringParseError;
+    fn from_strings<I: Iterator<Item = String>>(s: I) -> Result<Self, ParseError> {
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let parts = split::tab_or_space(s);
-        if parts.is_empty() {
-            return Err(StringParseError::InvalidLength(0, "parse rule"));
-        }
         let mut condition: Option<Condition> = None;
         let mut consequences = vec![];
         let mut label: Option<String> = None;
         let mut flag: Option<Flag> = None;
-        let mut args = parts
+        let mut args = s.map(|part| part.strip_prefix("--").unwrap_or(part.as_str()).to_string());
-            .into_iter()
-            .map(|part| part.strip_prefix("--").unwrap_or(part.as_str()).to_string());
 
+        let mut original = Vec::new(); // For InvalidValue error
         while let Some(arg) = args.next() {
+            original.push(arg.clone());
             if label.is_none() {
                 if let Some((name, value)) = arg.split_once('=') {
                     if name.trim() == "label" {
@@ -122,9 +121,10 @@ impl FromStr for Rule {
         }
 
         if consequences.is_empty() {
-            return Err(StringParseError::RequiredArgMissing(
+            return Err(ParseError::InvalidValue {
-                "condition and/or consequences".into(),
+                value: original.join(" "),
-            ));
+                expected: "condition and/or consequences",
+            });
         }
 
         Ok(Self {
@@ -158,8 +158,9 @@ impl<'de> Deserialize<'de> for Rule {
         }
 
         let str_val: String = Deserialize::deserialize(deserializer)?;
+        let strings = split::tab_or_space(&str_val);
 
-        Ok(Self::from_str(&str_val).map_err(|_| {
+        Ok(Self::from_strings(strings.into_iter()).map_err(|_| {
             serde::de::Error::invalid_value(serde::de::Unexpected::Str(&str_val), &Expect)
         })?)
     }
@@ -187,12 +188,12 @@ impl RuleOperator {
 }
 
 impl TryFrom<char> for RuleOperator {
-    type Error = StringParseError;
+    type Error = ParseError;
 
     fn try_from(value: char) -> Result<Self, Self::Error> {
         Self::iter()
             .find(|i| i.char() == value)
-            .ok_or(StringParseError::UnknownValue)
+            .ok_or(ParseError::InvalidCommand(value.to_string()))
     }
 }
 
@@ -209,13 +210,10 @@ impl Display for RuleOperator {
 }
 
 impl FromStr for RuleOperator {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        s.chars()
+        s.chars().next().ok_or(ParseError::Empty)?.try_into()
-            .next()
-            .ok_or(StringParseError::InvalidLength(s.len(), "rule operator"))?
-            .try_into()
     }
 }
 
@@ -294,7 +292,7 @@ impl ToCommandString for Condition {
 }
 
 impl FromStr for Condition {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         // Handle the case for fixedsize first so that we can treat the rest
@@ -307,11 +305,11 @@ impl FromStr for Condition {
         let ((name, match_val), match_char) =
             match split::on_first_match(s, &RuleOperator::match_set()) {
                 Some(parts) => parts,
-                None => return Err(StringParseError::InvalidLength(1, "property")),
+                None => return Err(ParseError::Empty),
             };
         let mut prop = Self::iter()
             .find(|i| i.to_string() == name)
-            .ok_or(StringParseError::UnknownValue)?;
+            .ok_or(ParseError::InvalidCommand(s.to_string()))?;
 
         match prop.borrow_mut() {
             Condition::Instance { operator, value }
@@ -327,7 +325,12 @@ impl FromStr for Condition {
             }
             // Should be handled at the top of the function. If it's here that's not a valid
             // use of fixedsize.
-            Condition::FixedSize => return Err(StringParseError::UnknownValue),
+            Condition::FixedSize => {
+                return Err(ParseError::InvalidValue {
+                    value: s.to_string(),
+                    expected: "[BUG] Should be handled at the top of the function",
+                })
+            }
         };
 
         Ok(prop)
@@ -433,7 +436,7 @@ impl ToCommandString for Consequence {
 }
 
 impl FromStr for Consequence {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let parts = split::tab_or_space(s);
@@ -442,7 +445,7 @@ impl FromStr for Consequence {
         let (name, value_str) = if name.contains('=') {
             let parts = name.split('=').collect::<Vec<_>>();
             if parts.len() != 2 {
-                return Err(StringParseError::UnknownValue);
+                return Err(ParseError::InvalidCommand(s.to_string()));
             }
             let mut parts = parts.into_iter();
             (
@@ -453,10 +456,10 @@ impl FromStr for Consequence {
             match parts.next() {
                 Some(op) => {
                     if op != "=" {
-                        return Err(StringParseError::UnknownValue);
+                        return Err(ParseError::InvalidCommand(s.to_string()));
                     }
                 }
-                None => return Err(StringParseError::UnknownValue),
+                None => return Err(ParseError::InvalidCommand(s.to_string())),
             };
             let value = parts.collect::<Vec<_>>().join("\t");
             (name, value)
@@ -464,7 +467,7 @@ impl FromStr for Consequence {
 
         let mut cons = Self::iter()
             .find(|i| i.to_string() == name)
-            .ok_or(StringParseError::UnknownValue)?;
+            .ok_or(ParseError::InvalidCommand(s.to_string()))?;
         match cons.borrow_mut() {
             Consequence::Focus(value)
             | Consequence::SwitchTag(value)
@@ -483,15 +486,13 @@ impl FromStr for Consequence {
             | Consequence::KeysInactive(value) => *value = value_str,
             Consequence::Index(value) => *value = i32::from_str(&value_str)?,
             Consequence::Hook(value) => {
-                *value = Hook::from_str(&value_str).map_err(|_| StringParseError::UnknownValue)?
+                *value = Hook::from_command_args(&name, [value_str].into_iter())?;
             }
             Consequence::FloatPlacement(value) => *value = FloatPlacement::from_str(&value_str)?,
             Consequence::FloatingGeometry { x, y } => {
                 let mut values = value_str.split('=');
-                *x = u32::from_str(values.next().ok_or(StringParseError::UnknownValue)?)
+                *x = u32::from_str(values.next().ok_or(ParseError::ValueMissing)?)?;
-                    .map_err(|_| StringParseError::UnknownValue)?;
+                *y = u32::from_str(values.next().ok_or(ParseError::ValueMissing)?)?;
-                *y = u32::from_str(values.next().ok_or(StringParseError::UnknownValue)?)
-                    .map_err(|_| StringParseError::UnknownValue)?;
             }
         };
 
@@ -513,12 +514,12 @@ pub enum FloatPlacement {
 }
 
 impl FromStr for FloatPlacement {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         Self::iter()
             .find(|i| i.to_string() == s)
-            .ok_or(StringParseError::UnknownValue)
+            .ok_or(ParseError::InvalidCommand(s.to_string()))
     }
 }
 
@@ -553,7 +554,7 @@ impl Display for Flag {
 }
 
 impl FromStr for Flag {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
@@ -561,7 +562,7 @@ impl FromStr for Flag {
             "once" => Ok(Self::Once),
             "printlabel" => Ok(Self::PrintLabel),
             "prepend" => Ok(Self::Prepend),
-            _ => Err(StringParseError::UnknownValue),
+            _ => Err(ParseError::InvalidCommand(s.to_string())),
         }
     }
 }
@@ -649,7 +650,10 @@ mod test {
 
     use serde::{Deserialize, Serialize};
 
-    use crate::hlwm::rule::FloatPlacement;
+    use crate::{
+        hlwm::{parser::FromStrings, rule::FloatPlacement},
+        split,
+    };
     use pretty_assertions::assert_eq;
 
     use super::{Condition, Consequence, Flag, Rule, RuleOperator};
@@ -697,7 +701,7 @@ label=5	fixedsize=0	floating=true"#;
 
         let parsed = INPUT
             .split('\n')
-            .map(|l| Rule::from_str(l))
+            .map(|l| Rule::from_strings(split::tab_or_space(l).into_iter()))
             .collect::<Result<Vec<_>, _>>()
             .expect("parsing error");
 

src/hlwm/setting.rs

@@ -1,12 +1,20 @@
-use std::str::FromStr;
+use std::{borrow::BorrowMut, str::FromStr};
 
-use log::debug;
+use log::trace;
-use serde::{Deserialize, Serialize};
+use serde::{de::Expected, Deserialize, Serialize};
 
-use crate::{gen_parse, hlwm::command::CommandParseError, split};
+use crate::{
+    hlwm::{command::CommandParseError, parser::ArgParser},
+    split,
+};
 use strum::IntoEnumIterator;
 
-use super::{color::Color, hlwmbool::ToggleBool, StringParseError, ToCommandString};
+use super::{
+    color::Color,
+    hlwmbool::ToggleBool,
+    parser::{FromCommandArgs, ParseError},
+    ToCommandString,
+};
 
 #[derive(Debug, Clone, strum::Display, strum::EnumIter, PartialEq, strum::EnumDiscriminants)]
 #[strum_discriminants(
@@ -201,12 +209,12 @@ impl Default for SettingName {
 }
 
 impl FromStr for SettingName {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         Self::iter()
             .find(|i| i.to_string() == s)
-            .ok_or(StringParseError::UnknownValue)
+            .ok_or(ParseError::InvalidCommand(s.to_string()))
     }
 }
 
@@ -224,6 +232,18 @@ impl<'de> Deserialize<'de> for Setting {
     where
         D: serde::Deserializer<'de>,
     {
+        pub enum Expect {
+            Command(String),
+        }
+        impl Expected for Expect {
+            fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+                match self {
+                    Expect::Command(err) => {
+                        write!(f, "a valid [command][space/tab][value] string: {err}")
+                    }
+                }
+            }
+        }
         let str_val: String = Deserialize::deserialize(deserializer)?;
         let ((command, arg), _) = split::on_first_match(&str_val, &['='])
             .ok_or(CommandParseError::InvalidArgumentCount(0, "setting".into()))
@@ -231,90 +251,92 @@ impl<'de> Deserialize<'de> for Setting {
                 serde::de::Error::invalid_value(serde::de::Unexpected::Str(&str_val), &err)
             })?;
         Ok(
-            Self::from_raw_parts(&command.trim(), &arg.trim()).map_err(|err| {
+            Self::from_command_args(&command.trim(), [arg.trim()].into_iter()).map_err(|err| {
-                serde::de::Error::invalid_value(serde::de::Unexpected::Str(&str_val), &err)
+                serde::de::Error::invalid_value(
+                    serde::de::Unexpected::Str(&str_val),
+                    &Expect::Command(err.to_string()),
+                )
             })?,
         )
     }
 }
 
 impl FromStr for Setting {
-    type Err = CommandParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        debug!("beginning to parse [{s}] as setting");
+        trace!("[from_str] beginning to parse [{s}] as setting");
-        let ((command, arg), _) = split::on_first_match(s, &['\t', ' '])
+        let ((command, arg), _) =
-            .ok_or(CommandParseError::InvalidArgumentCount(0, "setting".into()))?;
+            split::on_first_match(s, &['\t', ' ']).ok_or(ParseError::InvalidValue {
+                value: s.to_string(),
+                expected: "[setting] [value]",
+            })?;
 
-        Self::from_raw_parts(&command.trim(), &arg.trim())
+        Self::from_command_args(&command.trim(), [arg].into_iter())
     }
 }
 
-impl Setting {
+impl FromCommandArgs for Setting {
-    pub fn from_raw_parts(command: &str, arg: &str) -> Result<Self, CommandParseError> {
+    fn from_command_args<S: Into<String>, I: Iterator<Item = S>>(
-        let command = Self::iter()
+        cmd: &str,
-            .find(|cmd| cmd.to_string() == command)
+        args: I,
-            .ok_or(CommandParseError::UnknownCommand(command.to_string()))?;
+    ) -> Result<Self, ParseError> {
+        let mut command = Self::iter()
+            .find(|s| s.to_string() == cmd)
+            .ok_or(ParseError::InvalidCommand(cmd.to_string()))?;
 
-        let args = [arg.to_string()];
+        let mut parser = ArgParser::from_strings(args.map(|i| i.into()));
-        gen_parse!(command, args);
 
-        match command {
+        match command.borrow_mut() {
-            Setting::Verbose(_) => parse!(FromStr => Verbose),
+            Setting::Verbose(arg)
-            Setting::TabbedMax(_) => parse!(FromStr => TabbedMax),
+            | Setting::TabbedMax(arg)
-            Setting::GaplessGrid(_) => parse!(FromStr => GaplessGrid),
+            | Setting::GaplessGrid(arg)
-            Setting::RaiseOnClick(_) => parse!(FromStr => RaiseOnClick),
+            | Setting::RaiseOnClick(arg)
-            Setting::RaiseOnFocus(_) => parse!(FromStr => RaiseOnFocus),
+            | Setting::RaiseOnFocus(arg)
-            Setting::AutoDetectPanels(_) => parse!(FromStr => AutoDetectPanels),
+            | Setting::AutoDetectPanels(arg)
-            Setting::FocusFollowsMouse(_) => parse!(FromStr => FocusFollowsMouse),
+            | Setting::FocusFollowsMouse(arg)
-            Setting::HideCoveredWindows(_) => parse!(FromStr => HideCoveredWindows),
+            | Setting::HideCoveredWindows(arg)
-            Setting::AutoDetectMonitors(_) => parse!(FromStr => AutoDetectMonitors),
+            | Setting::AutoDetectMonitors(arg)
-            Setting::FrameBgTransparent(_) => parse!(FromStr => FrameBgTransparent),
+            | Setting::FrameBgTransparent(arg)
-            Setting::SwapMonitorsToGetTag(_) => parse!(FromStr => SwapMonitorsToGetTag),
+            | Setting::SwapMonitorsToGetTag(arg)
-            Setting::UpdateDraggedClients(_) => parse!(FromStr => UpdateDraggedClients),
+            | Setting::UpdateDraggedClients(arg)
-            Setting::FocusStealingPrevention(_) => parse!(FromStr => FocusStealingPrevention),
+            | Setting::FocusStealingPrevention(arg)
-            Setting::RaiseOnFocusTemporarily(_) => parse!(FromStr => RaiseOnFocusTemporarily),
+            | Setting::RaiseOnFocusTemporarily(arg)
-            Setting::SmartWindowSurroundings(_) => parse!(FromStr => SmartWindowSurroundings),
+            | Setting::SmartWindowSurroundings(arg)
-            Setting::DefaultDirectionExternalOnly(_) => {
+            | Setting::DefaultDirectionExternalOnly(arg)
-                parse!(FromStr => DefaultDirectionExternalOnly)
+            | Setting::FocusCrossesMonitorBoundaries(arg) => *arg = parser.next_from_str(cmd)?,
+            Setting::FrameBgActiveColor(arg)
+            | Setting::FrameBgNormalColor(arg)
+            | Setting::FrameBorderInnerColor(arg)
+            | Setting::FrameBorderActiveColor(arg)
+            | Setting::FrameBorderNormalColor(arg)
+            | Setting::WindowBorderInnerColor(arg)
+            | Setting::WindowBorderActiveColor(arg)
+            | Setting::WindowBorderNormalColor(arg)
+            | Setting::WindowBorderUrgentColor(arg) => *arg = parser.next_from_str(cmd)?,
+            Setting::DefaultFrameLayout(arg) => *arg = parser.next_from_str(cmd)?,
+            Setting::ShowFrameDecorations(arg) => *arg = parser.next_from_str(cmd)?,
+            Setting::SmartFrameSurroundings(arg) => *arg = parser.next_from_str(cmd)?,
+            Setting::MonitorsLocked(arg)
+            | Setting::FrameActiveOpacity(arg)
+            | Setting::FrameNormalOpacity(arg) => *arg = parser.next_from_str(cmd)?,
+            Setting::SnapGap(arg)
+            | Setting::FrameGap(arg)
+            | Setting::WindowGap(arg)
+            | Setting::SnapDistance(arg)
+            | Setting::FramePadding(arg)
+            | Setting::MouseRecenterGap(arg)
+            | Setting::FrameBorderWidth(arg)
+            | Setting::WindowBorderWidth(arg)
+            | Setting::FrameTransparentWidth(arg)
+            | Setting::FrameBorderInnerWidth(arg)
+            | Setting::WindowBorderInnerWidth(arg)
+            | Setting::PseudotileCenterThreshold(arg) => *arg = parser.next_from_str(cmd)?,
+            Setting::Wmname(arg) | Setting::Ellipsis(arg) | Setting::TreeStyle(arg) => {
+                *arg = parser.must_string(cmd)?
             }
-            Setting::FocusCrossesMonitorBoundaries(_) => {
-                parse!(FromStr => FocusCrossesMonitorBoundaries)
-            }
-
-            Setting::FrameBgActiveColor(_) => parse!(FromStr => FrameBgActiveColor),
-            Setting::FrameBgNormalColor(_) => parse!(FromStr => FrameBgNormalColor),
-            Setting::DefaultFrameLayout(_) => parse!(FromStr => DefaultFrameLayout),
-            Setting::ShowFrameDecorations(_) => parse!(FromStr => ShowFrameDecorations),
-            Setting::FrameBorderInnerColor(_) => parse!(FromStr => FrameBorderInnerColor),
-            Setting::FrameBorderActiveColor(_) => parse!(FromStr => FrameBorderActiveColor),
-            Setting::FrameBorderNormalColor(_) => parse!(FromStr => FrameBorderNormalColor),
-            Setting::SmartFrameSurroundings(_) => parse!(FromStr => SmartFrameSurroundings),
-            Setting::WindowBorderInnerColor(_) => parse!(FromStr => WindowBorderInnerColor),
-            Setting::WindowBorderActiveColor(_) => parse!(FromStr => WindowBorderActiveColor),
-            Setting::WindowBorderNormalColor(_) => parse!(FromStr => WindowBorderNormalColor),
-            Setting::WindowBorderUrgentColor(_) => parse!(FromStr => WindowBorderUrgentColor),
-
-            Setting::SnapGap(_) => parse!(FromStr => SnapGap),
-            Setting::FrameGap(_) => parse!(FromStr => FrameGap),
-            Setting::WindowGap(_) => parse!(FromStr => WindowGap),
-            Setting::SnapDistance(_) => parse!(FromStr => SnapDistance),
-            Setting::FramePadding(_) => parse!(FromStr => FramePadding),
-            Setting::MonitorsLocked(_) => parse!(FromStr => MonitorsLocked),
-            Setting::MouseRecenterGap(_) => parse!(FromStr => MouseRecenterGap),
-            Setting::FrameBorderWidth(_) => parse!(FromStr => FrameBorderWidth),
-            Setting::WindowBorderWidth(_) => parse!(FromStr => WindowBorderWidth),
-            Setting::FrameActiveOpacity(_) => parse!(FromStr => FrameActiveOpacity),
-            Setting::FrameNormalOpacity(_) => parse!(FromStr => FrameNormalOpacity),
-            Setting::FrameTransparentWidth(_) => parse!(FromStr => FrameTransparentWidth),
-            Setting::FrameBorderInnerWidth(_) => parse!(FromStr => FrameBorderInnerWidth),
-            Setting::WindowBorderInnerWidth(_) => parse!(FromStr => WindowBorderInnerWidth),
-            Setting::PseudotileCenterThreshold(_) => parse!(FromStr => PseudotileCenterThreshold),
-
-            Setting::Wmname(_) => parse!(String => Wmname),
-            Setting::Ellipsis(_) => parse!(String => Ellipsis),
-            Setting::TreeStyle(_) => parse!(String => TreeStyle),
         }
+        Ok(command)
     }
 }
 

src/hlwm/tag.rs

@@ -6,7 +6,7 @@ use std::{
 use serde::{Deserialize, Serialize};
 use strum::IntoEnumIterator;
 
-use super::StringParseError;
+use super::parser::ParseError;
 
 pub struct TagStatus {
     name: String,
@@ -25,13 +25,13 @@ impl TagStatus {
 }
 
 impl FromStr for TagStatus {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let mut parts = s.chars();
         let state = parts
             .next()
-            .ok_or(StringParseError::UnknownValue)?
+            .ok_or(ParseError::InvalidCommand(s.to_string()))?
             .try_into()?;
         let name = parts.collect();
 
@@ -56,24 +56,24 @@ pub enum TagState {
 }
 
 impl TryFrom<char> for TagState {
-    type Error = StringParseError;
+    type Error = ParseError;
 
     fn try_from(value: char) -> Result<Self, Self::Error> {
         Self::iter()
             .into_iter()
             .find(|i| char::from(i) == value)
-            .ok_or(StringParseError::UnknownValue)
+            .ok_or(ParseError::InvalidCommand(value.to_string()))
     }
 }
 
 impl FromStr for TagState {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         Self::iter()
             .into_iter()
             .find(|i| i.to_string() == s)
-            .ok_or(StringParseError::UnknownValue)
+            .ok_or(ParseError::InvalidCommand(s.to_string()))
     }
 }
 

src/hlwm/theme.rs

@@ -1,36 +1,23 @@
-use std::{borrow::BorrowMut, num::ParseIntError};
+use std::borrow::BorrowMut;
 
 use serde::{de::Expected, Deserialize, Serialize};
 use strum::IntoEnumIterator;
-use thiserror::Error;
 
 use crate::split;
 
 use super::{
     attribute::Attribute,
-    color::{self, Color},
+    color::Color,
-    StringParseError,
+    parser::{ArgParser, FromCommandArgs, ParseError},
 };
 
-#[derive(Debug, Error)]
-pub enum ThemeAttrParseError {
-    #[error("path not found")]
-    PathNotFound,
-    #[error("integer value parse error: [{0}]")]
-    ParseIntError(#[from] ParseIntError),
-    #[error("color value parse error: [{0}]")]
-    ColorParseError(#[from] color::ParseError),
-    #[error("string value parse error: [{0}]")]
-    StringParseError(#[from] StringParseError),
-}
-
 macro_rules! theme_attr {
-	($($name:tt($value:tt),)*) => {
+	($($name:tt($ty:tt),)*) => {
 		#[derive(Debug, Clone, strum::Display, strum::EnumIter)]
 		#[strum(serialize_all = "snake_case")]
 		pub enum ThemeAttr {
 		    $(
-				$name($value),
+				$name($ty),
 			)*
 		}
 
@@ -49,31 +36,35 @@ macro_rules! theme_attr {
 			fn from(value: ThemeAttr) -> Self {
 				match value {
 					$(
-						ThemeAttr::$name(val) => theme_attr!(Attr $value)(val),
+						ThemeAttr::$name(val) => theme_attr!(Attr $ty)(val),
 					)*
 				}
 			}
 		}
-		impl ThemeAttr {
+		impl FromCommandArgs for ThemeAttr {
-			pub fn from_raw_parts(path: &str, value: &str) -> Result<Self, ThemeAttrParseError> {
+			fn from_command_args<S: Into<String>, I: Iterator<Item = S>>(
-				match ThemeAttr::iter()
+					command: &str,
-					.find(|attr| attr.attr_path() == path)
+					args: I,
-					.map(|attr| -> Result<Self, ThemeAttrParseError> {
+				) -> Result<Self, ParseError> {
+					match ThemeAttr::iter()
+					.find(|attr| attr.attr_path() == command)
+					.map(|attr| -> Result<Self, ParseError> {
 						let mut attr = attr.clone();
+						let mut parser = ArgParser::from_strings(args.map(|a| a.into()));
 						match attr.borrow_mut() {
 							$(
-								ThemeAttr::$name(val) => *val = theme_attr!(Parse value: $value),
+								ThemeAttr::$name(val) => *val = parser.next_from_str(concat!("theme_attr(", stringify!($name), ")"))?,
 							)+
 						};
 						Ok(attr)
 					})
 					.map(|res| if let Err(err) = res  {panic!("::: {err}")} else {res})
-					.ok_or(ThemeAttrParseError::PathNotFound)
+					.ok_or(ParseError::InvalidCommand(command.to_string()))
-				{
+					{
-					Ok(res) => res,
+						Ok(res) => res,
-					Err(err) => return Err(err),
+						Err(err) => return Err(err),
+					}
 				}
-			}
 		}
 	};
 	(Attr u32) => {
@@ -91,24 +82,6 @@ macro_rules! theme_attr {
 	(Attr i32) => {
 		Attribute::Int
 	};
-	(Default u32) => {
-		0u32
-	};
-	(Default i32) => {
-		0i32
-	};
-	(Default Color) => {
-		Color::BLACK
-	};
-	(Default String) => {
-		String::new()
-	};
-	(Parse $v:tt: String) => {
-		$v.to_string()
-	};
-	(Parse $v:tt: $v_ty:tt) => {
-		$v.parse::<$v_ty>()?
-	}
 }
 
 theme_attr!(
@@ -178,13 +151,13 @@ impl<'de> Deserialize<'de> for ThemeAttr {
     {
         pub enum Expect {
             NoEquals,
-            ThemeAttrParseError(ThemeAttrParseError),
+            ParseError(ParseError),
         }
         impl Expected for Expect {
             fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
                 match self {
                     Expect::NoEquals => write!(f, "value having an equals sign"),
-                    Expect::ThemeAttrParseError(err) => write!(f, "parsing error: {err}"),
+                    Expect::ParseError(err) => write!(f, "parsing error: {err}"),
                 }
             }
         }
@@ -198,10 +171,10 @@ impl<'de> Deserialize<'de> for ThemeAttr {
             ))?;
 
         Ok(
-            Self::from_raw_parts(&first.trim(), &second.trim()).map_err(|err| {
+            Self::from_command_args(&first.trim(), [second.trim()].into_iter()).map_err(|err| {
                 serde::de::Error::invalid_value(
                     serde::de::Unexpected::Str(&str_val),
-                    &Expect::ThemeAttrParseError(err),
+                    &Expect::ParseError(err),
                 )
             })?,
         )

src/hlwm/window.rs

@@ -4,8 +4,8 @@ use serde::{Deserialize, Serialize};
 use strum::IntoEnumIterator;
 
 use super::{
+    parser::ParseError,
     rule::{Condition, RuleOperator},
-    StringParseError,
 };
 
 #[derive(Debug, Clone, Serialize, Deserialize, strum::EnumIter, PartialEq)]
@@ -21,7 +21,7 @@ pub enum Window {
 }
 
 impl FromStr for Window {
-    type Err = StringParseError;
+    type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         if let Ok(val) = i32::from_str(s) {
@@ -29,7 +29,7 @@ impl FromStr for Window {
         } else {
             Window::iter()
                 .find(|w| w.to_string() == s)
-                .ok_or(StringParseError::UnknownValue)
+                .ok_or(ParseError::ValueMissing)
         }
     }
 }

src/logerr.rs

@@ -2,10 +2,20 @@ use std::{error::Error, fmt::Debug};
 
 use log::{debug, error};
 
+// So we still get a useful panic when logs are filtered
+macro_rules! error_and_panic {
+	($($arg:tt)*) => {
+		log::error!($($arg)*);
+		panic!($($arg)*);
+	};
+}
+
 pub trait UnwrapLog {
     type Target;
 
     fn unwrap_or_log(self, default: Self::Target) -> Self::Target;
+    fn unwrap_log(self) -> Self::Target;
+    fn expect_log(self, expect: &str) -> Self::Target;
 }
 
 impl<T, E> UnwrapLog for Result<T, E>
@@ -28,6 +38,26 @@ where
             }
         }
     }
+
+    fn unwrap_log(self) -> Self::Target {
+        match self {
+            Ok(target) => target,
+            Err(err) => {
+                error_and_panic!("called `Result::unwrap_log()` on an `Err` value: {err}");
+            }
+        }
+    }
+
+    fn expect_log(self, expect: &str) -> Self::Target {
+        match self {
+            Ok(target) => target,
+            Err(err) => {
+                error_and_panic!(
+                    "[{expect}] called `Result::expect_log()` on an `Err` value: {err}"
+                );
+            }
+        }
+    }
 }
 
 impl<T> UnwrapLog for Option<T>
@@ -49,4 +79,22 @@ where
             }
         }
     }
+
+    fn unwrap_log(self) -> Self::Target {
+        match self {
+            Some(target) => target,
+            None => {
+                error_and_panic!("called `Option::unwrap_log()` on a `None` value");
+            }
+        }
+    }
+
+    fn expect_log(self, expect: &str) -> Self::Target {
+        match self {
+            Some(target) => target,
+            None => {
+                error_and_panic!("[{expect}] called `Option::expect_log()` on an `None` value");
+            }
+        }
+    }
 }

src/main.rs

@@ -1,10 +1,11 @@
-#![feature(macro_metavar_expr)]
 use std::path::{Path, PathBuf};
 
+use add::Add;
 use clap::{Parser, Subcommand};
 use config::Config;
-use log::{error, info};
+use log::{error, info, LevelFilter};
 
+mod add;
 pub mod cmd;
 mod config;
 pub mod environ;
@@ -41,10 +42,16 @@ enum HlctlCommand {
     },
     /// Start the top panel
     Panel,
+    /// Add an element to the existing hlctl config
+    #[command(subcommand)]
+    Add(Add),
 }
 
 fn main() {
-    pretty_env_logger::init();
+    pretty_env_logger::formatted_builder()
+        .filter_module("rustyline", LevelFilter::Error)
+        .parse_default_env()
+        .init();
 
     let args = Args::parse();
     match args.command {
@@ -56,6 +63,11 @@ fn main() {
                 error!("panel: {err}");
             }
         }
+        HlctlCommand::Add(add) => {
+            if let Err(err) = add.run() {
+                error!("add: {err}");
+            }
+        }
     }
 }
 

src/split.rs

@@ -38,6 +38,37 @@ impl<'a> From<&'a String> for SplitArg<'a> {
     }
 }
 
+/// Assuming the input string is in format `({type_name}){path}`,
+/// it will return `Some(("{type_name}", "{path}"))`
+pub fn parens(s: &str) -> Option<(String, String)> {
+    let mut chars = s.chars();
+    match chars.next() {
+        Some(c) => {
+            if c != '(' {
+                return None;
+            }
+        }
+        None => return None,
+    }
+
+    let mut working = Vec::with_capacity(s.len());
+    let type_name = loop {
+        if let Some(next) = chars.next() {
+            if next == ')' {
+                break working.into_iter().collect::<String>();
+            }
+            if !next.is_alphanumeric() {
+                return None;
+            }
+            working.push(next);
+        } else {
+            return None;
+        }
+    };
+
+    Some((type_name, chars.collect()))
+}
+
 pub fn tab_or_space<'a, S>(s: S) -> Vec<String>
 where
     S: Into<SplitArg<'a>>,