Repository navigation
Transform Each, Rasterize, Mosaic, Trim Marks and Rearrange: a number given as text is refused - #2917
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing: 3
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 78.26% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 3 files. (2 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the primary change: nonnumeric values are rejected for the listed menu commands. It is concise and specific. Description check The description includes the required What and why, How it was tested, and Checklist sections. It explains the behavior, affected keys, documentation changes, test coverage, validation commands, and c… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description includes the required What and why, How it was tested, and Checklist sections. It explains the behavior, affected keys, documentation changes, test coverage, validation commands, and completed checklist items.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 78.26% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 3 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
… given as text is refused
Transform Each, Rasterize, Create Object Mosaic, Create Trim Marks and
Rearrange All Artboards read their optional numbers with f64_or, which
falls back to a default for a value that is not a JSON number.
object.transformEach {"scaleH": "50"} left the selected objects at
their size and reported success. Rasterize did the same with ppi,
Create Object Mosaic did the same with width, height, spacingX and
spacingY, Create Trim Marks did the same with offset, length, bleed and
weight, and Rearrange All Artboards did the same with spacing. Crop
Image cropped to the artboard when rect was a string or another value
that is not an array.
These commands now fail with "`<key>` must be a number, not <value>"
for a value that is not a number or null, as the transform commands
do, and change nothing. Transform Each checks scaleH, scaleV, moveH,
moveV and rotate. Crop Image refuses a rect that is not an array of
four numbers with "rect must be [x, y, width, height]", the message it
already gave for an array of the wrong length; a null rect crops to the
artboard, as a missing one does.
Docs: docs/mcp.md, docs/target-app-parity.md.
a9befa3 to
cc8944a
Compare
|
Thanks @dotCooCoo, merged. It went through the full gate (fmt, clippy, tests, layers, wasm) in a batch with the other PRs landed today. |
2 participants
What and why
On
main, Transform Each, Rasterize, Create Object Mosaic, Create Trim Marks and Rearrange All Artboards read their optional numbers withf64_or, which falls back to a default for a value that is not a JSON number.object.transformEach {"scaleH": "50"}leaves the selected objects at their size and reports success, andobject.createTrimMarks {"offset": "10"}makes the marks at the default offset.object.transformEach(scaleH,scaleV,moveH,moveV,rotate),object.rasterize(ppi),object.createObjectMosaic(width,height,spacingX,spacingY),object.createTrimMarks(offset,length,bleed,weight) andartboard.rearrange(spacing) fail with "<key>must be a number, not " for a value that is not a number or null, as the transform commands do since Transform commands: a string for an optional number is an error (#1952) #1967 and the blend, envelope and mesh commands do since Blends, meshes and envelopes: a number given as text is refused #2851 (check_numbers,crates/engine/src/cmd/mod.rs:321). The document and the undo history stay as they were.object.cropImagerefuses arectthat is not an array, such as a string, with "rect must be [x, y, width, height]", the message it already gives for an array of the wrong length. Before, it cropped to the artboard. A nullrectcrops to the artboard, as a missing one does.docs/mcp.mdlists the keys, anddocs/target-app-parity.mdhas one line for the change. The change adds no message, so it adds no catalog rows.How it was tested
New tests in vectorcraft-engine, each failing on
main, where the first call succeeds:tests_menucmds::transform_each_refuses_a_number_given_as_textgives each scale, move androtateas a string, an array ortrue; the object keeps its bounds and no undo step is added, andscaleH: 50,scaleV: 50andmoveH: 10as numbers, withmoveV: null, still transform it.tests_menucmds::rasterize_refuses_a_ppi_given_as_textgivesppias a string or an array; the art stays as it was, andppi: 144still rasterizes it.tests::object_mosaic_refuses_a_size_or_spacing_given_as_textgiveswidth,height,spacingXandspacingYas strings or an array; no mosaic is made, andwidth: 200withspacingX: nullstill makes the tiles.tests_menucmds::crop_image_refuses_a_rect_given_as_textgivesrectas a string, an object and a number; the image stays as it was, and a nullrectcrops it to the artboard.tests_menucmds::trim_marks_refuse_a_number_given_as_textgives each key as a string,trueor an object, with an object selected and with nothing selected; no marks are made.tests_menucmds::rearrange_refuses_a_spacing_given_as_textgivesspacingas a string orfalse; the artboards stay where they were, andspacing: 50still spaces them 50 pt apart.Targeted checks while building:
cargo clippy -p vectorcraft-engine --all-targets -- -D warnings; the new tests;cargo fmt --all -- --check;cargo xtask brands,layersandassets.On Windows 11 (x86-64) with rustc 1.99.1, on top of
mainatcdba0aa:CRAFT_FONTS_DIR=<craft-fonts checkout> cargo xtask cipasses its 10 steps: 8,256 tests passed, 0 failed, 12 ignored.git merge-treereports no conflicts withmainatcdba0aa. #2592, #2532, #2403, #2038 and #1838 also changecrates/engine/src/cmd/menucmds.rsand merge with this branch there; their conflicts withmainin other files are the same with or without this branch.Checklist
cargo xtask cipasses (fmt, clippy, tests, assets, brands, layers, wasm).unwrap(),expect(),panic!,unreachable!,todo!orunimplemented!outside tests, and no unchecked indexing on data from files, parameters ormessages. Errors are returned as
Resultand shown to the user(why and how).
ASSETS.md.Summary by CodeRabbit
nullrectangle crops to the artboard.nullvalues are handled.