shifathosn
/
cadcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

doc: Space::tab_name / layout / is_paper replace repeated hand-written matches (#888) 

* doc: Space::is_paper/layout/tab_name helpers replace hand-written matches

Twenty-seven call sites in 15 files across engine, io and ui-egui matched Space by hand to get
"is this a layout", "which layout" or "Model or the layout's name". Route them
through three small helpers on Space instead. No behaviour change.

This also makes adding a space kind a few-file change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* doc: keep Space::Model comparisons that no helper shortens; test the helpers

Only the sites that repeated the same match (tab name, layout name,
paper lineweights) go through the helpers; plain Model comparisons and
plot_window's match on (space, plotArea) stay as they were.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
mattlafy 2026-10-13 05:21:19 -04:00 committed by GitHub
parent 76b0ef8897
commit 2f5bbbd68d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 70 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
crates/doc/src/lib.rs
View File

@ -61,6 +61,26 @@ pub enum Space {
Paper(String),
}
impl Space {
/// Whether this is a layout's paper space.
pub fn is_paper(&self) -> bool {
matches!(self, Space::Paper(_))
}
/// The layout whose paper space this is; `None` for model space.
pub fn layout(&self) -> Option<&str> {
match self {
Space::Paper(n) => Some(n),
Space::Model => None,
}
}
/// The tab name (CTAB): `Model`, or the layout's name.
pub fn tab_name(&self) -> &str {
self.layout().unwrap_or("Model")
}
}
#[derive(Clone, Debug, PartialEq)]
pub struct Drawing {
pub header: Header,

11
crates/doc/src/tests.rs
View File

@ -549,3 +549,14 @@ fn invisible_dimblock_entity_is_ignored_in_dimension_extents() {
dim(&mut d, far_line("0"));
assert_eq!(d.extents(&Space::Model).max, Vec2::new(6000.0, 6000.0));
}
#[test]
fn space_names_its_tab() {
assert!(!Space::Model.is_paper());
assert_eq!(Space::Model.layout(), None);
assert_eq!(Space::Model.tab_name(), "Model");
let p = Space::Paper("Sheet A".into());
assert!(p.is_paper());
assert_eq!(p.layout(), Some("Sheet A"));
assert_eq!(p.tab_name(), "Sheet A");
}

16
crates/engine/src/cmd/exchange.rs
View File

@ -292,9 +292,9 @@ fn plot_scale(b: &Bounds2, sheet: &Sheet, fit: bool, scale: Option<f64>) -> f64
fn run_preview(s: &mut Session, p: &Value) -> Result<Value> {
let space = plot_space("preview", s, p)?;
let d = s.doc()?;
let mut page = match &space {
Space::Paper(n) => d.layout(n).map(|l| l.page.clone()).unwrap_or_default(),
Space::Model => {
let mut page = match space.layout() {
Some(n) => d.layout(n).map(|l| l.page.clone()).unwrap_or_default(),
None => {
// Model space plots on A4 (metric) or ANSI A, like the PDF plotter.
let mut page = PageSetup::default();
if paper::paper_unit_mm(d) == 1.0
@ -317,10 +317,7 @@ fn run_preview(s: &mut Session, p: &Value) -> Result<Value> {
page.landscape = l;
}
let sheet = Sheet::from_page(&page, paper::paper_unit_mm(d));
let lineweights = p.get("lineweights").and_then(Value::as_bool).unwrap_or(match space {
Space::Paper(_) => page.lineweights,
Space::Model => true,
});
let lineweights = p.get("lineweights").and_then(Value::as_bool).unwrap_or(if space.is_paper() { page.lineweights } else { true });
let fit = p.get("fit").and_then(Value::as_bool).unwrap_or(matches!(space, Space::Model));
let scale = p.get("scale").and_then(Value::as_f64).filter(|v| v.is_finite() && *v > 0.0);
let width = p.get("width").and_then(Value::as_u64).unwrap_or(PREVIEW_PX).clamp(16, MAX_PREVIEW_PX);
@ -360,10 +357,7 @@ fn run_preview(s: &mut Session, p: &Value) -> Result<Value> {
antialias: true,
};
let png = cadcraft_render::raster::render_png(&list, &view, &ro).ok_or_else(|| bad("preview", "rendering the preview failed"))?;
let layout = match &space {
Space::Model => "Model".to_string(),
Space::Paper(n) => n.clone(),
};
let layout = space.tab_name();
Ok(json!({ "data": base64_encode(&png), "width": w, "height": h, "layout": layout, "paper": page.paper, "landscape": page.landscape }))
}

2
crates/engine/src/cmd/inquiry.rs
View File

@ -283,7 +283,7 @@ fn run_inspect(s: &mut Session, p: &Value) -> Result<Value> {
"title": st.title,
"path": st.path,
"dirty": st.is_dirty(),
"space": match &space { cadcraft_doc::Space::Model => "Model".to_string(), cadcraft_doc::Space::Paper(n) => n.clone() },
"space": space.tab_name(),
"entityCount": d.entity_count(),
"counts": counts,
"extents": if ext.is_empty() { Value::Null } else { json!([[ext.min.x, ext.min.y], [ext.max.x, ext.max.y]]) },

5
crates/engine/src/cmd/inquiry/count.rs
View File

@ -74,10 +74,7 @@ impl Scope {
}
/// The top-level objects in scope, each with the name of its space.
fn objects<'a>(&self, d: &'a Drawing) -> Vec<(String, &'a std::sync::Arc<Entity>)> {
let name = |sp: &Space| match sp {
Space::Model => "Model".to_string(),
Space::Paper(n) => n.clone(),
};
let name = |sp: &Space| sp.tab_name().to_string();
let mut out = Vec::new();
let mut add = |sp: &Space| {
if let Some(store) = d.space(sp) {

20
crates/engine/src/cmd/layout.rs
View File

@ -260,10 +260,10 @@ fn run_delete(s: &mut Session, p: &Value) -> Result<Value> {
}
fn run_rename(s: &mut Session, p: &Value) -> Result<Value> {
let from = match (str_param(p, "from"), s.layout_space()) {
let from = match (str_param(p, "from"), s.layout_space().layout()) {
(Some(f), _) => f.to_string(),
(None, Space::Paper(n)) => n,
(None, Space::Model) => return Err(bad("layout.rename", "`from` is required in the Model tab")),
(None, Some(n)) => n.to_string(),
(None, None) => return Err(bad("layout.rename", "`from` is required in the Model tab")),
};
let from = layout_name("layout.rename", s.doc()?, &from)?;
let to = str_param(p, "to").ok_or_else(|| bad("layout.rename", "`to` is required"))?;
@ -636,10 +636,9 @@ fn run_pagesetup(s: &mut Session, p: &Value) -> Result<Value> {
"displayPlotStyles",
];
let changes = keys.iter().any(|k| p.get(k).is_some());
let name = match (str_param(p, "layout"), s.space()) {
(Some(n), _) => Some(layout_name(cmd, s.doc()?, n)?),
(None, Space::Paper(n)) => Some(n),
(None, Space::Model) => None,
let name = match str_param(p, "layout") {
Some(n) => Some(layout_name(cmd, s.doc()?, n)?),
None => s.space().layout().map(str::to_string),
};
let Some(name) = name else {
if changes {
@ -806,10 +805,9 @@ fn run_plot(s: &mut Session, p: &Value, cmd: &str) -> Result<Value> {
}
}
let bytes = hook(s.doc()?, &space, &opts).map_err(|e| bad(cmd, e))?;
let layout = match (&sheets, &space) {
(Some(names), _) => names.first().cloned().unwrap_or_default(),
(None, Space::Model) => "Model".to_string(),
(None, Space::Paper(n)) => n.clone(),
let layout = match &sheets {
Some(names) => names.first().cloned().unwrap_or_default(),
None => space.tab_name().to_string(),
};
let pages = sheets.as_ref().map_or(1, Vec::len);
match str_param(p, "path") {

15
crates/engine/src/cmd/layout/prompts.rs
View File

@ -87,10 +87,7 @@ impl LayoutM {
let pats: Vec<&str> = pat.split(',').map(str::trim).filter(|p| !p.is_empty()).take(1000).collect();
let pats = if pats.is_empty() { vec!["*"] } else { pats };
let names: Vec<String> = ls.iter().filter(|l| pats.iter().any(|p| wildcard(p, &l.name))).map(|l| l.name.clone()).collect();
let current = match s.layout_space() {
Space::Paper(n) => n,
Space::Model => "Model".into(),
};
let current = s.layout_space().tab_name().to_string();
s.echo(format!("Active Layout: {current}"));
if names.is_empty() {
s.echo(format!("No layout matches \"{pat}\"."));
@ -131,10 +128,7 @@ impl Interactive for LayoutM {
Stage::RenameFrom => with_cur(Prompt::new("Enter layout to rename", Accept::TEXT)),
Stage::RenameTo(_) => Prompt::new("Enter new layout name", Accept::TEXT),
Stage::Set => {
let tab = match s.layout_space() {
Space::Paper(n) => n,
Space::Model => "Model".into(),
};
let tab = s.layout_space().tab_name().to_string();
Prompt::new("Enter layout to make current", Accept::TEXT).default(tab)
}
Stage::List => Prompt::new("Enter layout name(s) to list", Accept::TEXT).default("*"),
@ -199,10 +193,7 @@ impl Interactive for LayoutM {
Stage::Set => {
let n = match name {
Some(n) => n,
None => match s.layout_space() {
Space::Paper(n) => n,
Space::Model => "Model".into(),
},
None => s.layout_space().tab_name().to_string(),
};
let set = find_command("layout.set").ok_or_else(|| EngineError::UnknownCommand("layout.set".into()))?;
(set.run)(s, &json!({ "name": n })).map_err(retry)?;

2
crates/engine/src/cmd/modify2.rs
View File

@ -1452,7 +1452,7 @@ fn chspace(s: &mut Session, hs: &[Handle], to: Option<&str>, vp: Option<Handle>)
fn run_chspace(s: &mut Session, p: &Value) -> Result<Value> {
let hs = targets(s, p)?;
let (n, _, target) = chspace(s, &hs, str_param(p, "to"), curves::handle_param(p, "viewport"))?;
Ok(json!({ "moved": n, "to": match target { Space::Model => "model".to_string(), Space::Paper(n) => n } }))
Ok(json!({ "moved": n, "to": target.layout().unwrap_or("model") }))
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]

6
crates/engine/src/cmd/plotters.rs
View File

@ -286,9 +286,9 @@ pub(crate) fn plot_options(s: &Session, p: &Value, cmd: &str) -> Result<Value> {
let layout = match str_param(p, "layout") {
Some(n) if n.eq_ignore_ascii_case("model") => None,
Some(n) => s.doc()?.layout(n).map(|l| l.page.clone()),
None => match s.space() {
cadcraft_doc::Space::Paper(n) => s.doc()?.layout(&n).map(|l| l.page.clone()),
cadcraft_doc::Space::Model => None,
None => match s.space().layout() {
Some(n) => s.doc()?.layout(n).map(|l| l.page.clone()),
None => None,
},
};
let dev = match given {

6
crates/engine/src/cmd/view/named_views.rs
View File

@ -152,10 +152,10 @@ fn restore(s: &mut Session, name: &str) -> Result<Value> {
match &v.layout {
Some(l) => {
// A paper space view: its layout (an unknown one: the current or first layout).
let target = match (l.is_empty(), s.layout_space()) {
let target = match (l.is_empty(), s.layout_space().layout()) {
(false, _) => l.clone(),
(true, Space::Paper(cur)) => cur,
(true, Space::Model) => s.doc()?.layouts.iter().min_by_key(|x| x.tab_order).map(|x| x.name.clone()).unwrap_or_default(),
(true, Some(cur)) => cur.to_string(),
(true, None) => s.doc()?.layouts.iter().min_by_key(|x| x.tab_order).map(|x| x.name.clone()).unwrap_or_default(),
};
if s.doc()?.layout(&target).is_none() {
return Err(EngineError::Other(format!("View \"{}\" belongs to layout \"{target}\", which does not exist.", v.name)));

9
crates/engine/src/sysvars.rs
View File

@ -116,14 +116,7 @@ pub fn get(s: &Session, name: &str) -> Option<Value> {
"DBMOD" => json!(s.state().map(|d| i32::from(d.is_dirty())).unwrap_or(0)),
// The template NEW/QNEW start from (a profile setting, like FONTALT; "" = none).
"QNEWTEMPLATE" => json!(crate::cmd::file::qnew_template()),
"CTAB" => json!(
s.state()
.map(|d| match &d.space {
cadcraft_doc::Space::Model => "Model".to_string(),
cadcraft_doc::Space::Paper(n) => n.clone(),
})
.unwrap_or_default()
),
"CTAB" => json!(s.state().map(|d| d.space.tab_name().to_string()).unwrap_or_default()),
// UNDO state of the drawing: Control/Auto/group bits and the number of marks.
"UNDOCTL" => json!(s.state().ok()?.undo_ctl.bits()),
"UNDOMARKS" => json!(s.state().ok()?.undo_marks()),

16
crates/io/src/pdf.rs
View File

@ -265,9 +265,9 @@ fn links_json(v: Option<&Value>) -> Vec<(Handle, String)> {
/// The page setup a plot of `space` uses, with overrides applied.
pub fn page_for(d: &Drawing, space: &Space, o: &PdfOptions) -> Result<PageSetup> {
let mut page = match space {
Space::Paper(n) => d.layout(n).map(|l| l.page.clone()).ok_or_else(|| IoError::Format(format!("no layout `{n}`")))?,
Space::Model => {
let mut page = match space.layout() {
Some(n) => d.layout(n).map(|l| l.page.clone()).ok_or_else(|| IoError::Format(format!("no layout `{n}`")))?,
None => {
let mut p = PageSetup::default();
if paper::paper_unit_mm(d) == 1.0
&& let Some(a4) = paper::paper_size("A4")
@ -328,10 +328,7 @@ pub fn pdf(d: &Drawing, space: &Space, o: &PdfOptions) -> Result<Vec<u8>> {
/// The name of a sheet: `Model` or the layout's.
fn sheet_name(space: &Space) -> String {
match space {
Space::Model => "Model".to_string(),
Space::Paper(n) => n.clone(),
}
space.tab_name().to_string()
}
/// Write a vector PDF with one page per space (at most [`MAX_PAGES`]), each labelled with its
@ -394,10 +391,7 @@ fn sheet(d: &Drawing, space: &Space, o: &PdfOptions, links_to: &[(Handle, String
let page = page_for(d, space, o)?;
let unit_mm = paper::paper_unit_mm(d);
let sheet = Sheet::from_page(&page, unit_mm);
let lineweights = o.lineweights.unwrap_or(match space {
Space::Paper(_) => page.lineweights,
Space::Model => true,
});
let lineweights = o.lineweights.unwrap_or(if space.is_paper() { page.lineweights } else { true });
let plot_style_table = cadcraft_doc::plot_style_table(d, &page.plot_style_table).map(std::sync::Arc::new);
let ropts = cadcraft_render::Options {
tolerance: 0.001,

5
crates/io/src/plotter.rs
View File

@ -76,10 +76,7 @@ fn png(d: &Drawing, space: &Space, opts: &Value) -> Result<Vec<u8>> {
k *= shrink;
let px = |v: f64| (v * k).round().clamp(1.0, 16384.0) as u32;
let (width, height) = (px(sheet.size.x), px(sheet.size.y));
let lineweights = o.lineweights.unwrap_or(match space {
Space::Paper(_) => page.lineweights,
Space::Model => true,
});
let lineweights = o.lineweights.unwrap_or(if space.is_paper() { page.lineweights } else { true });
let table = cadcraft_doc::plot_style_table(d, &page.plot_style_table).map(Arc::new);
// Paper units per drawing unit: 1 for a layout at 1:1, the fitted scale for model space.
let fit = matches!(space, Space::Model) || o.fit == Some(true);

10
crates/ui-egui/src/managers.rs
View File

@ -474,10 +474,7 @@ fn page_setup(app: &mut CadApp, ctx: &egui::Context, open: &mut bool) -> Action
};
let mut layouts: Vec<(u32, String, cadcraft_doc::PageSetup)> = d.layouts.iter().map(|l| (l.tab_order, l.name.clone(), l.page.clone())).collect();
layouts.sort_by_key(|l| l.0);
let shown = match app.session.layout_space() {
cadcraft_doc::Space::Paper(n) => Some(n),
cadcraft_doc::Space::Model => None,
};
let shown = app.session.layout_space().layout().map(str::to_string);
let mut sel = temp::<String>(ctx, "pagesetup_sel")
.filter(|s| layouts.iter().any(|l| l.1 == *s))
.or(shown)
@ -900,10 +897,7 @@ fn plot(app: &mut CadApp, ctx: &egui::Context, open: &mut bool) -> Action {
targets.extend(layouts.into_iter().map(|l| l.1));
let generation = PLOT_GEN.load(Ordering::Relaxed);
let mut st = temp::<PlotState>(ctx, "plot_state").filter(|s| s.generation == generation && targets.contains(&s.target)).unwrap_or_else(|| {
let target = match app.session.layout_space() {
cadcraft_doc::Space::Paper(n) => n,
cadcraft_doc::Space::Model => "Model".into(),
};
let target = app.session.layout_space().tab_name().to_string();
let mut ext = d.extents(&cadcraft_doc::Space::Model);
if ext.is_empty() {
ext = cadcraft_geom::Bounds2::new(cadcraft_geom::Vec2::ZERO, cadcraft_geom::Vec2::new(12.0, 9.0));

6
crates/ui-egui/src/palettes.rs
View File

@ -749,10 +749,8 @@ pub(crate) fn properties_section(app: &mut CadApp, ui: &mut egui::Ui) {
}
});
// The current layout's plot style table; named tables give the current plot style.
let layout = match app.session.layout_space() {
cadcraft_doc::Space::Paper(n) => d.layout(&n).map(|l| (n.clone(), l.page.plot_style_table.clone())),
cadcraft_doc::Space::Model => None,
};
let space = app.session.layout_space();
let layout = space.layout().and_then(|n| d.layout(n).map(|l| (n.to_string(), l.page.plot_style_table.clone())));
let table = layout.as_ref().and_then(|(_, t)| cadcraft_doc::plot_style_table(d, t));
match table.as_ref().filter(|t| !t.is_color_dependent()) {
Some(t) => prop_row(ui, "Plot style", |ui| {

6
crates/ui-egui/src/plotstyles.rs
View File

@ -51,10 +51,8 @@ fn window(app: &mut CadApp, ctx: &egui::Context, open: &mut bool) -> Action {
return None;
};
let names = cadcraft_doc::plot_style_table_names(d);
let layout = match app.session.layout_space() {
cadcraft_doc::Space::Paper(n) => d.layout(&n).map(|l| (n.clone(), l.page.plot_style_table.clone(), l.page.show_plot_styles)),
cadcraft_doc::Space::Model => None,
};
let space = app.session.layout_space();
let layout = space.layout().and_then(|n| d.layout(n).map(|l| (n.to_string(), l.page.plot_style_table.clone(), l.page.show_plot_styles)));
let mut sel = temp::<String>(ctx, "pstyle_sel")
.filter(|s| names.iter().any(|n| n.eq_ignore_ascii_case(s)))
.or_else(|| layout.as_ref().map(|l| l.1.clone()).filter(|n| names.iter().any(|x| x.eq_ignore_ascii_case(n))))