From e009d2d7a3361fd1e48efb208d79f3df109cf832 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Wed, 22 Jul 2026 11:11:20 +0300 Subject: [PATCH] fix(rename): BLOCK type, stepped prompts, table re-keying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - RENAME gains the BLOCK type (stepped button + inline arm) via the shared Scene::rename_block. - Stepped command front-ends (Rename, UserReg, Keyword, TwoValue, SelectThenTwoValue) returned None after consuming a typed token, and the driver treats None as "not consumed" — the same text was offered again and advanced two steps at once (RENAME dispatched old==new, SETVAR "X" became "SETVAR X X"). Consumed-but-prompting inputs now return NeedPoint. - Symbol renames re-key the name-keyed Table (remove -> rename -> add) instead of mutating .name in place, which left the map keyed by the old name so later lookups silently failed. References (entity layer/linetype/style fields, layer linetypes, header current-* names, dim style refs on Dimension/Leader/Tolerance) are chased case-insensitively; layer 0/Defpoints and the built-in linetypes are protected; the undo snapshot is taken before the mutation so undo actually restores the old name. Co-Authored-By: Claude Fable 5 --- src/app/commands/styleprops.rs | 255 ++++++++++++++++++++++++--------- src/command.rs | 50 ++++--- 2 files changed, 220 insertions(+), 85 deletions(-) diff --git a/src/app/commands/styleprops.rs b/src/app/commands/styleprops.rs index 0cc8aea79..82c0e5c7a 100644 --- a/src/app/commands/styleprops.rs +++ b/src/app/commands/styleprops.rs @@ -1543,77 +1543,64 @@ impl OpenCADStudio { "Usage: RENAME (types: LAYER BLOCK STYLE DIMSTYLE LINETYPE UCS VIEW)" ); } else { - let doc = &mut self.tabs[i].scene.document; - let ok = match type_str.as_str() { - "LAYER" => { - if let Some(l) = doc.layers.get_mut(&old_name) { - l.name = new_name.clone(); - // Update entity references - for e in doc.entities_mut() { - if e.common().layer == old_name { - e.common_mut().layer = new_name.clone(); - } - } - true - } else { - false - } - } - "STYLE" | "TEXTSTYLE" => { - if let Some(s) = doc.text_styles.get_mut(&old_name) { - s.name = new_name.clone(); - true - } else { - false - } - } - "DIMSTYLE" => { - if let Some(s) = doc.dim_styles.get_mut(&old_name) { - s.name = new_name.clone(); - true - } else { - false - } - } - "LINETYPE" | "LT" => { - if let Some(lt) = doc.line_types.get_mut(&old_name) { - lt.name = new_name.clone(); - true - } else { - false - } - } - "UCS" => { - if let Some(u) = doc.ucss.get_mut(&old_name) { - u.name = new_name.clone(); - true - } else { - false - } - } - "VIEW" => { - if let Some(v) = doc.views.get_mut(&old_name) { - v.name = new_name.clone(); - true - } else { - false - } - } - _ => { - self.command_line.push_error(&format!("RENAME: unknown type '{}'. Use LAYER BLOCK STYLE DIMSTYLE LINETYPE UCS VIEW", type_str)); - false - } + // Snapshot BEFORE mutating (undo must restore the old + // name); dropped again if nothing was renamed. + self.push_undo_snapshot(i, "RENAME"); + let known = matches!( + type_str.as_str(), + "LAYER" + | "BLOCK" + | "STYLE" + | "TEXTSTYLE" + | "DIMSTYLE" + | "LINETYPE" + | "LT" + | "UCS" + | "VIEW" + ); + let ok = if type_str == "BLOCK" { + // Re-keys the record, syncs the Block marker and every + // INSERT reference; refuses anonymous/xref sources and + // invalid/taken names. + self.tabs[i].scene.rename_block(&old_name, &new_name) + } else if known { + rename_symbol( + &mut self.tabs[i].scene.document, + &type_str, + &old_name, + &new_name, + ) + } else { + false }; if ok { - self.push_undo_snapshot(i, "RENAME"); + // Renamed references resolve at tessellation time — + // rebuild so nothing renders off a stale name. + self.tabs[i].scene.bump_geometry_no_blocks(); + // The per-tab active layer tracks the name. + if type_str == "LAYER" + && self.tabs[i].active_layer.eq_ignore_ascii_case(&old_name) + { + self.tabs[i].active_layer = new_name.clone(); + } self.tabs[i].dirty = true; self.command_line .push_output(&format!("RENAME: '{}' → '{}'.", old_name, new_name)); - } else if type_str != "BLOCK" { - self.command_line.push_error(&format!( - "RENAME: '{}' not found in {}.", - old_name, type_str - )); + } else { + let _ = self.tabs[i].history.undo_stack.pop(); + if !known { + self.command_line.push_error(&format!("RENAME: unknown type '{}'. Use LAYER BLOCK STYLE DIMSTYLE LINETYPE UCS VIEW", type_str)); + } else if type_str == "BLOCK" { + self.command_line.push_error(&format!( + "RENAME: could not rename block '{}' — missing/anonymous/xref, or '{}' is invalid or taken.", + old_name, new_name + )); + } else { + self.command_line.push_error(&format!( + "RENAME: could not rename '{}' in {} — not found or protected, or '{}' is invalid or taken.", + old_name, type_str, new_name + )); + } } } } @@ -2035,3 +2022,139 @@ impl OpenCADStudio { Some(self.finish_dispatch(cmd)) } } + +/// RENAME for the name-keyed symbol tables (layer / text style / dim style / +/// linetype / UCS / view). `Table` stores entries in a map keyed by +/// UPPERCASE name, so a rename must remove + re-add — mutating `.name` in +/// place leaves the map keyed by the old name and every later +/// `get(new_name)` lookup silently fails. Name-based references (entity +/// layer/linetype/style fields, layer linetypes, header current-* names) are +/// chased case-insensitively, matching how the tables resolve names. +fn rename_symbol(doc: &mut acadrust::CadDocument, ty: &str, old: &str, new: &str) -> bool { + use acadrust::{EntityType, Table, TableEntry}; + + fn rekey(table: &mut Table, old: &str, new: &str) -> bool { + if !crate::scene::valid_block_name(new) { + return false; + } + // A case-only rename reuses its own key; any other target must be free. + if !old.eq_ignore_ascii_case(new) && table.contains(new) { + return false; + } + let Some(mut e) = table.remove(old) else { + return false; + }; + e.set_name(new.to_string()); + let _ = table.add(e); + true + } + + match ty { + "LAYER" => { + // Layer 0 and Defpoints are fixed names; xref-dependent ("|") + // layers belong to the referenced file. + if old == "0" || old.eq_ignore_ascii_case("Defpoints") || old.contains('|') { + return false; + } + if !rekey(&mut doc.layers, old, new) { + return false; + } + for e in doc.entities_mut() { + if e.common().layer.eq_ignore_ascii_case(old) { + e.common_mut().layer = new.to_string(); + } + } + if doc.header.current_layer_name.eq_ignore_ascii_case(old) { + doc.header.current_layer_name = new.to_string(); + } + true + } + "STYLE" | "TEXTSTYLE" => { + if !rekey(&mut doc.text_styles, old, new) { + return false; + } + for e in doc.entities_mut() { + match e { + EntityType::Text(t) if t.style.eq_ignore_ascii_case(old) => { + t.style = new.to_string(); + } + EntityType::MText(m) if m.style.eq_ignore_ascii_case(old) => { + m.style = new.to_string(); + } + EntityType::AttributeDefinition(a) + if a.text_style.eq_ignore_ascii_case(old) => + { + a.text_style = new.to_string(); + } + EntityType::Insert(ins) => { + for a in &mut ins.attributes { + if a.text_style.eq_ignore_ascii_case(old) { + a.text_style = new.to_string(); + } + } + } + _ => {} + } + } + if doc.header.current_text_style_name.eq_ignore_ascii_case(old) { + doc.header.current_text_style_name = new.to_string(); + } + true + } + "DIMSTYLE" => { + if !rekey(&mut doc.dim_styles, old, new) { + return false; + } + for e in doc.entities_mut() { + match e { + EntityType::Dimension(d) + if d.base().style_name.eq_ignore_ascii_case(old) => + { + d.base_mut().style_name = new.to_string(); + } + EntityType::Leader(l) if l.dimension_style.eq_ignore_ascii_case(old) => { + l.dimension_style = new.to_string(); + } + EntityType::Tolerance(t) + if t.dimension_style_name.eq_ignore_ascii_case(old) => + { + t.dimension_style_name = new.to_string(); + } + _ => {} + } + } + if doc.header.current_dimstyle_name.eq_ignore_ascii_case(old) { + doc.header.current_dimstyle_name = new.to_string(); + } + true + } + "LINETYPE" | "LT" => { + // The three built-ins are fixed names every drawing relies on. + if ["BYLAYER", "BYBLOCK", "CONTINUOUS"] + .contains(&old.to_uppercase().as_str()) + { + return false; + } + if !rekey(&mut doc.line_types, old, new) { + return false; + } + for e in doc.entities_mut() { + if e.common().linetype.eq_ignore_ascii_case(old) { + e.common_mut().linetype = new.to_string(); + } + } + for l in doc.layers.iter_mut() { + if l.line_type.eq_ignore_ascii_case(old) { + l.line_type = new.to_string(); + } + } + if doc.header.current_linetype_name.eq_ignore_ascii_case(old) { + doc.header.current_linetype_name = new.to_string(); + } + true + } + "UCS" => rekey(&mut doc.ucss, old, new), + "VIEW" => rekey(&mut doc.views, old, new), + _ => false, + } +} diff --git a/src/command.rs b/src/command.rs index 637ced4be..696998a57 100644 --- a/src/command.rs +++ b/src/command.rs @@ -148,10 +148,10 @@ enum RenameStep { impl RenameCommand { /// `(button label, keyword)` for the object types the inline handler can - /// actually rename. BLOCK is intentionally omitted — the inline handler - /// does not implement it, so offering the button would do nothing. - const TYPES: [(&'static str, &'static str); 6] = [ + /// actually rename. + const TYPES: [(&'static str, &'static str); 7] = [ ("Layer", "LAYER"), + ("Block", "BLOCK"), ("Text style", "STYLE"), ("Dim style", "DIMSTYLE"), ("Linetype", "LINETYPE"), @@ -200,30 +200,35 @@ impl CadCommand for RenameCommand { // Keep prompting for the current step. return None; } + // NOTE: a consumed-but-still-prompting input must return + // `Some(NeedPoint)` (reprint the prompt), NOT `None` — the driver + // treats `None` as "not consumed" and offers the same text to the + // command a second time, which would advance two steps at once. match &self.step { RenameStep::Type => { // Accept a known type (or a common alias); re-prompt otherwise so // a typo doesn't advance to a rename that can't happen. let canonical = match t.to_uppercase().as_str() { "LAYER" | "LA" => "LAYER", + "BLOCK" | "B" => "BLOCK", "STYLE" | "TEXTSTYLE" | "ST" => "STYLE", "DIMSTYLE" | "D" => "DIMSTYLE", "LINETYPE" | "LT" => "LINETYPE", "UCS" => "UCS", "VIEW" | "V" => "VIEW", - _ => return None, + _ => return Some(CmdResult::NeedPoint), }; self.step = RenameStep::Old { ty: canonical.to_string(), }; - None + Some(CmdResult::NeedPoint) } RenameStep::Old { ty } => { self.step = RenameStep::New { ty: ty.clone(), old: t.to_string(), }; - None + Some(CmdResult::NeedPoint) } RenameStep::New { ty, old } => { Some(CmdResult::Dispatch(format!("RENAME {ty} {old} {t}"))) @@ -289,14 +294,13 @@ impl CadCommand for UserRegCommand { } match self.slot { None => { - // Accept a register 1–5; re-prompt on anything else. - match t.parse::() { - Ok(n @ 1..=5) => { - self.slot = Some(n); - None - } - _ => None, + // Accept a register 1–5; re-prompt on anything else. Consumed + // either way (`Some(NeedPoint)`) — `None` would feed the same + // text to the command a second time. + if let Ok(n @ 1..=5) = t.parse::() { + self.slot = Some(n); } + Some(CmdResult::NeedPoint) } // The inline handler validates the value (int vs real) and reports // usage if it doesn't parse, so just hand the whole line over. @@ -381,18 +385,20 @@ impl CadCommand for KeywordCommand { // Second step: the argument for the already-chosen verb. Some((keyword, _)) => Some(CmdResult::Dispatch(format!("{} {keyword} {t}", self.name))), // First step: match the typed / clicked token to a sub-verb. + // Consumed inputs that keep prompting return `Some(NeedPoint)` — + // `None` would hand the same text to the command a second time. None => { let up = t.to_uppercase(); let Some((_, keyword, value_prompt)) = self.options.iter().find(|(_, k, _)| k.eq_ignore_ascii_case(&up)) else { // Unknown verb — keep prompting rather than dispatch garbage. - return None; + return Some(CmdResult::NeedPoint); }; match value_prompt { Some(vp) => { self.pending = Some((keyword, vp)); - None + Some(CmdResult::NeedPoint) } None => Some(CmdResult::Dispatch(format!("{} {keyword}", self.name))), } @@ -456,7 +462,9 @@ impl CadCommand for TwoValuePromptCommand { return None; } self.first = Some(t.to_string()); - None + // Consumed; `None` here would feed the same text back as the + // second value and dispatch ` ` in one step. + Some(CmdResult::NeedPoint) } Some(first) => Some(CmdResult::Dispatch(if t.is_empty() { format!("{} {first}", self.name) @@ -577,12 +585,14 @@ impl CadCommand for SelectThenKeywordCommand { let Some((_, keyword, value_prompt)) = self.options.iter().find(|(_, k, _)| k.eq_ignore_ascii_case(&up)) else { - return None; + // Unknown verb — consumed, keep prompting (`None` would + // feed the same text to the command a second time). + return Some(CmdResult::NeedPoint); }; match value_prompt { Some(vp) => { self.pending = Some((keyword, vp)); - None + Some(CmdResult::NeedPoint) } None => Some(CmdResult::Dispatch(format!("{} {keyword}", self.name))), } @@ -743,7 +753,9 @@ impl CadCommand for SelectThenTwoValueCommand { match &self.first { None => { self.first = Some(t.to_string()); - None + // Consumed; `None` would feed the same text back as the second + // value and dispatch ` ` in one step. + Some(CmdResult::NeedPoint) } Some(first) => Some(CmdResult::Dispatch(format!("{} {first} {t}", self.name))), }