BREAKING: KCL @settings are the source of truth for units (#5808)

This commit is contained in:
Jonathan Tran
2025-03-31 10:56:03 -04:00
committed by GitHub
parent eac5abba79
commit efc8c82d8b
237 changed files with 820 additions and 2146 deletions

View File

@ -97,15 +97,6 @@ pub(super) async fn get_changed_program(old: CacheInformation<'_>, new: CacheInf
// If the settings are different we might need to bust the cache.
// We specifically do this before checking if they are the exact same.
if old.settings != new.settings {
// If the units are different we need to re-execute the whole thing.
if old.settings.units != new.settings.units {
return CacheResult::ReExecute {
clear_scene: true,
reapply_settings: true,
program: new.ast.clone(),
};
}
// If anything else is different we may not need to re-execute, but rather just
// run the settings again.
reapply_settings = true;
@ -424,50 +415,6 @@ shell(firstSketch, faces = ['end'], thickness = 0.25)"#;
assert_eq!(result, CacheResult::NoAction(false));
}
// Changing the units with the exact same file should bust the cache.
#[tokio::test(flavor = "multi_thread")]
async fn test_get_changed_program_same_code_but_different_units() {
let new = r#"// Remove the end face for the extrusion.
firstSketch = startSketchOn('XY')
|> startProfileAt([-12, 12], %)
|> line(end = [24, 0])
|> line(end = [0, -24])
|> line(end = [-24, 0])
|> close()
|> extrude(length = 6)
// Remove the end face for the extrusion.
shell(firstSketch, faces = ['end'], thickness = 0.25)"#;
let ExecTestResults {
program, mut exec_ctxt, ..
} = parse_execute(new).await.unwrap();
// Change the settings to cm.
exec_ctxt.settings.units = crate::UnitLength::Cm;
let result = get_changed_program(
CacheInformation {
ast: &program.ast,
settings: &Default::default(),
},
CacheInformation {
ast: &program.ast,
settings: &exec_ctxt.settings,
},
)
.await;
assert_eq!(
result,
CacheResult::ReExecute {
clear_scene: true,
reapply_settings: true,
program: program.ast
}
);
}
// Changing the grid settings with the exact same file should NOT bust the cache.
#[tokio::test(flavor = "multi_thread")]
async fn test_get_changed_program_same_code_but_different_grid_setting() {
@ -615,4 +562,42 @@ startSketchOn('XY')
}
);
}
// Removing the units settings using an annotation, when it was non-default
// units, with the exact same file should bust the cache.
#[tokio::test(flavor = "multi_thread")]
async fn test_get_changed_program_same_code_but_removed_unit_setting_using_annotation() {
let old_code = r#"@settings(defaultLengthUnit = in)
startSketchOn('XY')
"#;
let new_code = r#"
startSketchOn('XY')
"#;
let ExecTestResults { program, exec_ctxt, .. } = parse_execute(old_code).await.unwrap();
let mut new_program = crate::Program::parse_no_errs(new_code).unwrap();
new_program.compute_digest();
let result = get_changed_program(
CacheInformation {
ast: &program.ast,
settings: &exec_ctxt.settings,
},
CacheInformation {
ast: &new_program.ast,
settings: &exec_ctxt.settings,
},
)
.await;
assert_eq!(
result,
CacheResult::ReExecute {
clear_scene: true,
reapply_settings: true,
program: new_program.ast
}
);
}
}

View File

