ByoungSooPark/modeling-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add display of units for calculated KCL values (#7619)

Browse Source 
* Add display of units in UI modals with calculated KCL values

* Fix command bar display to handle units

* Add display of units in the command bar

* Fix more cases of NaN from units

* Fix to support explicit plus for exponent in scientific notation

* Fix display in autocomplete

* Change to parseFloat to be more resilient

* Add e2e test for command bar

* Change an existing test to use explicit inline units

* Fix case when input string can't be parsed
This commit is contained in:
Jonathan Tran
2025-06-30 15:26:45 -04:00
committed by GitHub
parent 27af2d08a3
commit 85c721fb49
17 changed files with 360 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
e2e/playwright/command-bar-tests.spec.ts
View File

@ -525,7 +525,9 @@ test.describe('Command bar tests', () => {
const projectName = 'test'
const beforeKclCode = `a = 5
b = a * a
c = 3 + a`
c = 3 + a
theta = 45deg
`
await context.folderSetupFn(async (dir) => {
const testProject = join(dir, projectName)
await fsp.mkdir(testProject, { recursive: true })
@ -615,9 +617,45 @@ c = 3 + a`
stage: 'commandBarClosed',
})
})
await test.step(`Edit a parameter with explicit units via command bar`, async () => {
await cmdBar.cmdBarOpenBtn.click()
await cmdBar.chooseCommand('edit parameter')
await cmdBar
.selectOption({
name: 'theta',
})
.click()
await cmdBar.expectState({
stage: 'arguments',
commandName: 'Edit parameter',
currentArgKey: 'value',
currentArgValue: '45deg',
headerArguments: {
Name: 'theta',
Value: '',
},
highlightedHeaderArg: 'value',
})
await cmdBar.argumentInput
.locator('[contenteditable]')
.fill('45deg + 1deg')
await cmdBar.progressCmdBar()
await cmdBar.expectState({
stage: 'review',
commandName: 'Edit parameter',
headerArguments: {
Name: 'theta',
Value: '46deg',
},
})
await cmdBar.progressCmdBar()
await cmdBar.expectState({
stage: 'commandBarClosed',
})
})
await editor.expectEditor.toContain(
`a = 5b = a * amyParameter001 = ${newValue}c = 3 + a`
`a = 5b = a * amyParameter001 = ${newValue}c = 3 + atheta = 45deg + 1deg`
)
})

6
e2e/playwright/point-click.spec.ts
View File

