fixed spawn and rules not parsing/passing correctly

Cargo.toml

@@ -25,4 +25,3 @@ pretty_assertions = "1.4.0"
 [profile.release]
 opt-level = 3
 strip = "debuginfo"
-lto = "fat"

src/config.rs

@@ -22,6 +22,7 @@ use thiserror::Error;
 
 use crate::{
     hlwm::{
+        self,
         attribute::Attribute,
         color::Color,
         command::{CommandError, HlwmCommand},
@@ -329,6 +330,13 @@ impl Config {
         .chain(self.settings_command_set())
         .chain(self.rule_command_set())
         .chain(self.service_command_set())
+        .chain([
+            HlwmCommand::Unlock,
+            HlwmCommand::Spawn {
+                executable: "hlctl".into(),
+                args: vec!["panel".into()],
+            },
+        ])
         .collect())
     }
 
@@ -358,10 +366,14 @@ impl Config {
                 HlwmCommand::And {
                     separator: Separator::Comma,
                     commands: vec![
-                        HlwmCommand::Compare {
+                        HlwmCommand::Substitute {
+                            identifier: "CURRENT_INDEX".into(),
+                            attribute_path: "tags.focus.index".into(),
+                            command: Box::new(HlwmCommand::Compare {
                                 attribute: "tags.by-name.default.index".into(),
                                 operator: Operator::Equal,
-                            value: "tags.focus.index".into(),
+                                value: "CURRENT_INDEX".into(),
+                            }),
                         },
                         HlwmCommand::UseIndex {
                             index: Index::Relative(1),
@@ -372,9 +384,8 @@ impl Config {
                 .to_try(),
                 HlwmCommand::MergeTag {
                     tag: "default".to_string(),
-                    target: None,
+                    target: Some(hlwm::Tag::Name(self.tags.first().unwrap().to_string())),
-                }
+                },
-                .to_try(),
             ])
             .collect()
     }

src/hlwm/command.rs

@@ -588,12 +588,6 @@ impl HlwmCommand {
 impl HlwmCommand {
     #[inline(always)]
     pub(crate) fn args(&self) -> Vec<String> {
-        if let Self::Spawn { executable, args } = self {
-            return vec!["spawn".to_string(), executable.to_string()]
-                .into_iter()
-                .chain(args.into_iter().cloned())
-                .collect();
-        }
         self.to_command_string()
             .split('\t')
             .map(|a| a.to_string())
@@ -652,14 +646,7 @@ impl ToCommandString for HlwmCommand {
                 window.as_ref().map(|w| w.to_string()).unwrap_or_default(),
             ),
             HlwmCommand::Spawn { executable, args } => {
-                format!(
+                format!("{self}\t{executable}\t{}", (&args).join("\t"))
-                    "{self}\t\"{executable}\"\t{}",
-                    (&args)
-                        .into_iter()
-                        .map(|arg| format!("\"{arg}\""))
-                        .collect::<Vec<String>>()
-                        .join("\t")
-                )
             }
             HlwmCommand::GetAttr(attr) => format!("{self}\t{attr}"),
             HlwmCommand::SetAttr {
@@ -900,7 +887,7 @@ mod test {
                         executable: "grep".into(),
                         args: vec!["content".into()],
                     },
-                    "spawn\t\"grep\"\t\"content\"".into(),
+                    "spawn\tgrep\tcontent".into(),
                 ),
                 HlwmCommand::GetAttr(_) => (
                     HlwmCommand::GetAttr("my_attr".into()),
@@ -1175,4 +1162,16 @@ mod test {
             )
         }
     }
+
+    #[test]
+    fn rule_fixedsize_not_omitted() {
+        let cmd = HlwmCommand::Rule(Rule::new(
+            Some(Condition::FixedSize),
+            vec![Consequence::Floating(true)],
+            None,
+            None,
+        ))
+        .to_command_string();
+        assert_eq!("rule\tfixedsize\t--floating=on", cmd);
+    }
 }

src/hlwm/mod.rs

@@ -56,9 +56,6 @@ impl Client {
     }
 
     /// Run the command and wait for it to finish.
-    ///
-    /// To run the command and return a handle instead of waiting,
-    /// see [Client::spawn]
     pub fn execute(&self, command: HlwmCommand) -> Result<process::Output, CommandError> {
         let args = command.args();
         debug!("running command: [{}]", (&args).join(" "),);

src/hlwm/rule.rs

@@ -88,47 +88,19 @@ impl FromStr for Rule {
         let mut consequences = vec![];
         let mut label: Option<String> = None;
         let mut flag: Option<Flag> = None;
-
+        let mut args = parts
-        for arg in (&parts)
             .into_iter()
-            .map(|a| a.strip_prefix("--"))
+            .map(|part| part.strip_prefix("--").unwrap_or(part.as_str()).to_string());
-            .filter(|a| a.is_some())
-        {
-            let arg = arg.unwrap();
-            if condition.is_none() && arg == Condition::FixedSize.to_string() {
-                condition = Some(Condition::FixedSize);
-                continue;
-            }
-            let parts = arg.split(['=', '~']).collect::<Vec<_>>();
-            if parts.len() != 2 {
-                return Err(StringParseError::InvalidLength(parts.len(), "rule=parts"));
-            }
-            let mut parts = parts.into_iter();
-            let (name, value) = (parts.next().unwrap(), parts.next().unwrap());
 
-            if name == "label" && label.is_none() {
-                label = Some(value.to_string());
-                continue;
-            }
-
-            if condition.is_none() && Condition::iter().any(|prop| prop.to_string() == name) {
-                condition = Some(match Condition::from_str(arg) {
-                    Ok(arg) => arg,
-                    Err(err) => panic!("<<condition>>\n\n{err}\n\n\n"),
-                });
-                continue;
-            }
-
-            if Consequence::iter().any(|cons| cons.to_string() == name) {
-                consequences.push(match Consequence::from_str(arg) {
-                    Ok(arg) => arg,
-                    Err(err) => panic!("<<consequence>>\n\n{err}\n\n\n"),
-                });
-                continue;
-            }
-        }
-        let mut args = parts.into_iter().filter(|a| !a.starts_with("--"));
         while let Some(arg) = args.next() {
+            if label.is_none() {
+                if let Some((name, value)) = arg.split_once('=') {
+                    if name.trim() == "label" {
+                        label = Some(value.trim().to_string());
+                        continue;
+                    }
+                }
+            }
             if flag.is_none() {
                 if let Ok(flag_res) = Flag::from_str(&arg) {
                     flag = Some(flag_res);
@@ -325,7 +297,9 @@ impl FromStr for Condition {
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         // Handle the case for fixedsize first so that we can treat the rest
         // of the variants as having arguments
-        if s == Self::FixedSize.to_string() {
+        //
+        // Also, sometimes `fixedsize=0` could come back from `herbstclient list_rules`
+        if s.starts_with(&Self::FixedSize.to_string()) {
             return Ok(Self::FixedSize);
         }
         let ((name, match_val), match_char) =
@@ -669,8 +643,13 @@ impl FromStr for Unrule {
 
 #[cfg(test)]
 mod test {
+    use std::str::FromStr;
+
     use serde::{Deserialize, Serialize};
 
+    use crate::hlwm::rule::FloatPlacement;
+    use pretty_assertions::assert_eq;
+
     use super::{Condition, Consequence, Flag, Rule, RuleOperator};
 
     #[derive(Debug, Deserialize, Serialize)]
@@ -680,7 +659,8 @@ mod test {
 
     #[test]
     fn rule_serialize_deserialize() {
-        for rule in [Rule::new(
+        for rule in [
+            Rule::new(
                 Some(Condition::Class {
                     operator: RuleOperator::Equal,
                     value: "Netscape".into(),
@@ -688,7 +668,14 @@ mod test {
                 vec![Consequence::Tag(1.to_string())],
                 Some("label".into()),
                 Some(Flag::Not),
-        )] {
+            ),
+            Rule::new(
+                Some(Condition::FixedSize),
+                vec![Consequence::Floating(true)],
+                None,
+                None,
+            ),
+        ] {
             let serialized = toml::to_string_pretty(&RuleWrapper { rule: rule.clone() })
                 .expect("serializing rule");
             let deserialized: RuleWrapper =
@@ -696,4 +683,70 @@ mod test {
             assert_eq!(rule, deserialized.rule);
         }
     }
+
+    #[test]
+    fn rules_from_list_rules_parse() {
+        const INPUT: &str = r#"label=0	windowtype~_NET_WM_WINDOW_TYPE_(DIALOG|UTILITY|SPLASH)	floating=on
+label=1	focus=on
+label=2	floatplacement=smart
+label=3	windowtype~_NET_WM_WINDOW_TYPE_(DIALOG)	floating=on
+label=4	windowtype~_NET_WM_WINDOW_TYPE_(NOTIFICATION|DOCK|DESKTOP)	manage=off
+label=5	fixedsize=0	floating=true"#;
+
+        let parsed = INPUT
+            .split('\n')
+            .map(|l| Rule::from_str(l))
+            .collect::<Result<Vec<_>, _>>()
+            .expect("parsing error");
+
+        let expected = [
+            Rule::new(
+                Some(Condition::WindowType {
+                    operator: RuleOperator::Regex,
+                    value: "_NET_WM_WINDOW_TYPE_(DIALOG|UTILITY|SPLASH)".into(),
+                }),
+                vec![Consequence::Floating(true)],
+                Some("0".into()),
+                None,
+            ),
+            Rule::new(None, vec![Consequence::Focus(true)], Some("1".into()), None),
+            Rule::new(
+                None,
+                vec![Consequence::FloatPlacement(FloatPlacement::Smart)],
+                Some("2".into()),
+                None,
+            ),
+            Rule::new(
+                Some(Condition::WindowType {
+                    operator: RuleOperator::Regex,
+                    value: "_NET_WM_WINDOW_TYPE_(DIALOG)".into(),
+                }),
+                vec![Consequence::Floating(true)],
+                Some("3".to_string()),
+                None,
+            ),
+            Rule::new(
+                Some(Condition::WindowType {
+                    operator: RuleOperator::Regex,
+                    value: "_NET_WM_WINDOW_TYPE_(NOTIFICATION|DOCK|DESKTOP)".into(),
+                }),
+                vec![Consequence::Manage(false)],
+                Some("4".to_string()),
+                None,
+            ),
+            Rule::new(
+                Some(Condition::FixedSize),
+                vec![Consequence::Floating(true)],
+                Some("5".into()),
+                None,
+            ),
+        ];
+
+        parsed
+            .into_iter()
+            .zip(expected)
+            .for_each(|(parsed, expected)| {
+                assert_eq!(expected, parsed);
+            });
+    }
 }