Repository navigation
Transform commands: a string for an optional number is an error (#1952) - #1967
Merged
bflatastic merged 1 commit intoOct 15, 2026
Merged
Conversation
Messhias
approved these changes
Oct 15, 2026
Messhias
left a comment
Messhias
left a comment
Contributor
There was a problem hiding this comment.
Good fix for the silent ignore of stringy optional numbers. check_numbers is clear, covers the Transform panel commands, and the regression test checks both BadParams and that history/bounds stay put.
…ytold#1952) object.setBounds read x, y, width and height with f64_or, which falls back to the current value for anything that isn't a JSON number, so {"width": "60 mm"} returned ok and changed nothing. object.move, object.nudge, object.rotate, object.scale (sy) and object.shear did the same with their optional numbers. A value that is neither a number nor null is now a BadParams error naming the key, as a missing required number already is.
bflatastic
force-pushed
the
fix/1952-optional-number-strings
branch
from
October 15, 2026 20:20
136f28e to
e6170de
Compare
Member
|
Thanks @switch87! Merged after a full gate. Checked over MCP: |
This was referenced Oct 18, 2026
Transform Each, Rasterize, Mosaic, Trim Marks and Rearrange: a number given as text is refused
#2917
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1952
object.setBoundsreadx,y,widthandheightwithf64_or, which falls back to the current value for anything that isn't a JSON number.{"width": "60 mm"}therefore returned{"ids": [...]}, exited 0 and changed nothing. The other Transform commands did the same with their optional numbers:object.move(dx,dy),object.nudge(dx,dy),object.rotate(angle),object.scale(sy) andobject.shear(angle).A value that is neither a number nor
nullis now aBadParamserror that names the key (for exampleobject.setBounds: `width` must be a number, not "60 mm"), as a missing required number already is. Numbers andnullbehave as before. Unit strings are not parsed: the parameters stay in points.The check is a small helper,
check_numbersincmd/mod.rs, called at the top of each command, so nothing changes in the document or the undo history before the error. #1964 (audit) sketches the same check forobject.setBoundsonly. This PR also covers the sibling transform commands.Behaviour change: scripts that passed a string or other non-number to these optional parameters used to succeed silently with the value ignored. They now get an error.
Tests
a_string_for_an_optional_transform_number_is_an_error(crates/engine/src/tests.rs): every command above, given a string, bool or array, returnsBadParamsnaming the key and leaves the bounds and undo history unchanged;width: 60withheight: nullstill works. It fails onmain, whereobject.setBounds {"width": "60 mm"}returns Ok.cargo xtask ci: fmt, clippy, test (6575 passed, 0 failed), assets, brands and layers pass. I could not run the wasm step locally because the Gentoo Rust toolchain here has no wasm32 std (can't find crate for std), so CI has to check it.