diff --git a/crates/engine/src/cmd/file/pgp.rs b/crates/engine/src/cmd/file/pgp.rs --- a/crates/engine/src/cmd/file/pgp.rs +++ b/crates/engine/src/cmd/file/pgp.rs @@ -63,8 +63,8 @@ pub fn alias_name(a: &str) -> std::result::Result { } /// The aliases of a `.pgp` file's text, and its problems. -fn parse(text: &str) -> (Vec<(String, String)>, Vec) { - let mut out: Vec<(String, String)> = Vec::new(); +fn parse(text: &str) -> (BTreeMap, Vec) { + let mut out = BTreeMap::new(); let mut warnings = Vec::new(); let mut warn = |no: usize, msg: String| { if warnings.len() < MAX_WARNINGS { @@ -100,13 +100,13 @@ fn parse(text: &str) -> (Vec<(String, String)>, Vec) { warn(no, format!("{}: unknown command {}; skipped", alias.trim(), target.trim())); continue; }; + // Once full, stop at the next valid definition, even if it repeats an alias. if out.len() >= MAX_ALIASES { warn(no, format!("more than {MAX_ALIASES} aliases; the rest are skipped")); break; } - // A later line for the same alias wins, as it would when typed into the file twice. - out.retain(|(a, _)| *a != name); - out.push((name, id)); + // A later definition wins without scanning all the aliases accumulated so far. + out.insert(name, id); } (out, warnings) } @@ -120,7 +120,7 @@ fn listing(warnings: &[String], added: usize) -> Value { fn load(text: &str, path: Option) -> (usize, Vec) { let (aliases, warnings) = parse(text); let mut st = store(); - st.map = aliases.into_iter().collect(); + st.map = aliases; if path.is_some() { st.file = path; } diff --git a/crates/engine/tests/aliases.rs b/crates/engine/tests/aliases.rs --- a/crates/engine/tests/aliases.rs +++ b/crates/engine/tests/aliases.rs @@ -5,6 +5,7 @@ use std::sync::Mutex; use cadcraft_engine::Session; +use cadcraft_engine::cmd::file::MAX_ALIASES; use cadcraft_engine::cmd::resolve_alias; use serde_json::json; @@ -21,6 +22,10 @@ ZZBAD, *NOSUCHCOMMAND LINE, *CIRCLE "; +fn alias_prefix(count: usize) -> String { + (0..count).map(|i| format!("ZZP{i:05},*LINE\n")).collect() +} + #[test] fn an_alias_file_adds_and_overrides_aliases() { let _one = ONE_AT_A_TIME.lock().unwrap_or_else(|e| e.into_inner()); @@ -58,17 +63,142 @@ fn reinit_reads_the_alias_file_again() { let dir = std::env::temp_dir().join(format!("cadcraft-aliases-{}", std::process::id())); std::fs::create_dir_all(&dir).unwrap(); let file = dir.join("mine.pgp").to_string_lossy().into_owned(); - std::fs::write(&file, "ZZR, *RECTANG\n").unwrap(); + std::fs::write(&file, "ZZR, *LINE\nzzr, *RECTANG\n").unwrap(); let mut s = Session::new(); - s.execute("aliasedit", &json!({ "file": file })).unwrap(); + s.execute("aliasedit", &json!({ "clear": true })).unwrap(); + let r = s.execute("aliasedit", &json!({ "file": file })).unwrap(); + assert_eq!(r["loaded"], json!(1)); + assert_eq!(r["file"], json!(file)); + assert_eq!(r["warnings"], json!([])); assert_eq!(resolve_alias("zzr"), "rectang"); - std::fs::write(&file, "ZZR, *CIRCLE\n").unwrap(); + std::fs::write(&file, "ZZR, *MOVE\nzzr, *CIRCLE\n").unwrap(); s.cmdline("reinit").unwrap(); assert_eq!(resolve_alias("zzr"), "circle"); assert!(s.log.iter().any(|l| l.contains("1 command alias")), "{:?}", s.log); - let r = s.execute("reinit", &json!({ "text": "ZZS, *SCALE\n" })).unwrap(); + let r = s.execute("aliasedit", &json!({})).unwrap(); + assert_eq!(r["file"], json!(file)); + assert_eq!(r["aliases"], json!({ "zzr": "circle" })); + let r = s.execute("reinit", &json!({ "text": "ZZS, *LINE\nzzs, *SCALE\n" })).unwrap(); assert_eq!(r["aliases"], json!({ "zzs": "scale" })); + assert_eq!(r["loaded"], json!(1)); + assert_eq!(r["warnings"], json!([])); + assert_eq!(r["file"], json!(file), "reloading text keeps the remembered file"); + let r = s.execute("reinit", &json!({})).unwrap(); + assert_eq!(r["aliases"], json!({ "zzr": "circle" })); + assert_eq!(r["loaded"], json!(1)); + assert_eq!(r["file"], json!(file)); + s.execute("aliasedit", &json!({ "clear": true })).unwrap(); +} + +#[test] +fn valid_redefinitions_win_and_the_listing_stays_sorted() { + let _one = ONE_AT_A_TIME.lock().unwrap_or_else(|e| e.into_inner()); + let mut s = Session::new(); + s.execute("aliasedit", &json!({ "clear": true })).unwrap(); + let text = "ZZZ,*LINE\nZza,*LINE\nzzz,*CIRCLE\nZZA,*NO_SUCH_1933\nLINE,*CIRCLE\n"; + let r = s.execute("aliasedit", &json!({ "text": text })).unwrap(); + let aliases = json!({ "zza": "line", "zzz": "circle" }); + assert_eq!(r["aliases"], aliases); + assert_eq!(r["loaded"], json!(2), "loaded counts unique aliases, not definitions"); + assert_eq!(r["file"], json!(null)); + assert_eq!( + r["warnings"], + json!(["line 4: ZZA: unknown command *NO_SUCH_1933; skipped", "line 5: LINE is a command name; it stays the command; skipped"]) + ); + assert_eq!(resolve_alias("ZZZ"), "circle"); + assert_eq!(resolve_alias("zza"), "line", "an invalid redefinition keeps the previous target"); + assert_eq!(resolve_alias("line"), "line"); + assert_eq!(serde_json::to_string(&r["aliases"]).unwrap(), r#"{"zza":"line","zzz":"circle"}"#); + let listing = s.execute("aliasedit", &json!({})).unwrap(); + assert_eq!(listing, json!({ "aliases": aliases, "file": null, "loaded": 0, "warnings": [] })); + + let empty = s.execute("aliasedit", &json!({ "text": "" })).unwrap(); + assert_eq!(empty, json!({ "aliases": {}, "file": null, "loaded": 0, "warnings": [] })); + assert_eq!(resolve_alias("zza"), "zza", "an empty import replaces the old set"); + assert_eq!(resolve_alias("zzz"), "zzz"); + s.execute("aliasedit", &json!({ "clear": true })).unwrap(); +} + +#[test] +fn duplicates_below_the_limit_leave_room_for_another_alias() { + let _one = ONE_AT_A_TIME.lock().unwrap_or_else(|e| e.into_inner()); + let mut s = Session::new(); + s.execute("aliasedit", &json!({ "clear": true })).unwrap(); + let mut text = alias_prefix(MAX_ALIASES - 1); + for _ in 0..16 { + text.push_str("ZZP00000,*MOVE\nzzp00001,*CIRCLE\n"); + } + let r = s.execute("aliasedit", &json!({ "text": text })).unwrap(); + assert_eq!(r["loaded"], json!(MAX_ALIASES - 1)); + assert_eq!(r["aliases"].as_object().unwrap().len(), MAX_ALIASES - 1); + assert_eq!(r["warnings"], json!([])); + for i in 0..MAX_ALIASES - 1 { + let expected = match i { + 0 => "move", + 1 => "circle", + _ => "line", + }; + assert_eq!(r["aliases"][format!("zzp{i:05}")], json!(expected)); + } + + text.push_str("ZZNEW,*PLINE\n"); + let r = s.execute("aliasedit", &json!({ "text": text })).unwrap(); + assert_eq!(r["loaded"], json!(MAX_ALIASES)); + assert_eq!(r["aliases"].as_object().unwrap().len(), MAX_ALIASES); + assert_eq!(r["aliases"]["zznew"], json!("pline")); + assert_eq!(r["aliases"]["zzp00000"], json!("move")); + assert_eq!(r["aliases"]["zzp00001"], json!("circle")); + assert_eq!(r["warnings"], json!([]), "duplicate definitions do not use up the alias limit"); + s.execute("aliasedit", &json!({ "clear": true })).unwrap(); +} + +#[test] +fn a_full_file_validates_lines_before_stopping_at_the_next_valid_definition() { + let _one = ONE_AT_A_TIME.lock().unwrap_or_else(|e| e.into_inner()); + let mut s = Session::new(); + s.execute("aliasedit", &json!({ "clear": true })).unwrap(); + let mut text = alias_prefix(MAX_ALIASES); + text.push_str("; comment\nZZP00000,*NO_SUCH_1933\nLINE,*CIRCLE\nZZEXTERNAL, START NOTHING\nmalformed\nZZP00000,*MOVE\nZZAFTER,*CIRCLE\nanother malformed sentinel\n"); + let r = s.execute("aliasedit", &json!({ "text": text })).unwrap(); + assert_eq!(r["loaded"], json!(MAX_ALIASES)); + assert_eq!(r["aliases"].as_object().unwrap().len(), MAX_ALIASES); + assert_eq!(r["aliases"]["zzp00000"], json!("line"), "the cap stops even a valid duplicate replacement"); + assert!(r["aliases"].get("zzafter").is_none(), "the remainder of the file is skipped"); + assert_eq!( + r["warnings"], + json!([ + format!("line {}: ZZP00000: unknown command *NO_SUCH_1933; skipped", MAX_ALIASES + 2), + format!("line {}: LINE is a command name; it stays the command; skipped", MAX_ALIASES + 3), + format!("line {}: ZZEXTERNAL is an external command; CADCraft doesn't run programs from alias files", MAX_ALIASES + 4), + format!("line {}: `malformed` is not ALIAS, *COMMAND; skipped", MAX_ALIASES + 5), + format!("line {}: more than {MAX_ALIASES} aliases; the rest are skipped", MAX_ALIASES + 6) + ]) + ); + + // Setting one existing alias through JSON has a different rule from importing a full file. + let r = s.execute("aliasedit", &json!({ "set": { "ZZP00000": "move" } })).unwrap(); + assert_eq!(r["aliases"]["zzp00000"], json!("move")); + assert_eq!(r["loaded"], json!(1)); + assert!(s.execute("aliasedit", &json!({ "set": { "ZZNEW": "line" } })).is_err()); + s.execute("aliasedit", &json!({ "clear": true })).unwrap(); +} + +#[test] +fn the_warning_limit_does_not_stop_a_later_valid_redefinition() { + let _one = ONE_AT_A_TIME.lock().unwrap_or_else(|e| e.into_inner()); + let mut s = Session::new(); + s.execute("aliasedit", &json!({ "clear": true })).unwrap(); + let mut text = String::from("ZZW,*LINE\n"); + for _ in 0..101 { + text.push_str("ZZW,*NO_SUCH_1933\n"); + } + text.push_str("zzw,*CIRCLE\n"); + let r = s.execute("aliasedit", &json!({ "text": text })).unwrap(); + assert_eq!(r["aliases"], json!({ "zzw": "circle" })); + assert_eq!(r["loaded"], json!(1)); + let warnings: Vec<_> = (2..=101).map(|no| format!("line {no}: ZZW: unknown command *NO_SUCH_1933; skipped")).collect(); + assert_eq!(r["warnings"], json!(warnings), "only the first 100 warnings are returned"); s.execute("aliasedit", &json!({ "clear": true })).unwrap(); } diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-22 · **Change:** minor (DIMASSOC objects for angular, diametric and radial dimensions) · **Target:** Autodesk AutoCAD 2027 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-26 · **Change:** minor (PGP alias import performance) · **Target:** Autodesk AutoCAD 2027 Every format AutoCAD 2027 reads or writes, with CADCraft's support, fidelity and tests. Work items are in [gaps.md](gaps.md#file-formats); the weighted score feeds @@ -121,7 +121,7 @@ on read with a note, so such files no longer exceed the DXF limit (#1482). **DWG | Linetype (`.lin`) and hatch pattern (`.pat`) files | load user files | user files load (#552); linetype shape (SHX) elements are skipped | 1–2 | | Plot styles (`.ctb`, `.stb`), plotter configs (`.pc3`), `.pmp` | yes | plot style tables in our own JSON format ([plot-styles.md](plot-styles.md)) and kept in the drawing; plotter devices with papers and printable areas in our own JSON format ([plotters.md](plotters.md), #564); binary `.ctb`/`.stb`/`.pc3`/`.pmp` not read (no public spec) | 4–8 | | Scripts (`.scr`) | SCRIPT | script text through the CLI, MCP and control channel; no SCRIPT command reading a `.scr` file | 1–2 | -| Aliases (`.pgp`), CUI(x), workspaces | yes | user alias files and ALIASEDIT (#552); no CUI(x) or workspaces | 4–8 | +| Aliases (`.pgp`), CUI(x), workspaces | yes | user alias files and ALIASEDIT (#552); duplicate definitions replace aliases without scanning the accumulated list (#1933); no CUI(x) or workspaces | 4–8 | | SVG import (`.svg`, `.svgz`) | yes (via scripts) | OPEN reads shapes, paths, groups and Inkscape layers into entities (#636); no CSS classes, `use`, fills, text or images (what the drawing loses is named in a note on open, #627) | 2–4 | | AutoLISP (`.lsp`, `.fas`, `.vlx`), `.dll`/`.arx` plug-ins | yes | `.lsp` loads (APPLOAD, `(load)`, a dropped file; #758); no Visual LISP ActiveX (`vlax-`), DCL dialogs or reactors; `.fas`/`.vlx` are compiled and undocumented; no `.dll`/`.arx` | see [gaps.md](gaps.md) (E1) | | Data extraction (`.csv`, `.xls`) and data links | yes | DATAEXTRACTION/EATTEXT to CSV or a table (#433); table data links read CSV (#534); no `.xls`, no `.dxe` templates | 3–5 | @@ -140,6 +140,7 @@ our output in AutoCAD black-box. | Date | Change | Summary | |---|---|---| +| 2026-10-26 | minor | PGP alias imports replace duplicate definitions by keyed lookup, avoiding repeated scans of the accumulated aliases; validation, warnings and alias limits are unchanged (#1933). Parity estimates are unchanged. | | 2026-10-22 | minor | DXF/DWG save: angular, diametric and radial dimensions get standard DIMASSOC objects (one centre reference to the circle for radial/diametric, the definition points for angular), so AutoCAD follows their geometry; the standard objects of those kinds are read back too, without CADCraft's xdata (#1651) | | 2026-10-19 | minor | DXF/DWG save: angular dimension block text honors DIMDSEP, including per-dimension overrides (#1646) | | 2026-10-19 | minor | A file's own arrowhead blocks with standard names (AutoCAD's `_Dot`, `_DotSmall`), named only by a dimension style, are kept and saved as they were instead of being replaced by CADCraft's solids; PURGE keeps arrowhead blocks dimension styles name |