forked from 1b-agent/opencadstudio-upstream
Fix four things the Options survey turned up
**TEXTFILL marked the drawing modified.** It is an application preference — it lives in a process global and is snapshotted into settings.json — but it was missing from the list of names that persist, so setting it dirtied the open drawing instead and never reached the settings file through that path. **LTSCALE and CELTSCALE validated differently depending on the door.** The standalone commands reject a value of zero or less, because a non-positive linetype scale degenerates every dash pattern in the drawing. `SETVAR LTSCALE -5` accepted it. A test drives both entry points for both variables and fails on the SETVAR path without the fix. **ISAVEBAK, SAVETIME and FILEASSOC were missing from the autocomplete registry** although all three dispatch, so the command line never offered them. **`bg_color` and `paper_bg_color` were dead fields**, always written as `None` and ignored on load since the model-space configuration superseded them. Removing them is safe: the config is `#[serde(default)]` and ignores unknown keys, so existing settings files simply stop carrying them. One thing the survey flagged that turned out not to be a bug: on the Display page the model background row hides itself outside Custom mode while the paper and desk rows do not. That asymmetry is correct — the canvas mode governs only the model background, and the other two apply in every mode. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
55b2dc1796
commit
4cc8e7db62
4 files changed
+59
-13
No files matched your search
@@ -554,6 +554,11 @@ inventory::submit!(crate::command::CommandRegistration {
|
|||||||
"GRIPHOT",
|
"GRIPHOT",
|
||||||
"GRIPHOVER",
|
"GRIPHOVER",
|
||||||
"GRIPOBJLIMIT",
|
"GRIPOBJLIMIT",
|
||||||
|
// Dispatched all along, but absent from the registry, so command-line
|
||||||
|
// completion never offered them.
|
||||||
|
"ISAVEBAK",
|
||||||
|
"SAVETIME",
|
||||||
|
"FILEASSOC",
|
||||||
// Reset selected entities' overrides to follow their layer.
|
// Reset selected entities' overrides to follow their layer.
|
||||||
"SETBYLAYER",
|
"SETBYLAYER",
|
||||||
// Remove duplicate objects; set drawing base point; audit integrity;
|
// Remove duplicate objects; set drawing base point; audit integrity;
|
||||||
|
|||||||
@@ -1167,24 +1167,37 @@ impl OpenCADStudio {
|
|||||||
let outcome: Result<(String, bool), String> = {
|
let outcome: Result<(String, bool), String> = {
|
||||||
let h = &mut self.tabs[i].scene.document.header;
|
let h = &mut self.tabs[i].scene.document.header;
|
||||||
match name.as_str() {
|
match name.as_str() {
|
||||||
|
// The standalone LTSCALE and CELTSCALE commands
|
||||||
|
// require a positive value; reaching the same
|
||||||
|
// variable through SETVAR used to accept zero or a
|
||||||
|
// negative one, which degenerates every dash
|
||||||
|
// pattern in the drawing.
|
||||||
"LTSCALE" => match &value {
|
"LTSCALE" => match &value {
|
||||||
Some(v) => v
|
Some(v) => v
|
||||||
.parse::<f64>()
|
.parse::<f64>()
|
||||||
|
.ok()
|
||||||
|
.filter(|x| x.is_finite() && *x > 0.0)
|
||||||
.map(|x| {
|
.map(|x| {
|
||||||
h.linetype_scale = x;
|
h.linetype_scale = x;
|
||||||
(format!("LTSCALE = {x}"), true)
|
(format!("LTSCALE = {x}"), true)
|
||||||
})
|
})
|
||||||
.map_err(|_| "SETVAR: numeric value required.".into()),
|
.ok_or_else(|| {
|
||||||
|
"SETVAR: positive numeric value required.".into()
|
||||||
|
}),
|
||||||
None => Ok((format!("LTSCALE = {}", h.linetype_scale), false)),
|
None => Ok((format!("LTSCALE = {}", h.linetype_scale), false)),
|
||||||
},
|
},
|
||||||
"CELTSCALE" => match &value {
|
"CELTSCALE" => match &value {
|
||||||
Some(v) => v
|
Some(v) => v
|
||||||
.parse::<f64>()
|
.parse::<f64>()
|
||||||
|
.ok()
|
||||||
|
.filter(|x| x.is_finite() && *x > 0.0)
|
||||||
.map(|x| {
|
.map(|x| {
|
||||||
h.current_entity_linetype_scale = x;
|
h.current_entity_linetype_scale = x;
|
||||||
(format!("CELTSCALE = {x}"), true)
|
(format!("CELTSCALE = {x}"), true)
|
||||||
})
|
})
|
||||||
.map_err(|_| "SETVAR: numeric value required.".into()),
|
.ok_or_else(|| {
|
||||||
|
"SETVAR: positive numeric value required.".into()
|
||||||
|
}),
|
||||||
None => Ok((
|
None => Ok((
|
||||||
format!("CELTSCALE = {}", h.current_entity_linetype_scale),
|
format!("CELTSCALE = {}", h.current_entity_linetype_scale),
|
||||||
false,
|
false,
|
||||||
@@ -2156,6 +2169,12 @@ impl OpenCADStudio {
|
|||||||
| "GRIPHOVER"
|
| "GRIPHOVER"
|
||||||
| "GRIPOBJLIMIT"
|
| "GRIPOBJLIMIT"
|
||||||
| "DELOBJ"
|
| "DELOBJ"
|
||||||
|
// App-level, not part of the drawing:
|
||||||
|
// it lives in a process global and is
|
||||||
|
// snapshotted into the settings file.
|
||||||
|
// Without this it marked the drawing
|
||||||
|
// modified instead of persisting.
|
||||||
|
| "TEXTFILL"
|
||||||
) {
|
) {
|
||||||
self.persist_settings_if_changed();
|
self.persist_settings_if_changed();
|
||||||
} else {
|
} else {
|
||||||
@@ -3144,3 +3163,36 @@ mod tests {
|
|||||||
assert_eq!(app.delete_objects, 3);
|
assert_eq!(app.delete_objects, 3);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod scale_validation_tests {
|
||||||
|
use crate::app::OpenCADStudio;
|
||||||
|
|
||||||
|
/// A linetype scale of zero or less degenerates every dash pattern in the
|
||||||
|
/// drawing, which is why the standalone commands reject it. Reaching the
|
||||||
|
/// same variable through SETVAR used to accept it, so which door you came
|
||||||
|
/// through decided whether the drawing could be broken.
|
||||||
|
#[test]
|
||||||
|
fn both_doors_to_ltscale_refuse_a_non_positive_value() {
|
||||||
|
for name in ["LTSCALE", "CELTSCALE"] {
|
||||||
|
for entry in [format!("{name} -5"), format!("SETVAR {name} -5")] {
|
||||||
|
let mut app = OpenCADStudio::new_for_test();
|
||||||
|
app.automation_op(r#"{"op":"new"}"#);
|
||||||
|
let i = app.active_tab;
|
||||||
|
let before = if name == "LTSCALE" {
|
||||||
|
app.tabs[i].scene.document.header.linetype_scale
|
||||||
|
} else {
|
||||||
|
app.tabs[i].scene.document.header.current_entity_linetype_scale
|
||||||
|
};
|
||||||
|
let _ = app.run_command_line(&entry);
|
||||||
|
let after = if name == "LTSCALE" {
|
||||||
|
app.tabs[i].scene.document.header.linetype_scale
|
||||||
|
} else {
|
||||||
|
app.tabs[i].scene.document.header.current_entity_linetype_scale
|
||||||
|
};
|
||||||
|
assert_eq!(before, after, "{entry} must be refused");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@@ -269,12 +269,6 @@ pub struct UserSettings {
|
|||||||
pub pick_drag_rect: bool,
|
pub pick_drag_rect: bool,
|
||||||
/// Show the floating Quick Properties panel when objects are selected.
|
/// Show the floating Quick Properties panel when objects are selected.
|
||||||
pub quick_properties: bool,
|
pub quick_properties: bool,
|
||||||
/// Persisted viewport background colours (0–255 RGB); `None` = app default
|
|
||||||
/// (dark grey model / off-white paper). Applied to every drawing tab on
|
|
||||||
/// launch and to tabs opened later, so a chosen background survives restarts
|
|
||||||
/// (#188).
|
|
||||||
pub bg_color: Option<[u8; 3]>,
|
|
||||||
pub paper_bg_color: Option<[u8; 3]>,
|
|
||||||
/// Interface language preference. `System` negotiates against the
|
/// Interface language preference. `System` negotiates against the
|
||||||
/// platform locale on every launch.
|
/// platform locale on every launch.
|
||||||
pub language: crate::i18n::Language,
|
pub language: crate::i18n::Language,
|
||||||
@@ -384,8 +378,6 @@ impl Default for UserSettings {
|
|||||||
pick_add: true,
|
pick_add: true,
|
||||||
pick_drag_rect: false,
|
pick_drag_rect: false,
|
||||||
quick_properties: false,
|
quick_properties: false,
|
||||||
bg_color: None,
|
|
||||||
paper_bg_color: None,
|
|
||||||
language: crate::i18n::Language::default(),
|
language: crate::i18n::Language::default(),
|
||||||
cliprompt_lines: 3,
|
cliprompt_lines: 3,
|
||||||
commandline_fade_ms: 3000,
|
commandline_fade_ms: 3000,
|
||||||
|
|||||||
@@ -435,8 +435,6 @@ impl OpenCADStudio {
|
|||||||
pick_add: self.pick_add,
|
pick_add: self.pick_add,
|
||||||
pick_drag_rect: self.pick_drag_rect,
|
pick_drag_rect: self.pick_drag_rect,
|
||||||
quick_properties: self.quick_properties,
|
quick_properties: self.quick_properties,
|
||||||
bg_color: None,
|
|
||||||
paper_bg_color: None,
|
|
||||||
language: self.language,
|
language: self.language,
|
||||||
cliprompt_lines: crate::app::settings::clamp_clipromptlines(self.cliprompt_lines),
|
cliprompt_lines: crate::app::settings::clamp_clipromptlines(self.cliprompt_lines),
|
||||||
commandline_fade_ms: crate::app::settings::clamp_commandline_fade_ms(
|
commandline_fade_ms: crate::app::settings::clamp_commandline_fade_ms(
|
||||||
@@ -516,7 +514,6 @@ impl OpenCADStudio {
|
|||||||
self.pick_add = s.pick_add;
|
self.pick_add = s.pick_add;
|
||||||
self.pick_drag_rect = s.pick_drag_rect;
|
self.pick_drag_rect = s.pick_drag_rect;
|
||||||
self.quick_properties = s.quick_properties;
|
self.quick_properties = s.quick_properties;
|
||||||
// Legacy settings.bg_color / paper_bg_color are superseded by model_space config.
|
|
||||||
if crate::i18n::set_language(s.language).is_ok() {
|
if crate::i18n::set_language(s.language).is_ok() {
|
||||||
self.language = s.language;
|
self.language = s.language;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user