@ -136,17 +136,17 @@ test.describe('Point-and-click tests', () => {
highlightedHeaderArg: 'length',
commandName: 'Extrude',
})
await page.keyboard.insertText('width - 0.001')
await page.keyboard.insertText('width - 0.001in')
await cmdBar.progressCmdBar()
await cmdBar.expectState({
stage: 'review',
headerArguments: {
Length: '4.999',
Length: '4.999in',
},
commandName: 'Extrude',
})
await cmdBar.progressCmdBar()
await editor.expectEditor.toContain('extrude(length = width - 0.001)')
await editor.expectEditor.toContain('extrude(length = width - 0.001in)')
})
await test.step(`Edit second extrude via feature tree`, async () => {

12
rust/kcl-lib/src/execution/types.rs
View File

@ -840,6 +840,18 @@ pub enum UnitType {
Angle(UnitAngle),
}
impl UnitType {
pub(crate) fn to_suffix(self) -> Option<String> {
match self {
UnitType::Count => Some("_".to_owned()),
UnitType::Length(UnitLen::Unknown) => None,
UnitType::Angle(UnitAngle::Unknown) => None,
UnitType::Length(l) => Some(l.to_string()),
UnitType::Angle(a) => Some(a.to_string()),
}
}
}
impl std::fmt::Display for UnitType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {

95
rust/kcl-lib/src/fmt.rs
View File

@ -45,6 +45,31 @@ pub fn format_number_literal(value: f64, suffix: NumericSuffix) -> Result<String
}
}
#[derive(Debug, Clone, PartialEq, Serialize, thiserror::Error)]
#[serde(tag = "type")]
pub enum FormatNumericTypeError {
#[error("Invalid numeric type: {0:?}")]
Invalid(NumericType),
}
/// For UI code generation, format a number value with a suffix such that the
/// result can parse as a literal. If it can't be done, returns an error.
///
/// This is used by TS.
pub fn format_number_value(value: f64, ty: NumericType) -> Result<String, FormatNumericTypeError> {
match ty {
NumericType::Default { .. } => Ok(value.to_string()),
// There isn't a syntactic suffix for these. For unknown, we don't want
// to ever generate the unknown suffix. We currently warn on it, and we
// may remove it in the future.
NumericType::Unknown | NumericType::Any => Err(FormatNumericTypeError::Invalid(ty)),
NumericType::Known(unit_type) => unit_type
.to_suffix()
.map(|suffix| format!("{value}{suffix}"))
.ok_or(FormatNumericTypeError::Invalid(ty)),
}
}
#[cfg(test)]
mod tests {
use pretty_assertions::assert_eq;
@ -134,4 +159,74 @@ mod tests {
Err(FormatNumericSuffixError::Invalid(NumericSuffix::Unknown))
);
}
#[test]
fn test_format_number_value() {
assert_eq!(
format_number_value(
1.0,
NumericType::Default {
len: Default::default(),
angle: Default::default()
}
),
Ok("1".to_owned())
);
assert_eq!(
format_number_value(1.0, NumericType::Known(UnitType::Length(UnitLen::Unknown))),
Err(FormatNumericTypeError::Invalid(NumericType::Known(UnitType::Length(
UnitLen::Unknown
))))
);
assert_eq!(
format_number_value(1.0, NumericType::Known(UnitType::Angle(UnitAngle::Unknown))),
Err(FormatNumericTypeError::Invalid(NumericType::Known(UnitType::Angle(
UnitAngle::Unknown
))))
);
assert_eq!(
format_number_value(1.0, NumericType::Known(UnitType::Count)),
Ok("1_".to_owned())
);
assert_eq!(
format_number_value(1.0, NumericType::Known(UnitType::Length(UnitLen::Mm))),
Ok("1mm".to_owned())
);
assert_eq!(
format_number_value(1.0, NumericType::Known(UnitType::Length(UnitLen::Cm))),
Ok("1cm".to_owned())
);
assert_eq!(
format_number_value(1.0, NumericType::Known(UnitType::Length(UnitLen::M))),
Ok("1m".to_owned())
);
assert_eq!(
format_number_value(1.0, NumericType::Known(UnitType::Length(UnitLen::Inches))),
Ok("1in".to_owned())
);
assert_eq!(
format_number_value(1.0, NumericType::Known(UnitType::Length(UnitLen::Feet))),
Ok("1ft".to_owned())
);
assert_eq!(
format_number_value(1.0, NumericType::Known(UnitType::Length(UnitLen::Yards))),
Ok("1yd".to_owned())
);
assert_eq!(
format_number_value(1.0, NumericType::Known(UnitType::Angle(UnitAngle::Degrees))),
Ok("1deg".to_owned())
);
assert_eq!(
format_number_value(1.0, NumericType::Known(UnitType::Angle(UnitAngle::Radians))),
Ok("1rad".to_owned())
);
assert_eq!(
format_number_value(1.0, NumericType::Unknown),
Err(FormatNumericTypeError::Invalid(NumericType::Unknown))
);
assert_eq!(
format_number_value(1.0, NumericType::Any),
Err(FormatNumericTypeError::Invalid(NumericType::Any))
);
}
}

2
rust/kcl-lib/src/lib.rs
View File

