forked from 1b-agent/opencadstudio-upstream
feat(attribute): route ATTMAN/BATTMAN to the attribute editor (ATTEDIT)
ATTMAN and BATTMAN previously produced a command-line listing of block attributes. Route both to the attribute editor instead: extract the bare ATTEDIT behavior into a shared open_attedit_dialog helper (edit the selected block, or start the block-pick command) and call it from the ATTEDIT, ATTMAN, and BATTMAN arms. Kept as a match arm rather than an ocad.pgp alias so it works for existing users, whose config pgp is only seeded on first run. Replace the listing regression tests with one asserting ATTMAN and BATTMAN behave byte-for-byte like ATTEDIT. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
Hakan SevenandClaude Opus 4.8 committed
1 parent
bd6a8845b4
commit
2a72227d5d
3 files changed
+53
-135
No files matched your search
+1
-1
@@ -194,7 +194,7 @@ Status of every standard CAD command in Open CAD Studio:
|
||||
| `ATTEXT` | — | Extract attributes (legacy) | ✅ |
|
||||
| `ATTSYNC` | — | Synchronize attribute definitions | ✅ |
|
||||
| `BLOCKPALETTE` | — | Multi-view block palette (command-line list) | 🔶 |
|
||||
| `ATTMAN` | `BATTMAN` | Attribute manager (command-line list) | 🔶 |
|
||||
| `ATTMAN` | `BATTMAN` | Block attribute manager (opens the attribute editor) | ✅ |
|
||||
| `XBIND` | XB | Bind xref elements to drawing | ❌ |
|
||||
| `XOPEN` | — | Open xref for editing | ✅ |
|
||||
| `BSAVE` | — | Save block in editor | ❌ |
|
||||
|
||||
+20
-112
@@ -427,64 +427,15 @@ impl OpenCADStudio {
|
||||
}
|
||||
}
|
||||
|
||||
// ATTMAN — list the attribute definitions of each block (or one named
|
||||
// block). The full graphical manager is not built; this reports the
|
||||
// same information from the command line.
|
||||
// ATTMAN / BATTMAN — the Block Attribute Manager. Rather than a
|
||||
// command-line listing, both route to the attribute editor
|
||||
// (ATTEDIT): edit the selected block's attributes, or pick a block.
|
||||
cmd if cmd == "ATTMAN"
|
||||
|| cmd == "BATTMAN"
|
||||
|| cmd.starts_with("ATTMAN ")
|
||||
|| cmd.starts_with("BATTMAN ") =>
|
||||
{
|
||||
// BATTMAN is an alias for ATTMAN. Strip whichever keyword was
|
||||
// typed so the remainder is the optional block-name argument;
|
||||
// check BATTMAN first since ATTMAN is a suffix of it.
|
||||
let arg = cmd
|
||||
.strip_prefix("BATTMAN")
|
||||
.or_else(|| cmd.strip_prefix("ATTMAN"))
|
||||
.unwrap_or("")
|
||||
.trim();
|
||||
let blocks = self.tabs[i].scene.custom_block_names();
|
||||
let targets: Vec<String> = if arg.is_empty() {
|
||||
blocks
|
||||
} else {
|
||||
blocks
|
||||
.into_iter()
|
||||
.filter(|b| b.eq_ignore_ascii_case(arg))
|
||||
.collect()
|
||||
};
|
||||
if targets.is_empty() {
|
||||
self.command_line
|
||||
.push_info("ATTMAN: no matching block. Usage: ATTMAN [block name]");
|
||||
} else {
|
||||
let doc = &self.tabs[i].scene.document;
|
||||
let mut lines: Vec<String> = Vec::new();
|
||||
for block in &targets {
|
||||
let attdefs: Vec<String> = doc
|
||||
.block_records
|
||||
.get(block)
|
||||
.map(|br| {
|
||||
br.entity_handles
|
||||
.iter()
|
||||
.filter_map(|h| doc.get_entity(*h))
|
||||
.filter_map(|e| match e {
|
||||
acadrust::EntityType::AttributeDefinition(a) => {
|
||||
Some(format!("{}={}", a.tag, a.default_value))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default();
|
||||
lines.push(if attdefs.is_empty() {
|
||||
format!(" {block}: no attributes.")
|
||||
} else {
|
||||
format!(" {block}: {}", attdefs.join(", "))
|
||||
});
|
||||
}
|
||||
for l in lines {
|
||||
self.command_line.push_output(&l);
|
||||
}
|
||||
}
|
||||
self.open_attedit_dialog();
|
||||
}
|
||||
|
||||
"XATTACH" => {
|
||||
@@ -653,30 +604,9 @@ impl OpenCADStudio {
|
||||
mod tests {
|
||||
use crate::app::OpenCADStudio;
|
||||
|
||||
/// Build a headless app with a single custom block "MYBLK" carrying one
|
||||
/// attribute definition (COST=0), so ATTMAN/BATTMAN have something to list.
|
||||
fn app_with_attributed_block() -> OpenCADStudio {
|
||||
fn fresh_app() -> OpenCADStudio {
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
app.automation_op(r#"{"op":"new"}"#);
|
||||
|
||||
let i = app.active_tab;
|
||||
let doc = &mut app.tabs[i].scene.document;
|
||||
let br_h = acadrust::Handle::new(doc.next_handle());
|
||||
let mut br = acadrust::tables::BlockRecord::new("MYBLK");
|
||||
br.handle = br_h;
|
||||
doc.block_records.add(br).unwrap();
|
||||
|
||||
let mut ad = acadrust::entities::AttributeDefinition {
|
||||
tag: "COST".into(),
|
||||
prompt: "Cost".into(),
|
||||
default_value: "0".into(),
|
||||
insertion_point: acadrust::types::Vector3::new(0.0, 0.0, 0.0),
|
||||
height: 1.0,
|
||||
..Default::default()
|
||||
};
|
||||
ad.common.owner_handle = br_h;
|
||||
doc.add_entity(acadrust::EntityType::AttributeDefinition(ad))
|
||||
.unwrap();
|
||||
app
|
||||
}
|
||||
|
||||
@@ -691,49 +621,27 @@ mod tests {
|
||||
.join("\n")
|
||||
}
|
||||
|
||||
// Regression for #295: BATTMAN triggered the ATTMAN handler but the argument
|
||||
// was extracted with `trim_start_matches("ATTMAN")`, which leaves "BATTMAN"
|
||||
// as a bogus block-name argument — so BATTMAN always reported
|
||||
// "ATTMAN: no matching block" instead of listing attributes like ATTMAN.
|
||||
// ATTMAN and BATTMAN are the Block Attribute Manager; this build routes both
|
||||
// to the attribute editor (ATTEDIT) instead of the old command-line listing.
|
||||
// With nothing selected they start the same interactive pick as ATTEDIT,
|
||||
// byte for byte, and never emit the old "no matching block" listing text.
|
||||
#[test]
|
||||
fn battman_is_a_working_alias_for_attman() {
|
||||
let attman_out = run_capture(&mut app_with_attributed_block(), "ATTMAN");
|
||||
let battman_out = run_capture(&mut app_with_attributed_block(), "BATTMAN");
|
||||
fn attman_and_battman_route_to_attedit() {
|
||||
let attedit = run_capture(&mut fresh_app(), "ATTEDIT");
|
||||
let attman = run_capture(&mut fresh_app(), "ATTMAN");
|
||||
let battman = run_capture(&mut fresh_app(), "BATTMAN");
|
||||
|
||||
// ATTMAN lists the block; the reported bug is that BATTMAN did not.
|
||||
assert!(
|
||||
attman_out.contains("MYBLK"),
|
||||
"ATTMAN should list the block, got: {attman_out:?}"
|
||||
);
|
||||
assert!(
|
||||
battman_out.contains("MYBLK"),
|
||||
"BATTMAN should list the block like ATTMAN, got: {battman_out:?}"
|
||||
);
|
||||
assert!(
|
||||
!battman_out.contains("no matching block"),
|
||||
"BATTMAN must not fall into the no-argument error path, got: {battman_out:?}"
|
||||
);
|
||||
// The alias must behave identically, attribute listing included.
|
||||
assert_eq!(
|
||||
attman_out, battman_out,
|
||||
"BATTMAN output must match ATTMAN output exactly"
|
||||
attman, attedit,
|
||||
"ATTMAN must behave like ATTEDIT, got: {attman:?} vs {attedit:?}"
|
||||
);
|
||||
}
|
||||
|
||||
// The keyword-stripping fix must also honour an explicit block-name argument
|
||||
// on BATTMAN (`BATTMAN <name>`), which the old code never matched at all.
|
||||
#[test]
|
||||
fn battman_honours_a_block_name_argument() {
|
||||
let hit = run_capture(&mut app_with_attributed_block(), "BATTMAN MYBLK");
|
||||
assert!(
|
||||
hit.contains("MYBLK") && !hit.contains("no matching block"),
|
||||
"BATTMAN MYBLK should list that block, got: {hit:?}"
|
||||
assert_eq!(
|
||||
battman, attedit,
|
||||
"BATTMAN must behave like ATTEDIT, got: {battman:?} vs {attedit:?}"
|
||||
);
|
||||
|
||||
let miss = run_capture(&mut app_with_attributed_block(), "BATTMAN NOPE");
|
||||
assert!(
|
||||
miss.contains("no matching block"),
|
||||
"BATTMAN NOPE should report no match, got: {miss:?}"
|
||||
!attman.contains("no matching block"),
|
||||
"ATTMAN must not run the old command-line listing, got: {attman:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
+32
-22
@@ -258,28 +258,7 @@ impl OpenCADStudio {
|
||||
// that block; otherwise the pick command runs and the editor opens
|
||||
// once a block is chosen (see `command_driver`).
|
||||
"ATTEDIT" => {
|
||||
let selected_attr_insert = {
|
||||
let sel = self.tabs[i].scene.selected_entities();
|
||||
if sel.len() == 1 {
|
||||
let (h, e) = sel[0];
|
||||
match e {
|
||||
acadrust::EntityType::Insert(ins) if !ins.attributes.is_empty() => {
|
||||
Some(h)
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
};
|
||||
if let Some(handle) = selected_attr_insert {
|
||||
self.open_attribute_editor(handle);
|
||||
} else {
|
||||
use crate::modules::draw::modify::attedit::AtteditCommand;
|
||||
let cmd_obj = AtteditCommand::new();
|
||||
self.command_line.push_info(&cmd_obj.prompt());
|
||||
self.tabs[i].active_cmd = Some(Box::new(cmd_obj));
|
||||
}
|
||||
self.open_attedit_dialog();
|
||||
}
|
||||
|
||||
// ── REFEDIT — in-place block editing ─────────────────────────────
|
||||
@@ -1250,6 +1229,37 @@ fn arith_eval(expr: &str) -> Result<f64, String> {
|
||||
Ok(v)
|
||||
}
|
||||
|
||||
impl OpenCADStudio {
|
||||
/// Open the attribute editor. If a single block with attributes is already
|
||||
/// selected it opens directly on that block; otherwise it starts the
|
||||
/// block-pick command and opens once a block is chosen (see
|
||||
/// `command_driver`). Shared by ATTEDIT and its command aliases
|
||||
/// ATTMAN / BATTMAN.
|
||||
pub(in crate::app) fn open_attedit_dialog(&mut self) {
|
||||
let i = self.active_tab;
|
||||
let selected_attr_insert = {
|
||||
let sel = self.tabs[i].scene.selected_entities();
|
||||
if sel.len() == 1 {
|
||||
let (h, e) = sel[0];
|
||||
match e {
|
||||
acadrust::EntityType::Insert(ins) if !ins.attributes.is_empty() => Some(h),
|
||||
_ => None,
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
};
|
||||
if let Some(handle) = selected_attr_insert {
|
||||
self.open_attribute_editor(handle);
|
||||
} else {
|
||||
use crate::modules::draw::modify::attedit::AtteditCommand;
|
||||
let cmd_obj = AtteditCommand::new();
|
||||
self.command_line.push_info(&cmd_obj.prompt());
|
||||
self.tabs[i].active_cmd = Some(Box::new(cmd_obj));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
struct ArithParser {
|
||||
chars: Vec<char>,
|
||||
pos: usize,
|
||||
|
||||
Reference in new issue
Block a user