@ -110,12 +110,7 @@ impl ExecutorContext {
let old_units = exec_state.length_unit();
let original_execution = self.engine.replace_execution_kind(exec_kind).await;
let mut local_state = ModuleState::new(
&self.settings,
path.std_path(),
exec_state.stack().memory.clone(),
Some(module_id),
);
let mut local_state = ModuleState::new(path.std_path(), exec_state.stack().memory.clone(), Some(module_id));
if !preserve_mem {
std::mem::swap(&mut exec_state.mod_local, &mut local_state);
}

View File

@ -39,7 +39,6 @@ use crate::{
fs::FileManager,
modules::{ModuleId, ModulePath},
parsing::ast::types::{Expr, ImportPath, NodeRef},
settings::types::UnitLength,
source_range::SourceRange,
std::StdLib,
CompilationError, ExecError, ExecutionKind, KclErrorWithOutputs,
@ -265,8 +264,6 @@ pub struct ExecutorContext {
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
#[ts(export)]
pub struct ExecutorSettings {
/// The project-default unit to use in modeling dimensions.
pub units: UnitLength,
/// Highlight edges of 3D objects?
pub highlight_edges: bool,
/// Whether or not Screen Space Ambient Occlusion (SSAO) is enabled.
@ -287,7 +284,6 @@ pub struct ExecutorSettings {
impl Default for ExecutorSettings {
fn default() -> Self {
Self {
units: Default::default(),
highlight_edges: true,
enable_ssao: false,
show_grid: false,
@ -301,7 +297,6 @@ impl Default for ExecutorSettings {
impl From<crate::settings::types::Configuration> for ExecutorSettings {
fn from(config: crate::settings::types::Configuration) -> Self {
Self {
units: config.settings.modeling.base_unit,
highlight_edges: config.settings.modeling.highlight_edges.into(),
enable_ssao: config.settings.modeling.enable_ssao.into(),
show_grid: config.settings.modeling.show_scale_grid,
@ -315,7 +310,6 @@ impl From<crate::settings::types::Configuration> for ExecutorSettings {
impl From<crate::settings::types::project::ProjectConfiguration> for ExecutorSettings {
fn from(config: crate::settings::types::project::ProjectConfiguration) -> Self {
Self {
units: config.settings.modeling.base_unit,
highlight_edges: config.settings.modeling.highlight_edges.into(),
enable_ssao: config.settings.modeling.enable_ssao.into(),
show_grid: Default::default(),
@ -329,7 +323,6 @@ impl From<crate::settings::types::project::ProjectConfiguration> for ExecutorSet
impl From<crate::settings::types::ModelingSettings> for ExecutorSettings {
fn from(modeling: crate::settings::types::ModelingSettings) -> Self {
Self {
units: modeling.base_unit,
highlight_edges: modeling.highlight_edges.into(),
enable_ssao: modeling.enable_ssao.into(),
show_grid: modeling.show_scale_grid,
@ -343,7 +336,6 @@ impl From<crate::settings::types::ModelingSettings> for ExecutorSettings {
impl From<crate::settings::types::project::ProjectModelingSettings> for ExecutorSettings {
fn from(modeling: crate::settings::types::project::ProjectModelingSettings) -> Self {
Self {
units: modeling.base_unit,
highlight_edges: modeling.highlight_edges.into(),
enable_ssao: modeling.enable_ssao.into(),
show_grid: Default::default(),
@ -476,26 +468,17 @@ impl ExecutorContext {
/// This allows for passing in `ZOO_API_TOKEN` and `ZOO_HOST` as environment
/// variables.
#[cfg(not(target_arch = "wasm32"))]
pub async fn new_with_default_client(units: UnitLength) -> Result<Self> {
pub async fn new_with_default_client() -> Result<Self> {
// Create the client.
let ctx = Self::new_with_client(
ExecutorSettings {
units,
..Default::default()
},
None,
None,
)
.await?;
let ctx = Self::new_with_client(Default::default(), None, None).await?;
Ok(ctx)
}
/// For executing unit tests.
#[cfg(not(target_arch = "wasm32"))]
pub async fn new_for_unit_test(units: UnitLength, engine_addr: Option<String>) -> Result<Self> {
pub async fn new_for_unit_test(engine_addr: Option<String>) -> Result<Self> {
let ctx = ExecutorContext::new_with_client(
ExecutorSettings {
units,
highlight_edges: true,
enable_ssao: false,
show_grid: false,
@ -862,11 +845,6 @@ impl ExecutorContext {
Ok(())
}
/// Update the units for the executor.
pub(crate) fn update_units(&mut self, units: UnitLength) {
self.settings.units = units;
}
/// Get a snapshot of the current scene.
pub async fn prepare_snapshot(&self) -> std::result::Result<TakeSnapshot, ExecError> {
// Zoom to fit.
@ -1008,11 +986,7 @@ mod tests {
use pretty_assertions::assert_eq;
use super::*;
use crate::{
errors::{KclErrorDetails, Severity},
execution::memory::Stack,
ModuleId,
};
use crate::{errors::KclErrorDetails, execution::memory::Stack, ModuleId};
/// Convenience function to get a JSON value from memory and unwrap.
#[track_caller]
@ -1615,34 +1589,6 @@ const inInches = 2.0 * inch()"#;
);
}
#[tokio::test(flavor = "multi_thread")]
async fn test_unit_suggest() {
let src = "foo = 42";
let program = crate::Program::parse_no_errs(src).unwrap();
let ctx = ExecutorContext {
engine: Arc::new(Box::new(
crate::engine::conn_mock::EngineConnection::new().await.unwrap(),
)),
fs: Arc::new(crate::fs::FileManager::new()),
stdlib: Arc::new(crate::std::StdLib::new()),
settings: ExecutorSettings {
units: UnitLength::Ft,
..Default::default()
},
context_type: ContextType::Mock,
};
let mut exec_state = ExecState::new(&ctx);
ctx.run(&program, &mut exec_state).await.unwrap();
let errs = exec_state.errors();
assert_eq!(errs.len(), 1, "{errs:?}");
let warn = &errs[0];
assert_eq!(warn.severity, Severity::Warning);
assert_eq!(
warn.apply_suggestion(src).unwrap(),
"@settings(defaultLengthUnit = ft)\nfoo = 42"
)
}
#[tokio::test(flavor = "multi_thread")]
async fn test_zero_param_fn() {
let ast = r#"const sigmaAllow = 35000 // psi
@ -1971,9 +1917,7 @@ let w = f() + f()
)
"#;
let ctx = crate::test_server::new_context(UnitLength::Mm, true, None)
.await
.unwrap();
let ctx = crate::test_server::new_context(true, None).await.unwrap();
let old_program = crate::Program::parse_no_errs(code).unwrap();
// Execute the program.
@ -2026,9 +1970,7 @@ let w = f() + f()
)
"#;
let mut ctx = crate::test_server::new_context(UnitLength::Mm, true, None)
.await
.unwrap();
let mut ctx = crate::test_server::new_context(true, None).await.unwrap();
let old_program = crate::Program::parse_no_errs(code).unwrap();
// Execute the program.
@ -2066,7 +2008,7 @@ let w = f() + f()
#[tokio::test(flavor = "multi_thread")]
async fn mock_after_not_mock() {
let ctx = ExecutorContext::new_with_default_client(UnitLength::Mm).await.unwrap();
let ctx = ExecutorContext::new_with_default_client().await.unwrap();
let program = crate::Program::parse_no_errs("x = 2").unwrap();
let result = ctx.run_with_caching(program).await.unwrap();
assert_eq!(result.variables.get("x").unwrap().as_f64().unwrap(), 2.0);

View File

@ -82,7 +82,7 @@ impl ExecState {
pub fn new(exec_context: &super::ExecutorContext) -> Self {
ExecState {
global: GlobalState::new(&exec_context.settings),
mod_local: ModuleState::new(&exec_context.settings, None, ProgramMemory::new(), Default::default()),
mod_local: ModuleState::new(None, ProgramMemory::new(), Default::default()),
exec_context: Some(exec_context.clone()),
}
}
@ -92,7 +92,7 @@ impl ExecState {
*self = ExecState {
global,
mod_local: ModuleState::new(&exec_context.settings, None, ProgramMemory::new(), Default::default()),
mod_local: ModuleState::new(None, ProgramMemory::new(), Default::default()),
exec_context: Some(exec_context.clone()),
};
}
@ -289,12 +289,7 @@ impl GlobalState {
}
impl ModuleState {
pub(super) fn new(
exec_settings: &ExecutorSettings,
std_path: Option<String>,
memory: Arc<ProgramMemory>,
module_id: Option<ModuleId>,
) -> Self {
pub(super) fn new(std_path: Option<String>, memory: Arc<ProgramMemory>, module_id: Option<ModuleId>) -> Self {
ModuleState {
id_generator: IdGenerator::new(module_id),
stack: memory.new_stack(),
@ -303,14 +298,14 @@ impl ModuleState {
explicit_length_units: false,
std_path,
settings: MetaSettings {
default_length_units: exec_settings.units.into(),
default_length_units: Default::default(),
default_angle_units: Default::default(),
},
}
}
}
#[derive(Debug, Default, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
#[derive(Debug, Default, Clone, Deserialize, Serialize, PartialEq, Eq, ts_rs::TS, JsonSchema)]
#[ts(export)]
#[serde(rename_all = "camelCase")]
pub struct MetaSettings {