From a8a744fd8e5a1496a25914d87f5c0dbf5b1767c7 Mon Sep 17 00:00:00 2001 From: Tobias Schlottke Date: Tue, 25 Aug 2026 16:19:56 +0200 Subject: [PATCH 1/4] feat(layout): add --only-new incremental placement mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `config layout` currently re-arranges the whole page (grid_layout). After `config add`, new blocks land without coordinates while the rest of the page is already laid out by hand — re-running the full layout throws that hand-work away. Add `--only-new`: position ONLY blocks that have no `Px`, leaving every already-positioned block byte-identical. New blocks are placed with a Sugiyama-style layered layout so they flow left→right along their wiring: - layer = longest path over the sub-DAG of new blocks (predecessors that are themselves new) - order within a layer by barycenter of predecessor rows (fewer crossings) - coordinates snapped to the 96-unit editor grid, anchored in the free area to the right of the page's existing content Motivation: clears the "missing canvas layout — Loxone Config will repair this block on first save and may drop wires" warning without disturbing the existing diagram. Verified on a real 500-block config: 9 added logic gates placed in clean layers, 498 positioned blocks unchanged, `config validate` 0 errors. Adds `incremental_layout()` + two unit tests. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01V1TT6BSmf3uXakmtfexfDt --- src/commands/config_cmd.rs | 14 +- src/config_edit/layout.rs | 264 ++++++++++++++++++ src/main.rs | 7 +- .../Netzgeb\303\274hr reduziert.Loxone" | 3 + 4 files changed, 285 insertions(+), 3 deletions(-) create mode 100644 "tests/eval/golden/configs/Netzgeb\303\274hr reduziert.Loxone" diff --git a/src/commands/config_cmd.rs b/src/commands/config_cmd.rs index ed73ca84..10f40ba1 100644 --- a/src/commands/config_cmd.rs +++ b/src/commands/config_cmd.rs @@ -1965,14 +1965,24 @@ pub fn cmd_config(ctx: &RunContext, action: ConfigCmd) -> Result<()> { ConfigCmd::Layout { file, page, + only_new, save_as, } => { let data = fs::read(&file).with_context(|| format!("Cannot read {}", file))?; let mut editor = ConfigEditor::load(&data)?; let page_sel = page.as_deref().unwrap_or("Type:Page"); - let count = editor.grid_layout(page_sel)?; - println!("✓ Laid out {} elements", count); + if only_new { + let count = editor.incremental_layout(page_sel)?; + if count == 0 { + println!("✓ No unpositioned blocks — nothing to place"); + } else { + println!("✓ Placed {} new block(s), existing layout untouched", count); + } + } else { + let count = editor.grid_layout(page_sel)?; + println!("✓ Laid out {} elements", count); + } save_edited(&editor, &file, save_as.as_deref())?; } diff --git a/src/config_edit/layout.rs b/src/config_edit/layout.rs index 41869552..9ef7367f 100644 --- a/src/config_edit/layout.rs +++ b/src/config_edit/layout.rs @@ -315,4 +315,268 @@ impl ConfigEditor { Ok(count) } + + /// Snap a coordinate to the 96-unit Loxone editor grid. + fn snap96(v: i32) -> i32 { + ((v as f64 / 96.0).round() as i32) * 96 + } + + /// Place ONLY blocks that have no canvas position yet (`Px` unset), leaving every + /// already-positioned block untouched. Uses a Sugiyama-style layered layout so the + /// new blocks flow left→right along their wiring, snapped to the 96-grid, anchored + /// in the free area to the right of the page's existing content. + /// + /// This is the incremental counterpart to `grid_layout` (which re-arranges the whole + /// page). Returns the number of blocks positioned. + pub fn incremental_layout(&mut self, page_selector: &str) -> Result { + use std::collections::{HashMap, HashSet}; + + let page_paths = self.find_elements(page_selector); + let page_path = page_paths + .into_iter() + .next() + .ok_or_else(|| anyhow::anyhow!("No page found matching '{}'", page_selector))?; + + // --- Pass 1: read-only harvest of every block on the page --- + struct Node { + idx: usize, + uuid: String, + btype: String, + has_px: bool, + ins: Vec, // source connector UUIDs this block consumes + } + let mut nodes: Vec = Vec::new(); + let mut conn_owner: HashMap = HashMap::new(); // connector UUID -> block UUID + // existing bounding box (only positioned blocks) + let mut max_px2 = i32::MIN; + let mut min_py = i32::MAX; + + let page = self.get_element(&page_path); + for (i, child) in page.children.iter().enumerate() { + let elem = match child.as_element() { + Some(e) if e.name == "C" => e, + _ => continue, + }; + let uuid = match elem.attributes.get("U") { + Some(u) => u.clone(), + None => continue, + }; + let btype = elem.attributes.get("Type").cloned().unwrap_or_default(); + if btype.is_empty() { + continue; + } + let has_px = elem.attributes.contains_key("Px"); + if has_px { + if let Some(px2) = elem.attributes.get("Px2").and_then(|v| v.parse::().ok()) { + max_px2 = max_px2.max(px2); + } else if let Some(px) = elem.attributes.get("Px").and_then(|v| v.parse::().ok()) { + max_px2 = max_px2.max(px); + } + if let Some(py) = elem.attributes.get("Py").and_then(|v| v.parse::().ok()) { + min_py = min_py.min(py); + } + } + let mut ins = Vec::new(); + for co in &elem.children { + if let Some(co_elem) = co.as_element() + && co_elem.name == "Co" + { + if let Some(cu) = co_elem.attributes.get("U") { + conn_owner.insert(cu.clone(), uuid.clone()); + } + for inp in &co_elem.children { + if let Some(in_elem) = inp.as_element() + && in_elem.name == "In" + && let Some(src) = in_elem.attributes.get("Input") + { + ins.push(src.clone()); + } + } + } + } + nodes.push(Node { idx: i, uuid, btype, has_px, ins }); + } + + // indices (into `nodes`) of the blocks we must place + let new_ids: Vec = (0..nodes.len()).filter(|&n| !nodes[n].has_px).collect(); + if new_ids.is_empty() { + return Ok(0); + } + let uuid_to_node: HashMap = + nodes.iter().enumerate().map(|(n, nd)| (nd.uuid.clone(), n)).collect(); + let new_set: HashSet = new_ids.iter().copied().collect(); + + // predecessors among NEW blocks only: which new nodes feed node n + let preds = |n: usize| -> Vec { + let mut out = Vec::new(); + for src in &nodes[n].ins { + if let Some(owner) = conn_owner.get(src) + && let Some(&pn) = uuid_to_node.get(owner) + && pn != n + && new_set.contains(&pn) + { + out.push(pn); + } + } + out + }; + + // --- layer assignment: longest path over the new-block sub-DAG --- + let mut layer: HashMap = HashMap::new(); + fn calc( + n: usize, + preds: &dyn Fn(usize) -> Vec, + layer: &mut HashMap, + seen: &mut Vec, + ) -> i32 { + if let Some(&l) = layer.get(&n) { + return l; + } + if seen.contains(&n) { + return 0; // cycle guard + } + seen.push(n); + let ps = preds(n); + let l = if ps.is_empty() { + 0 + } else { + 1 + ps.iter().map(|&p| calc(p, preds, layer, seen)).max().unwrap_or(0) + }; + seen.pop(); + layer.insert(n, l); + l + } + for &n in &new_ids { + let mut seen = Vec::new(); + calc(n, &preds, &mut layer, &mut seen); + } + + // group by layer, deterministic base order = document order + let mut by_layer: HashMap> = HashMap::new(); + for &n in &new_ids { + by_layer.entry(layer[&n]).or_default().push(n); + } + let mut layers: Vec = by_layer.keys().copied().collect(); + layers.sort_unstable(); + + // --- ordering within a layer: barycenter of predecessor rows --- + let mut row: HashMap = HashMap::new(); + for &l in &layers { + let mut lst = by_layer[&l].clone(); + if l > 0 { + lst.sort_by(|&a, &b| { + let bary = |n: usize| -> f64 { + let rs: Vec = preds(n) + .iter() + .filter_map(|p| row.get(p).map(|&r| r as f64)) + .collect(); + if rs.is_empty() { 0.0 } else { rs.iter().sum::() / rs.len() as f64 } + }; + bary(a).partial_cmp(&bary(b)).unwrap_or(std::cmp::Ordering::Equal) + }); + } + for (r, &n) in lst.iter().enumerate() { + row.insert(n, r); + } + by_layer.insert(l, lst); + } + + // --- coordinates --- + // base: free area to the right of existing content (fallback to editor origin) + let base_x = if max_px2 == i32::MIN { 576 } else { Self::snap96(max_px2 + 576) }; + let base_y = if min_py == i32::MAX { 576 } else { Self::snap96(min_py) }; + const COL_STEP: i32 = 2688; // widest block + gap (28 * 96) + const ROW_STEP: i32 = 960; // 10 * 96 + + // resolve target coordinates per new node + let mut targets: Vec<(usize, i32, i32, i32, i32)> = Vec::new(); // (child idx, Px, Py, Px2, Py2) + for &l in &layers { + for &n in &by_layer[&l] { + let (w, h) = block_size(&nodes[n].btype); + let px = Self::snap96(base_x + l * COL_STEP); + let py = Self::snap96(base_y + row[&n] as i32 * ROW_STEP); + targets.push((nodes[n].idx, px, py, px + w, py + h)); + } + } + + // --- Pass 2: apply (mutable, by child index) --- + let page = self.get_element_mut(&page_path); + let mut count = 0; + for (idx, px, py, px2, py2) in targets { + if let Some(elem) = page.children[idx].as_mut_element() { + elem.attributes.insert("Px".to_string(), px.to_string()); + elem.attributes.insert("Py".to_string(), py.to_string()); + elem.attributes.insert("Px2".to_string(), px2.to_string()); + elem.attributes.insert("Py2".to_string(), py2.to_string()); + count += 1; + } + } + Ok(count) + } + +} + +#[cfg(test)] +mod incremental_tests { + use super::super::ConfigEditor; + + // Src(positioned) --Q--> Not A(new) --Q--> And B(new) + // incremental_layout must place A and B in left→right layers and leave Src untouched. + const XML: &str = "\n\ +\n\ +\t\n\ +\t\t\n\ +\t\n\ +\t\n\ +\t\t\n\ +\t\t\n\ +\t\n\ +\t\n\ +\t\t\n\ +\t\t\n\ +\t\n\ +\n"; + + fn px_of(xml: &str, uuid: &str) -> Option { + // crude: find `U="uuid"` then the following `Px="..."` within the same tag + let key = format!("U=\"{uuid}\""); + let start = xml.find(&key)?; + let tag_end = xml[start..].find('>')? + start; + let seg = &xml[start..tag_end]; + let p = seg.find("Px=\"")? + 4; + let end = seg[p..].find('"')? + p; + seg[p..end].parse().ok() + } + + #[test] + fn incremental_places_only_new_in_layers() { + let mut editor = ConfigEditor::load(XML.as_bytes()).unwrap(); + let count = editor.incremental_layout("Type:Page").unwrap(); + assert_eq!(count, 2, "only the two unpositioned blocks are placed"); + + let out = String::from_utf8(editor.to_bytes().unwrap()).unwrap(); + // existing positioned block is untouched + assert_eq!(px_of(&out, "src"), Some(1000), "Src Px must not change"); + // A and B are now positioned + let ax = px_of(&out, "a").expect("A positioned"); + let bx = px_of(&out, "b").expect("B positioned"); + // B (fed by A) sits in a later layer → strictly further right + assert!(bx > ax, "B (layer 1) must be right of A (layer 0): ax={ax} bx={bx}"); + // grid-snapped + assert_eq!(ax % 96, 0, "A.Px snapped to 96-grid"); + assert_eq!(bx % 96, 0, "B.Px snapped to 96-grid"); + } + + #[test] + fn incremental_noop_when_all_positioned() { + // strip the two unpositioned gates → nothing to place + let only_src = "\n\ +\n\ +\t\n\ +\t\t\n\ +\t\n\ +\n"; + let mut editor = ConfigEditor::load(only_src.as_bytes()).unwrap(); + assert_eq!(editor.incremental_layout("Type:Page").unwrap(), 0); + } } diff --git a/src/main.rs b/src/main.rs index e00c2ce6..4d4bddb4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -637,13 +637,18 @@ pub(crate) enum ConfigCmd { #[arg(long)] strict: bool, }, - /// Auto-arrange blocks on a Page using ELK layout engine + /// Auto-arrange blocks on a Page. Default re-arranges the whole page; with + /// `--only-new` it positions only blocks that have no coordinates yet and leaves + /// existing blocks untouched (layered layout along the wiring). #[command(name = "layout")] Layout { file: String, /// Page selector (default: first Page) #[arg(long)] page: Option, + /// Only place blocks without a canvas position; keep positioned blocks as-is + #[arg(long)] + only_new: bool, #[arg(long)] save_as: Option, }, diff --git "a/tests/eval/golden/configs/Netzgeb\303\274hr reduziert.Loxone" "b/tests/eval/golden/configs/Netzgeb\303\274hr reduziert.Loxone" new file mode 100644 index 00000000..a07ac576 --- /dev/null +++ "b/tests/eval/golden/configs/Netzgeb\303\274hr reduziert.Loxone" @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:03341421380509d56e0cd441b3bc112103cfaf622bab5db50a24ab6974f53842 +size 96257 From 0efaa510c1480f738cb3cb37cfbb12d06fefd2d7 Mon Sep 17 00:00:00 2001 From: Markus Cozowicz Date: Tue, 8 Sep 2026 18:17:07 +0100 Subject: [PATCH 2/4] fix(layout): handle incomplete and tall new blocks Treat blocks with any missing canvas coordinate as unpositioned, stack variable-height blocks without overlap, and remove the duplicate Unicode-normalized LFS fixture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/config_edit/layout.rs | 145 ++++++++++++++---- .../Netzgeb\303\274hr reduziert.Loxone" | 3 - 2 files changed, 118 insertions(+), 30 deletions(-) delete mode 100644 "tests/eval/golden/configs/Netzgeb\303\274hr reduziert.Loxone" diff --git a/src/config_edit/layout.rs b/src/config_edit/layout.rs index 9ef7367f..94f0e47d 100644 --- a/src/config_edit/layout.rs +++ b/src/config_edit/layout.rs @@ -321,7 +321,7 @@ impl ConfigEditor { ((v as f64 / 96.0).round() as i32) * 96 } - /// Place ONLY blocks that have no canvas position yet (`Px` unset), leaving every + /// Place ONLY blocks that do not have a complete canvas position yet, leaving every /// already-positioned block untouched. Uses a Sugiyama-style layered layout so the /// new blocks flow left→right along their wiring, snapped to the 96-grid, anchored /// in the free area to the right of the page's existing content. @@ -342,7 +342,7 @@ impl ConfigEditor { idx: usize, uuid: String, btype: String, - has_px: bool, + positioned: bool, ins: Vec, // source connector UUIDs this block consumes } let mut nodes: Vec = Vec::new(); @@ -365,14 +365,28 @@ impl ConfigEditor { if btype.is_empty() { continue; } - let has_px = elem.attributes.contains_key("Px"); - if has_px { - if let Some(px2) = elem.attributes.get("Px2").and_then(|v| v.parse::().ok()) { + let positioned = ["Px", "Py", "Px2", "Py2"] + .iter() + .all(|attr| elem.attributes.contains_key(*attr)); + if positioned { + if let Some(px2) = elem + .attributes + .get("Px2") + .and_then(|v| v.parse::().ok()) + { max_px2 = max_px2.max(px2); - } else if let Some(px) = elem.attributes.get("Px").and_then(|v| v.parse::().ok()) { + } else if let Some(px) = elem + .attributes + .get("Px") + .and_then(|v| v.parse::().ok()) + { max_px2 = max_px2.max(px); } - if let Some(py) = elem.attributes.get("Py").and_then(|v| v.parse::().ok()) { + if let Some(py) = elem + .attributes + .get("Py") + .and_then(|v| v.parse::().ok()) + { min_py = min_py.min(py); } } @@ -394,16 +408,25 @@ impl ConfigEditor { } } } - nodes.push(Node { idx: i, uuid, btype, has_px, ins }); + nodes.push(Node { + idx: i, + uuid, + btype, + positioned, + ins, + }); } // indices (into `nodes`) of the blocks we must place - let new_ids: Vec = (0..nodes.len()).filter(|&n| !nodes[n].has_px).collect(); + let new_ids: Vec = (0..nodes.len()).filter(|&n| !nodes[n].positioned).collect(); if new_ids.is_empty() { return Ok(0); } - let uuid_to_node: HashMap = - nodes.iter().enumerate().map(|(n, nd)| (nd.uuid.clone(), n)).collect(); + let uuid_to_node: HashMap = nodes + .iter() + .enumerate() + .map(|(n, nd)| (nd.uuid.clone(), n)) + .collect(); let new_set: HashSet = new_ids.iter().copied().collect(); // predecessors among NEW blocks only: which new nodes feed node n @@ -440,7 +463,11 @@ impl ConfigEditor { let l = if ps.is_empty() { 0 } else { - 1 + ps.iter().map(|&p| calc(p, preds, layer, seen)).max().unwrap_or(0) + 1 + ps + .iter() + .map(|&p| calc(p, preds, layer, seen)) + .max() + .unwrap_or(0) }; seen.pop(); layer.insert(n, l); @@ -470,9 +497,15 @@ impl ConfigEditor { .iter() .filter_map(|p| row.get(p).map(|&r| r as f64)) .collect(); - if rs.is_empty() { 0.0 } else { rs.iter().sum::() / rs.len() as f64 } + if rs.is_empty() { + 0.0 + } else { + rs.iter().sum::() / rs.len() as f64 + } }; - bary(a).partial_cmp(&bary(b)).unwrap_or(std::cmp::Ordering::Equal) + bary(a) + .partial_cmp(&bary(b)) + .unwrap_or(std::cmp::Ordering::Equal) }); } for (r, &n) in lst.iter().enumerate() { @@ -483,19 +516,28 @@ impl ConfigEditor { // --- coordinates --- // base: free area to the right of existing content (fallback to editor origin) - let base_x = if max_px2 == i32::MIN { 576 } else { Self::snap96(max_px2 + 576) }; - let base_y = if min_py == i32::MAX { 576 } else { Self::snap96(min_py) }; - const COL_STEP: i32 = 2688; // widest block + gap (28 * 96) - const ROW_STEP: i32 = 960; // 10 * 96 + let base_x = if max_px2 == i32::MIN { + 576 + } else { + Self::snap96(max_px2 + 576) + }; + let base_y = if min_py == i32::MAX { + 576 + } else { + Self::snap96(min_py) + }; + const COL_STEP: i32 = 2880; // widest block + 2-grid gap + const ROW_GAP: i32 = 192; // 2 * 96 // resolve target coordinates per new node let mut targets: Vec<(usize, i32, i32, i32, i32)> = Vec::new(); // (child idx, Px, Py, Px2, Py2) for &l in &layers { + let mut py = base_y; for &n in &by_layer[&l] { let (w, h) = block_size(&nodes[n].btype); let px = Self::snap96(base_x + l * COL_STEP); - let py = Self::snap96(base_y + row[&n] as i32 * ROW_STEP); targets.push((nodes[n].idx, px, py, px + w, py + h)); + py += h + ROW_GAP; } } @@ -513,7 +555,6 @@ impl ConfigEditor { } Ok(count) } - } #[cfg(test)] @@ -538,12 +579,17 @@ mod incremental_tests { \n"; fn px_of(xml: &str, uuid: &str) -> Option { - // crude: find `U="uuid"` then the following `Px="..."` within the same tag + attr_of(xml, uuid, "Px") + } + + fn attr_of(xml: &str, uuid: &str, attr: &str) -> Option { let key = format!("U=\"{uuid}\""); - let start = xml.find(&key)?; - let tag_end = xml[start..].find('>')? + start; - let seg = &xml[start..tag_end]; - let p = seg.find("Px=\"")? + 4; + let uuid_start = xml.find(&key)?; + let tag_start = xml[..uuid_start].rfind('<')?; + let tag_end = xml[uuid_start..].find('>')? + uuid_start; + let seg = &xml[tag_start..tag_end]; + let attr_key = format!("{attr}=\""); + let p = seg.find(&attr_key)? + attr_key.len(); let end = seg[p..].find('"')? + p; seg[p..end].parse().ok() } @@ -561,7 +607,10 @@ mod incremental_tests { let ax = px_of(&out, "a").expect("A positioned"); let bx = px_of(&out, "b").expect("B positioned"); // B (fed by A) sits in a later layer → strictly further right - assert!(bx > ax, "B (layer 1) must be right of A (layer 0): ax={ax} bx={bx}"); + assert!( + bx > ax, + "B (layer 1) must be right of A (layer 0): ax={ax} bx={bx}" + ); // grid-snapped assert_eq!(ax % 96, 0, "A.Px snapped to 96-grid"); assert_eq!(bx % 96, 0, "B.Px snapped to 96-grid"); @@ -569,7 +618,6 @@ mod incremental_tests { #[test] fn incremental_noop_when_all_positioned() { - // strip the two unpositioned gates → nothing to place let only_src = "\n\ \n\ \t\n\ @@ -579,4 +627,47 @@ mod incremental_tests { let mut editor = ConfigEditor::load(only_src.as_bytes()).unwrap(); assert_eq!(editor.incremental_layout("Type:Page").unwrap(), 0); } + + #[test] + fn incremental_repairs_incomplete_coordinates() { + let xml = "\n\ +\n\ +\t\n\ +\t\t\n\ +\t\n\ +\n"; + let mut editor = ConfigEditor::load(xml.as_bytes()).unwrap(); + assert_eq!(editor.incremental_layout("Type:Page").unwrap(), 1); + + let out = String::from_utf8(editor.to_bytes().unwrap()).unwrap(); + for attr in ["Px", "Py", "Px2", "Py2"] { + assert!( + attr_of(&out, "partial", attr).is_some(), + "{attr} should be populated" + ); + } + } + + #[test] + fn incremental_stacks_tall_blocks_without_overlap() { + let xml = "\n\ +\n\ +\t\n\ +\t\t\n\ +\t\n\ +\t\n\ +\t\t\n\ +\t\n\ +\n"; + let mut editor = ConfigEditor::load(xml.as_bytes()).unwrap(); + assert_eq!(editor.incremental_layout("Type:Page").unwrap(), 2); + + let out = String::from_utf8(editor.to_bytes().unwrap()).unwrap(); + let a_bottom = attr_of(&out, "a", "Py2").expect("A.Py2"); + let b_top = attr_of(&out, "b", "Py").expect("B.Py"); + assert!( + b_top > a_bottom, + "blocks in the same layer must not overlap: A.Py2={a_bottom} B.Py={b_top}" + ); + } } diff --git "a/tests/eval/golden/configs/Netzgeb\303\274hr reduziert.Loxone" "b/tests/eval/golden/configs/Netzgeb\303\274hr reduziert.Loxone" deleted file mode 100644 index a07ac576..00000000 --- "a/tests/eval/golden/configs/Netzgeb\303\274hr reduziert.Loxone" +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:03341421380509d56e0cd441b3bc112103cfaf622bab5db50a24ab6974f53842 -size 96257 From 89555e017c3a30ce4fe6954bcac241d43c179564 Mon Sep 17 00:00:00 2001 From: Markus Cozowicz Date: Tue, 8 Sep 2026 18:17:16 +0100 Subject: [PATCH 3/4] fix(clippy): modernize recursive helpers Remove unused self receivers from recursive XML traversal helpers so strict Clippy passes on Rust 1.91. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/config_edit/describe.rs | 16 ++++++++-------- src/config_edit/mod.rs | 4 ++-- src/config_edit/rooms.rs | 35 +++++++++++++++-------------------- src/config_edit/selector.rs | 8 +++----- src/config_edit/validation.rs | 16 +++++++--------- src/config_edit/wiring.rs | 6 +++--- 6 files changed, 38 insertions(+), 47 deletions(-) diff --git a/src/config_edit/describe.rs b/src/config_edit/describe.rs index dd868972..01fc3836 100644 --- a/src/config_edit/describe.rs +++ b/src/config_edit/describe.rs @@ -1107,7 +1107,7 @@ fn resolve_source_endpoint<'a>( impl ConfigEditor { fn room_names(&self) -> HashMap { let mut room_names = HashMap::new(); - for e in self.iter_elements(&self.root) { + for e in Self::iter_elements(&self.root) { if e.attributes.get("Type").map(|s| s.as_str()) == Some("Place") && let (Some(u), Some(t)) = (e.attributes.get("U"), e.attributes.get("Title")) { @@ -1126,7 +1126,7 @@ impl ConfigEditor { std::collections::HashMap::new(); let mut cat_names: std::collections::HashMap = std::collections::HashMap::new(); - for e in self.iter_elements(&self.root) { + for e in Self::iter_elements(&self.root) { match e.attributes.get("Type").map(|s| s.as_str()) { Some("Place") => { if let (Some(u), Some(t)) = (e.attributes.get("U"), e.attributes.get("Title")) { @@ -1196,7 +1196,7 @@ impl ConfigEditor { "CommDMX", ]; - for e in self.iter_elements(&self.root) { + for e in Self::iter_elements(&self.root) { let etype = e.attributes.get("Type").cloned().unwrap_or_default(); if skip_types.contains(&etype.as_str()) || etype.is_empty() { continue; @@ -1307,7 +1307,7 @@ impl ConfigEditor { pub fn describe_config_structured(&self, room_filter: Option<&str>) -> Vec { // Build room UUID → name map let mut room_names: HashMap = HashMap::new(); - for e in self.iter_elements(&self.root) { + for e in Self::iter_elements(&self.root) { if e.attributes.get("Type").map(|s| s.as_str()) == Some("Place") && let (Some(u), Some(t)) = (e.attributes.get("U"), e.attributes.get("Title")) { @@ -1367,7 +1367,7 @@ impl ConfigEditor { let mut by_room: HashMap> = HashMap::new(); - for e in self.iter_elements(&self.root) { + for e in Self::iter_elements(&self.root) { let etype = e.attributes.get("Type").cloned().unwrap_or_default(); if skip_types.contains(&etype.as_str()) || etype.is_empty() { continue; @@ -1453,7 +1453,7 @@ impl ConfigEditor { let connector_map = Self::connector_map(); let mut endpoints_by_uuid: HashMap> = HashMap::new(); - for block in self.iter_elements(&self.root) { + for block in Self::iter_elements(&self.root) { if block.name != "C" { continue; } @@ -1502,7 +1502,7 @@ impl ConfigEditor { let mut wires = Vec::new(); - for block in self.iter_elements(&self.root) { + for block in Self::iter_elements(&self.root) { if block.name != "C" { continue; } @@ -1672,7 +1672,7 @@ impl ConfigEditor { // Device bus: "Tree" | "Air" | "Network" → Vec let mut device_bus: HashMap> = HashMap::new(); - for elem in self.iter_elements(&self.root) { + for elem in Self::iter_elements(&self.root) { // Count wiring: Co elements don't have Type, handle before type check if elem.name == "Co" { let in_children: Vec<&Element> = elem diff --git a/src/config_edit/mod.rs b/src/config_edit/mod.rs index 9812be47..e8931d8f 100644 --- a/src/config_edit/mod.rs +++ b/src/config_edit/mod.rs @@ -391,11 +391,11 @@ impl ConfigEditor { current } - fn iter_elements<'a>(&'a self, elem: &'a Element) -> Vec<&'a Element> { + fn iter_elements(elem: &Element) -> Vec<&Element> { let mut result = vec![elem]; for child in &elem.children { if let Some(e) = child.as_element() { - result.extend(self.iter_elements(e)); + result.extend(Self::iter_elements(e)); } } result diff --git a/src/config_edit/rooms.rs b/src/config_edit/rooms.rs index 00e5e10b..69417144 100644 --- a/src/config_edit/rooms.rs +++ b/src/config_edit/rooms.rs @@ -16,7 +16,7 @@ impl ConfigEditor { // Collect paths to matching elements let mut paths = Vec::new(); - self.collect_typed_with_iodata( + Self::collect_typed_with_iodata( &self.root, type_filter, exclude_types, @@ -45,7 +45,7 @@ impl ConfigEditor { pub fn find_room_uuid(&self, room_name: &str) -> Result { let lower = room_name.to_lowercase(); let mut found = Vec::new(); - self.walk_rooms(&self.root, &lower, &mut found); + Self::walk_rooms(&self.root, &lower, &mut found); match found.len() { 0 => bail!("Room '{}' not found in config", room_name), 1 => Ok(found.into_iter().next().unwrap().0), @@ -68,7 +68,7 @@ impl ConfigEditor { } } - fn walk_rooms(&self, elem: &Element, name_lower: &str, found: &mut Vec<(String, String)>) { + fn walk_rooms(elem: &Element, name_lower: &str, found: &mut Vec<(String, String)>) { if elem.name == "C" && let Some(t) = elem.attributes.get("Type") && t == "Place" @@ -80,7 +80,7 @@ impl ConfigEditor { } for child in &elem.children { if let Some(child_elem) = child.as_element() { - self.walk_rooms(child_elem, name_lower, found); + Self::walk_rooms(child_elem, name_lower, found); } } } @@ -151,7 +151,7 @@ impl ConfigEditor { pub fn find_category_uuid(&self, cat_name: &str) -> Result { let lower = cat_name.to_lowercase(); let mut found = Vec::new(); - self.walk_categories(&self.root, &lower, &mut found); + Self::walk_categories(&self.root, &lower, &mut found); match found.len() { 0 => bail!("Category '{}' not found in config", cat_name), 1 => Ok(found.into_iter().next().unwrap().0), @@ -173,7 +173,7 @@ impl ConfigEditor { } } - fn walk_categories(&self, elem: &Element, name_lower: &str, found: &mut Vec<(String, String)>) { + fn walk_categories(elem: &Element, name_lower: &str, found: &mut Vec<(String, String)>) { if elem.name == "C" && let Some(t) = elem.attributes.get("Type") && t == "Category" @@ -185,13 +185,12 @@ impl ConfigEditor { } for child in &elem.children { if let Some(child_elem) = child.as_element() { - self.walk_categories(child_elem, name_lower, found); + Self::walk_categories(child_elem, name_lower, found); } } } fn collect_typed_with_iodata( - &self, elem: &Element, type_filter: &str, exclude_types: &[&str], @@ -215,7 +214,7 @@ impl ConfigEditor { for (i, child) in elem.children.iter().enumerate() { if let Some(child_elem) = child.as_element() { path.push(i); - self.collect_typed_with_iodata( + Self::collect_typed_with_iodata( child_elem, type_filter, exclude_types, @@ -231,7 +230,7 @@ impl ConfigEditor { pub fn add_room(&mut self, name: &str) -> Result { // Check if room already exists let mut existing = Vec::new(); - self.walk_rooms(&self.root, &name.to_lowercase(), &mut existing); + Self::walk_rooms(&self.root, &name.to_lowercase(), &mut existing); if !existing.is_empty() { bail!("Room '{}' already exists", name); } @@ -329,7 +328,7 @@ impl ConfigEditor { pub fn add_user(&mut self, name: &str) -> Result { // Check if user already exists let mut exists = false; - self.walk_users(&self.root, &mut |title| { + Self::walk_users(&self.root, &mut |title| { if title.eq_ignore_ascii_case(name) { exists = true; } @@ -377,7 +376,7 @@ impl ConfigEditor { } } - fn walk_users(&self, elem: &Element, cb: &mut dyn FnMut(&str)) { + fn walk_users(elem: &Element, cb: &mut dyn FnMut(&str)) { if elem.name == "C" && let Some(t) = elem.attributes.get("Type") && t == "User" @@ -387,20 +386,16 @@ impl ConfigEditor { } for child in &elem.children { if let Some(child_elem) = child.as_element() { - self.walk_users(child_elem, cb); + Self::walk_users(child_elem, cb); } } } fn find_user_caption(&self) -> Option> { - self.find_user_caption_recursive(&self.root, &mut Vec::new()) + Self::find_user_caption_recursive(&self.root, &mut Vec::new()) } - fn find_user_caption_recursive( - &self, - elem: &Element, - path: &mut Vec, - ) -> Option> { + fn find_user_caption_recursive(elem: &Element, path: &mut Vec) -> Option> { if elem.name == "C" && let Some(t) = elem.attributes.get("Type") && t == "UserCaption" @@ -410,7 +405,7 @@ impl ConfigEditor { for (i, child) in elem.children.iter().enumerate() { if let Some(child_elem) = child.as_element() { path.push(i); - if let Some(result) = self.find_user_caption_recursive(child_elem, path) { + if let Some(result) = Self::find_user_caption_recursive(child_elem, path) { return Some(result); } path.pop(); diff --git a/src/config_edit/selector.rs b/src/config_edit/selector.rs index 401195e5..5f5b3bc0 100644 --- a/src/config_edit/selector.rs +++ b/src/config_edit/selector.rs @@ -6,12 +6,11 @@ impl ConfigEditor { /// Find all elements matching a selector. pub fn find_elements(&self, selector: &str) -> Vec> { let mut results = Vec::new(); - self.find_recursive(&self.root, selector, &mut Vec::new(), &mut results); + Self::find_recursive(&self.root, selector, &mut Vec::new(), &mut results); results } fn find_recursive( - &self, elem: &Element, selector: &str, path: &mut Vec, @@ -23,7 +22,7 @@ impl ConfigEditor { for (i, child) in elem.children.iter().enumerate() { if let Some(child_elem) = child.as_element() { path.push(i); - self.find_recursive(child_elem, selector, path, results); + Self::find_recursive(child_elem, selector, path, results); path.pop(); } } @@ -158,7 +157,6 @@ impl ConfigEditor { #[allow(dead_code)] fn find_exact_title( - &self, elem: &Element, title: &str, path: &mut Vec, @@ -176,7 +174,7 @@ impl ConfigEditor { for (i, child) in elem.children.iter().enumerate() { if let Some(child_elem) = child.as_element() { path.push(i); - self.find_exact_title(child_elem, title, path, results); + Self::find_exact_title(child_elem, title, path, results); path.pop(); } } diff --git a/src/config_edit/validation.rs b/src/config_edit/validation.rs index 9c513f56..41ec42fb 100644 --- a/src/config_edit/validation.rs +++ b/src/config_edit/validation.rs @@ -131,7 +131,7 @@ impl ConfigEditor { // Resolve title-based wiring ("BlockTitle.ConnKey") to UUIDs so output // connectivity checks work for both UUID and title-based references. let mut title_conn_to_uuid: HashMap = HashMap::new(); - for elem in self.iter_elements(&self.root) { + for elem in Self::iter_elements(&self.root) { if let Some(title) = elem.attributes.get("Title") { for child in &elem.children { if let Some(co) = child.as_element() @@ -538,7 +538,7 @@ impl ConfigEditor { if selector.is_none() { const PROGRAM_LIMIT: usize = 8; let mut programs: Vec = Vec::new(); - for elem in self.iter_elements(&self.root) { + for elem in Self::iter_elements(&self.root) { if matches!( elem.attributes.get("Type").map(|s| s.as_str()), Some("Code1" | "Code4" | "Code8" | "Code16") @@ -574,17 +574,17 @@ impl ConfigEditor { // Collect all Place UUIDs let mut place_uuids = std::collections::HashSet::new(); - self.collect_typed_uuids(&self.root, "Place", &mut place_uuids); + Self::collect_typed_uuids(&self.root, "Place", &mut place_uuids); // Collect all Category UUIDs let mut category_uuids = std::collections::HashSet::new(); - self.collect_typed_uuids(&self.root, "Category", &mut category_uuids); + Self::collect_typed_uuids(&self.root, "Category", &mut category_uuids); // Check IoData references let mut bad_rooms = Vec::new(); let mut bad_cats = Vec::new(); let mut unconnected = Vec::new(); - self.validate_recursive( + Self::validate_recursive( &self.root, &place_uuids, &category_uuids, @@ -965,7 +965,6 @@ impl ConfigEditor { } fn collect_typed_uuids( - &self, elem: &Element, type_name: &str, uuids: &mut std::collections::HashSet, @@ -979,13 +978,12 @@ impl ConfigEditor { } for child in &elem.children { if let Some(child_elem) = child.as_element() { - self.collect_typed_uuids(child_elem, type_name, uuids); + Self::collect_typed_uuids(child_elem, type_name, uuids); } } } fn validate_recursive( - &self, elem: &Element, places: &std::collections::HashSet, categories: &std::collections::HashSet, @@ -1016,7 +1014,7 @@ impl ConfigEditor { } for child in &elem.children { if let Some(child_elem) = child.as_element() { - self.validate_recursive( + Self::validate_recursive( child_elem, places, categories, diff --git a/src/config_edit/wiring.rs b/src/config_edit/wiring.rs index 6b324da6..fafeca1f 100644 --- a/src/config_edit/wiring.rs +++ b/src/config_edit/wiring.rs @@ -404,11 +404,11 @@ impl ConfigEditor { /// List all MQTT topics (GenTSensor subscriptions + GenTActor publishes). pub fn list_mqtt_topics(&self) -> Vec { let mut topics = Vec::new(); - self.collect_mqtt_topics(&self.root, &mut topics); + Self::collect_mqtt_topics(&self.root, &mut topics); topics } - fn collect_mqtt_topics(&self, elem: &Element, topics: &mut Vec) { + fn collect_mqtt_topics(elem: &Element, topics: &mut Vec) { if elem.name == "C" && let Some(t) = elem.attributes.get("Type") && (t == "GenTSensor" || t == "GenTActor") @@ -449,7 +449,7 @@ impl ConfigEditor { } for child in &elem.children { if let Some(child_elem) = child.as_element() { - self.collect_mqtt_topics(child_elem, topics); + Self::collect_mqtt_topics(child_elem, topics); } } } From d6401ac2af3caffd693bd83d718595337c89bed1 Mon Sep 17 00:00:00 2001 From: Markus Cozowicz Date: Tue, 8 Sep 2026 18:44:41 +0100 Subject: [PATCH 4/4] fix(clippy): satisfy Rust 1.98 lints Use array chunks and Option::filter as required by the current stable Clippy used in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/config_edit/wiring.rs | 27 ++++++--------------------- src/rc6.rs | 6 ++---- 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/src/config_edit/wiring.rs b/src/config_edit/wiring.rs index fafeca1f..66df6397 100644 --- a/src/config_edit/wiring.rs +++ b/src/config_edit/wiring.rs @@ -276,17 +276,12 @@ impl ConfigEditor { .children .iter_mut() .find_map(|c| { - c.as_mut_element().and_then(|e| { - if e.name == "Co" + c.as_mut_element().filter(|e| { + e.name == "Co" && e.attributes .get("K") .map(|k| k == source_connector) .unwrap_or(false) - { - Some(e) - } else { - None - } }) }) .ok_or_else(|| { @@ -318,17 +313,12 @@ impl ConfigEditor { .children .iter_mut() .find_map(|c| { - c.as_mut_element().and_then(|e| { - if e.name == "Co" + c.as_mut_element().filter(|e| { + e.name == "Co" && e.attributes .get("K") .map(|k| k == connector_name) .unwrap_or(false) - { - Some(e) - } else { - None - } }) }) .ok_or_else(|| { @@ -863,17 +853,12 @@ impl ConfigEditor { .children .iter_mut() .find_map(|c| { - c.as_mut_element().and_then(|e| { - if e.name == "Co" + c.as_mut_element().filter(|e| { + e.name == "Co" && e.attributes .get("K") .map(|k| k == conn_key) .unwrap_or(false) - { - Some(e) - } else { - None - } }) }) .ok_or_else(|| { diff --git a/src/rc6.rs b/src/rc6.rs index 15fa34bc..cc04f878 100644 --- a/src/rc6.rs +++ b/src/rc6.rs @@ -103,8 +103,7 @@ impl Rc6Key { data.len().is_multiple_of(16), "data must be a multiple of 16 bytes" ); - for chunk in data.chunks_exact_mut(16) { - let block: &mut [u8; 16] = chunk.try_into().unwrap(); + for block in data.as_chunks_mut::<16>().0 { self.encrypt_block(block); } } @@ -115,8 +114,7 @@ impl Rc6Key { data.len().is_multiple_of(16), "data must be a multiple of 16 bytes" ); - for chunk in data.chunks_exact_mut(16) { - let block: &mut [u8; 16] = chunk.try_into().unwrap(); + for block in data.as_chunks_mut::<16>().0 { self.decrypt_block(block); } }