@ -140,7 +140,7 @@ pub mod std_utils {
pub mod pretty {
pub use crate::{
fmt::{format_number_literal, human_display_number},
fmt::{format_number_literal, format_number_value, human_display_number},
parsing::token::NumericSuffix,
};
}

31
rust/kcl-wasm-lib/src/wasm.rs
View File

@ -61,6 +61,37 @@ pub fn format_number_literal(value: f64, suffix_json: &str) -> Result<String, Js
kcl_lib::pretty::format_number_literal(value, suffix).map_err(JsError::from)
}
#[wasm_bindgen]
pub fn format_number_value(value: f64, numeric_type_json: &str) -> Result<String, String> {
console_error_panic_hook::set_once();
// ts-rs can't handle tuple types, so it mashes all of these types together.
if let Ok(ty) = serde_json::from_str::<NumericType>(numeric_type_json) {
if let Ok(formatted) = kcl_lib::pretty::format_number_value(value, ty) {
return Ok(formatted);
}
}
if let Ok(unit_type) = serde_json::from_str::<UnitType>(numeric_type_json) {
let ty = NumericType::Known(unit_type);
if let Ok(formatted) = kcl_lib::pretty::format_number_value(value, ty) {
return Ok(formatted);
}
}
if let Ok(unit_len) = serde_json::from_str::<UnitLen>(numeric_type_json) {
let ty = NumericType::Known(UnitType::Length(unit_len));
if let Ok(formatted) = kcl_lib::pretty::format_number_value(value, ty) {
return Ok(formatted);
}
}
if let Ok(unit_angle) = serde_json::from_str::<UnitAngle>(numeric_type_json) {
let ty = NumericType::Known(UnitType::Angle(unit_angle));
if let Ok(formatted) = kcl_lib::pretty::format_number_value(value, ty) {
return Ok(formatted);
}
}
Err(format!("Invalid type: {numeric_type_json}"))
}
#[wasm_bindgen]
pub fn human_display_number(value: f64, ty_json: &str) -> Result<String, String> {
console_error_panic_hook::set_once();

8
src/components/CommandBar/CommandBarHeaderFooter.tsx
View File

@ -12,7 +12,7 @@ import type {
} from '@src/lib/commandTypes'
import type { Selections } from '@src/lib/selections'
import { getSelectionTypeDisplayText } from '@src/lib/selections'
import { roundOff } from '@src/lib/utils'
import { roundOffWithUnits } from '@src/lib/utils'
import { commandBarActor, useCommandBarState } from '@src/lib/singletons'
function CommandBarHeaderFooter({
@ -163,10 +163,8 @@ function CommandBarHeaderFooter({
arg.inputType === 'selectionMixed' ? (
getSelectionTypeDisplayText(argValue as Selections)
) : arg.inputType === 'kcl' ? (
roundOff(
Number(
(argValue as KclCommandValue).valueCalculated
),
roundOffWithUnits(
(argValue as KclCommandValue).valueCalculated,
4
)
) : arg.inputType === 'text' &&

26
src/components/CommandBar/CommandBarKclInput.tsx
View File

@ -21,13 +21,13 @@ import { Spinner } from '@src/components/Spinner'
import { createLocalName, createVariableDeclaration } from '@src/lang/create'
import { getNodeFromPath } from '@src/lang/queryAst'
import type { SourceRange, VariableDeclarator } from '@src/lang/wasm'
import { isPathToNode } from '@src/lang/wasm'
import { formatNumberValue, isPathToNode } from '@src/lang/wasm'
import type { CommandArgument, KclCommandValue } from '@src/lib/commandTypes'
import { kclManager } from '@src/lib/singletons'
import { getSystemTheme } from '@src/lib/theme'
import { err } from '@src/lib/trap'
import { useCalculateKclExpression } from '@src/lib/useCalculateKclExpression'
import { roundOff } from '@src/lib/utils'
import { roundOff, roundOffWithUnits } from '@src/lib/utils'
import { varMentions } from '@src/lib/varCompletionExtension'
import { useSettings } from '@src/lib/singletons'
import { commandBarActor, useCommandBarState } from '@src/lib/singletons'
@ -128,10 +128,22 @@ function CommandBarKclInput({
sourceRange: sourceRangeForPrevVariables,
})
const varMentionData: Completion[] = prevVariables.map((v) => ({
label: v.key,
detail: String(roundOff(Number(v.value))),
}))
const varMentionData: Completion[] = prevVariables.map((v) => {
const roundedWithUnits = (() => {
if (typeof v.value !== 'number' || !v.ty) {
return undefined
}
const numWithUnits = formatNumberValue(v.value, v.ty)
if (err(numWithUnits)) {
return undefined
}
return roundOffWithUnits(numWithUnits)
})()
return {
label: v.key,
detail: roundedWithUnits ?? String(roundOff(Number(v.value))),
}
})
const varMentionsExtension = varMentions(varMentionData)
const { setContainer, view } = useCodeMirror({
@ -282,7 +294,7 @@ function CommandBarKclInput({
) : calcResult === 'NAN' ? (
"Can't calculate"
) : (
roundOff(Number(calcResult), 4)
roundOffWithUnits(calcResult, 4)
)}
</span>
</label>

33
src/lang/queryAst.test.ts
View File

@ -63,11 +63,36 @@ variableBelowShouldNotBeIncluded = 3
execState.variables,
topLevelRange(rangeStart, rangeStart)
)
const defaultTy = {
type: 'Default',
angle: {
type: 'Degrees',
},
len: {
type: 'Mm',
},
}
expect(variables).toEqual([
{ key: 'baseThick', value: 1 },
{ key: 'armAngle', value: 60 },
{ key: 'baseThickHalf', value: 0.5 },
{ key: 'halfArmAngle', value: 30 },
{
key: 'baseThick',
value: 1,
ty: defaultTy,
},
{
key: 'armAngle',
value: 60,
ty: defaultTy,
},
{
key: 'baseThickHalf',
value: 0.5,
ty: defaultTy,
},
{
key: 'halfArmAngle',
value: 30,
ty: defaultTy,
},
// no arrExpShouldNotBeIncluded, variableBelowShouldNotBeIncluded etc
])
// there are 4 number variables and 2 non-number variables before the sketch var

3
src/lang/queryAst.ts
View File

@ -55,6 +55,7 @@ import type { OpKclValue, Operation } from '@rust/kcl-lib/bindings/Operation'
import { ARG_INDEX_FIELD, LABELED_ARG_FIELD } from '@src/lang/queryAstConstants'
import type { KclCommandValue } from '@src/lib/commandTypes'
import type { UnaryExpression } from 'typescript'
import type { NumericType } from '@rust/kcl-lib/bindings/NumericType'
/**
* Retrieves a node from a given path within a Program node structure, optionally stopping at a specified node type.
@ -306,6 +307,7 @@ export function traverse(
export interface PrevVariable<T> {
key: string
value: T
ty: NumericType | undefined
}
export function findAllPreviousVariablesPath(
@ -353,6 +355,7 @@ export function findAllPreviousVariablesPath(
variables.push({
key: varName,
value: varValue.value,
ty: varValue.type === 'Number' ? varValue.ty : undefined,
})
})

18
src/lang/wasm.ts
View File

@ -45,6 +45,7 @@ import {
default_app_settings,
default_project_settings,
format_number_literal,
format_number_value,
get_kcl_version,
get_tangential_arc_to_info,
human_display_number,
@ -448,6 +449,23 @@ export function formatNumberLiteral(
}
}
/**
* Format a number from a KclValue such that it could be parsed as KCL.
*/
export function formatNumberValue(
value: number,
numericType: NumericType
): string | Error {
try {
return format_number_value(value, JSON.stringify(numericType))
} catch (e) {
return new Error(
`Error formatting number value: value=${value}, numericType=${numericType}`,
{ cause: e }
)
}
}
/**
* Debug display a number with suffix, for human consumption only.
*/

9
src/lib/kclHelpers.test.ts
View File

@ -2,13 +2,20 @@ import type { ParseResult } from '@src/lang/wasm'
import { getCalculatedKclExpressionValue } from '@src/lib/kclHelpers'
describe('KCL expression calculations', () => {
it('calculates a simple expression', async () => {
it('calculates a simple expression without units', async () => {
const actual = await getCalculatedKclExpressionValue('1 + 2')
const coercedActual = actual as Exclude<typeof actual, Error | ParseResult>
expect(coercedActual).not.toHaveProperty('errors')
expect(coercedActual.valueAsString).toEqual('3')
expect(coercedActual?.astNode).toBeDefined()
})
it('calculates a simple expression with units', async () => {
const actual = await getCalculatedKclExpressionValue('1deg + 30deg')
const coercedActual = actual as Exclude<typeof actual, Error | ParseResult>
expect(coercedActual).not.toHaveProperty('errors')
expect(coercedActual.valueAsString).toEqual('31deg')
expect(coercedActual?.astNode).toBeDefined()
})
it('returns NAN for an invalid expression', async () => {
const actual = await getCalculatedKclExpressionValue('1 + x')
const coercedActual = actual as Exclude<typeof actual, Error | ParseResult>

28
src/lib/kclHelpers.ts
View File

@ -1,5 +1,10 @@
import { executeAstMock } from '@src/lang/langHelpers'
import { type CallExpressionKw, parse, resultIsOk } from '@src/lang/wasm'
import {
type CallExpressionKw,
formatNumberValue,
parse,
resultIsOk,
} from '@src/lang/wasm'
import type { KclCommandValue, KclExpression } from '@src/lib/commandTypes'
import { rustContext } from '@src/lib/singletons'
import { err } from '@src/lib/trap'
@ -32,12 +37,27 @@ export async function getCalculatedKclExpressionValue(value: string) {
const variableDeclaratorAstNode =
resultDeclaration?.type === 'VariableDeclaration' &&
resultDeclaration?.declaration.init
const resultRawValue = execState.variables[DUMMY_VARIABLE_NAME]?.value
const varValue = execState.variables[DUMMY_VARIABLE_NAME]
// If the value is a number, attempt to format it with units.
const resultValueWithUnits = (() => {
if (!varValue || varValue.type !== 'Number') {
return undefined
}
const formatted = formatNumberValue(varValue.value, varValue.ty)
if (err(formatted)) return undefined
return formatted
})()
// Prefer the formatted value with units. Fallback to the raw value.
const resultRawValue = varValue?.value
const valueAsString = resultValueWithUnits
? resultValueWithUnits
: typeof resultRawValue === 'number'
? String(resultRawValue)
: 'NAN'
return {
astNode: variableDeclaratorAstNode,
valueAsString:
typeof resultRawValue === 'number' ? String(resultRawValue) : 'NAN',
valueAsString,
}
}

2
src/lib/useCalculateKclExpression.ts
View File

@ -79,7 +79,7 @@ export function useCalculateKclExpression({
isValueParsable = false
}
const initialCalcResult: number | string =
Number.isNaN(Number(value)) || !isValueParsable ? 'NAN' : value
Number.isNaN(parseFloat(value)) || !isValueParsable ? 'NAN' : value
const [calcResult, setCalcResult] = useState(initialCalcResult)
const [newVariableName, _setNewVariableName] = useState('')
const [isNewVariableNameUnique, setIsNewVariableNameUnique] = useState(true)

43
src/lib/utils.test.ts
View File

@ -8,6 +8,7 @@ import {
isOverlap,
onDragNumberCalculation,
roundOff,
roundOffWithUnits,
simulateOnMouseDragMatch,
} from '@src/lib/utils'
@ -43,6 +44,48 @@ describe('testing roundOff', () => {
})
})
describe('roundOffWithUnits', () => {
it('works with no units', () => {
expect(roundOffWithUnits('1.23456789')).toBe('1.23')
expect(roundOffWithUnits('1.23456789', 3)).toBe('1.235')
expect(roundOffWithUnits('1.', 3)).toBe('1')
expect(roundOffWithUnits('-1.23456789')).toBe('-1.23')
expect(roundOffWithUnits('-1.23456789', 3)).toBe('-1.235')
expect(roundOffWithUnits('-1.', 3)).toBe('-1')
})
it('works with standard units', () => {
expect(roundOffWithUnits('1.23456789mm', 3)).toBe('1.235mm')
expect(roundOffWithUnits('1.23456789m', 3)).toBe('1.235m')
expect(roundOffWithUnits('1.23456789in', 3)).toBe('1.235in')
expect(roundOffWithUnits('1.23456789_', 3)).toBe('1.235_')
expect(roundOffWithUnits('1._', 3)).toBe('1_')
expect(roundOffWithUnits('-1.23456789mm', 3)).toBe('-1.235mm')
expect(roundOffWithUnits('-1.23456789m', 3)).toBe('-1.235m')
expect(roundOffWithUnits('-1.23456789in', 3)).toBe('-1.235in')
expect(roundOffWithUnits('-1.23456789_', 3)).toBe('-1.235_')
expect(roundOffWithUnits('-1._', 3)).toBe('-1_')
expect(roundOffWithUnits('1.23456789e3mm', 3)).toBe('1234.568mm')
expect(roundOffWithUnits('1.23456789e3m', 3)).toBe('1234.568m')
expect(roundOffWithUnits('1.23456789e3in', 3)).toBe('1234.568in')
expect(roundOffWithUnits('1.23456789e3_', 3)).toBe('1234.568_')
expect(roundOffWithUnits('1.e3_', 3)).toBe('1000_')
expect(roundOffWithUnits('1e3_', 3)).toBe('1000_')
expect(roundOffWithUnits('1.23456789e-3mm', 3)).toBe('0.001mm')
expect(roundOffWithUnits('1.23456789e-3m', 3)).toBe('0.001m')
expect(roundOffWithUnits('1.23456789e-3in', 3)).toBe('0.001in')
expect(roundOffWithUnits('1.23456789e-3_', 3)).toBe('0.001_')
expect(roundOffWithUnits('1.e-3_', 3)).toBe('0.001_')
expect(roundOffWithUnits('1e-3_', 3)).toBe('0.001_')
})
it('works with weird units', () => {
expect(roundOffWithUnits('1.23456789_?', 3)).toBe('1.235_?')
expect(roundOffWithUnits('-1.23456789_?', 3)).toBe('-1.235_?')
})
it('returns the original string when used with something not parsable as a number', () => {
expect(roundOffWithUnits('foo', 3)).toBe('foo')
})
})
describe('testing hasLeadingZero', () => {
it('.1 should have no leading zero', () => {
const actual = hasLeadingZero('.1')

26
src/lib/utils.ts
View File

@ -328,6 +328,32 @@ export function roundOff(num: number, precision: number = 2): number {
return Math.round(num * x) / x
}
export function roundOffWithUnits(
numWithUnits: string,
precision: number = 2
): string {
const match = numWithUnits.match(
/^([+-]?[\d.]+(?:[eE][+-]?\d+)?)([a-zA-Z_?]+)$/
)
let num: string
let suffix: string
if (match) {
num = match[1]
suffix = match[2] ?? ''
} else {
// If no match, assume it's just a number with no units.
num = numWithUnits
suffix = ''
}
const parsedNum = parseFloat(num)
if (Number.isNaN(parsedNum)) {
// If parsing fails, return the original string.
return numWithUnits
}
const roundedNum = roundOff(parsedNum, precision)
return `${roundedNum}${suffix}`
}
/**
* Determine if the number as a string has any precision in the decimal places
* '1' -> 0

4
src/lib/wasm_lib_wrapper.ts
View File

@ -13,6 +13,7 @@ import type {
default_app_settings as DefaultAppSettings,
default_project_settings as DefaultProjectSettings,
format_number_literal as FormatNumberLiteral,
format_number_value as FormatNumberValue,
human_display_number as HumanDisplayNumber,
get_kcl_version as GetKclVersion,
get_tangential_arc_to_info as GetTangentialArcToInfo,
@ -59,6 +60,9 @@ export const recast_wasm: typeof RecastWasm = (...args) => {
export const format_number_literal: typeof FormatNumberLiteral = (...args) => {
return getModule().format_number_literal(...args)
}
export const format_number_value: typeof FormatNumberValue = (...args) => {
return getModule().format_number_value(...args)
}
export const human_display_number: typeof HumanDisplayNumber = (...args) => {
return getModule().human_display_number(...args)
}