You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
path.blob clamped a supplied angle to -180 to 180 degrees, a roundness to 0 to 100% and a fidelity to 0.01 to 100 pt, so a call with angle: 200 or fidelity: -1 painted with a different value from the one it gave. The Blob Brush help page lists Angle, which rotates the brush tip, Roundness, which sets the tip's shape, and Fidelity as tool options, and gives no numeric limits for them (help page); the ranges are VectorCraft's own values, the ones the Tool Options dialog already uses. This changes existing behavior: a value outside its range is now an error instead of being clamped.
path.blob (crates/engine/src/cmd/draw2/freehand.rs) returns an invalid-parameters error for an angle outside -180 to 180, a roundness outside 0 to 100 and a fidelity outside 0.01 to 100 pt, before the document changes, and records no undo step. NaN and infinities fail the same checks. A key left out or null still takes its default: 0 degrees, 100% and no fitting. The endpoints are accepted.
path.freehand treated any style other than "brush" as "pencil", and any extendend other than "start" as "end". It now rejects an unknown value with an error, as it already did for the end of connect. Left out or null, they keep "pencil" and "end".
The new messages have rows in every complete-message catalog, and connect end must be "start" or "end", which 22 of the 28 catalogs lack on main, gets rows in the 18 of those that are complete-message catalogs. The other 4 (bg, bs, gu and id) still lack it. Nothing emits the message angle and roundness must be finite any more, and its row is removed from the 15 catalogs that have one on main.
The params text of path.blob and path.freehand, docs/mcp.md and one line in docs/target-app-parity.md describe the refusals.
How it was tested
New tests, each failing without the change:
tests::draw2::blob_brush_rejects_out_of_range_tip_and_fidelity_without_edit (vectorcraft-engine): values just outside each bound are refused with their message and add no path; the endpoints and omitted or null keys still paint.
tests::draw2::pencil_refuses_an_unknown_style_or_extend_end (vectorcraft-engine): an unknown style and an unknown extendend are refused and leave the paths as they were, and "pencil", "brush", "end", "start" and null still draw.
i18n::tests::blob_brush_range_refusals_translate_with_their_values and i18n::tests::freehand_choice_refusals_translate_with_their_values (vectorcraft-ui-egui): every complete-message catalog translates the new messages; the path.blob messages keep the parameter names and numbers, and the path.freehand messages keep the accepted values as written, in the catalog's own quotation marks.
The existing test tests::draw2::blob_tip_zero_and_invalid_values_are_safe painted with an angle and roundness of 1e300 and -1e300; it now expects those calls to be refused.
Focused checks on Linux, with the branch as one commit on main at 0b29b2b1f6:
cargo test -p vectorcraft-engine --lib filtered to tests::draw2::pencil, tests::draw2::blob, drawing_commands and tests::params: 57 passed
cargo test -p vectorcraft-ui-egui --lib i18n: 112 passed
On 2ec0af8eb0, with main at d89f593bfa merged in: cargo fmt --all -- --check, cargo clippy -p vectorcraft-ui-egui --all-targets -- -D warnings, and cargo test -p vectorcraft-ui-egui --lib i18n: 124 passed.
On 0f342cd689, with main at 0adbdce6cb merged in: cargo fmt --all -- --check, cargo check --workspace --tests, and cargo test -p vectorcraft-ui-egui --lib i18n: 124 passed.
On 963d38d040, with main at 9ddbf25bef merged in (Danish is now a complete-message language, and da.tsv gets the rows above): cargo fmt --all -- --check, cargo check --workspace --tests, and cargo test -p vectorcraft-ui-egui --lib i18n: 125 passed.
On the earlier base 4558196e16: cargo xtask brands, layers (30 crates) and assets (337 files).
No panics in shipped code: no unwrap(), expect(), panic!, unreachable!, todo! or unimplemented! outside tests, and no unchecked indexing on data from files, parameters or
messages. Errors are returned as Result and shown to the user
(why and how).
Clean-room: nothing copied from Adobe products or GPL/AGPL code (AGENTS.md).
Every new asset has a row in ASSETS.md.
Summary by CodeRabbit
Bug Fixes
Blob Brush rejects out-of-range fidelity, angle, and roundness values instead of adjusting them automatically. Invalid inputs leave the document unchanged.
Freehand drawing reports errors for unsupported styles and endpoint values. Omitted or null values default to pencil style and extending from the end.
Documentation
Clarified accepted values, defaults, and the effects of invalid inputs for these commands.
path.blob used to clamp a supplied angle to -180..180, a roundness to
0..100 and a fidelity to 0.01..100 pt, so a value such as 200 or -1
painted with a different value than the one given. It now returns an
invalid-parameters error for a value outside its range, before the
document changes, and records no undo step. A key left out or null
still takes its default: 0 degrees, 100% and no fitting.
path.freehand (and brush.freehand) treated any style other than
"brush" as "pencil", and any extend end other than "start" as "end".
It now rejects an unknown style or extend end with an error, as it
already did for connect end; left out or null, they keep their
defaults.
The new messages have rows in every complete-message catalog, and the
connect end message gets rows in the catalogs that lacked one.
Docs: path.blob and path.freehand params text, docs/mcp.md,
docs/target-app-parity.md.
No actionable comments were generated in the recent review.
Walkthrough
path.blob now rejects angle, roundness, and fidelity values outside their supported ranges. path.freehand now rejects unsupported styles and extension endpoints. Tests and documentation cover these validation rules, defaults, and error behavior.
Changes: Drawing command validation
Layer / File(s)
Summary
Blob Brush range validation crates/engine/src/cmd/draw2/commands.rs, crates/engine/src/cmd/draw2/freehand.rs, crates/engine/src/tests/draw2.rs, crates/ui-egui/src/i18n/tests.rs, docs/mcp.md, docs/target-app-parity.md
path.blob rejects angle, roundness, and fidelity values outside their supported ranges. Tests cover invalid values, boundaries, defaults, and translated errors. Documentation describes the ranges and failure behavior.
path.blob and path.freehand now reject out-of-range or unsupported option values instead of silently clamping or falling back, and the docs and tests describe this. Valid inputs, endpoints, and defaults still work. No remaining merge-blocking risk was identified.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the main change: path.blob now rejects out-of-range fidelity, angle and roundness instead of clamping them. It is specific and matches the changeset, which also covers path.freehand va…
Description check
The description follows the template. It has a 'What and why' section explaining the clamping-to-error change and its rationale, and a 'How it was tested' section listing new tests, focused checks, an…
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: Title check
Explanation
The title names the main change: path.blob now rejects out-of-range fidelity, angle and roundness instead of clamping them. It is specific and matches the changeset, which also covers path.freehand validation and translations, but those are secondary.
Full details: Description check
Explanation
The description follows the template. It has a 'What and why' section explaining the clamping-to-error change and its rationale, and a 'How it was tested' section listing new tests, focused checks, and a CI run. The checklist is completed. It also covers the path.freehand change, translation catalog work, and conflicts with open PRs. The template's HTML comments are removed, which is normal.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
LanguageTool
docs/target-app-parity.md
LanguageTool checks are incomplete because the per-file request limit of 5 was reached. Remaining text was skipped; findings from completed checks are retained.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between 2ec0af8 and 0b34f7b.
Files ignored due to path filters: 24
crates/ui-egui/src/i18n/ca.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/cs.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/da.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/de.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/el.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/es.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/fi.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/fr.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/hi.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/hu.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/it.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/ja.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/ka.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/ko.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/nl.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/pl.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/pt-br.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/ru.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/sv.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/tr.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/uk.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/vi.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/zh-hans.tsv is excluded by !**/*.tsv
crates/ui-egui/src/i18n/zh-hant.tsv is excluded by !**/*.tsv
Files selected for processing: 4
crates/engine/src/cmd/draw2/freehand.rs
crates/ui-egui/src/i18n/tests.rs
docs/mcp.md
docs/target-app-parity.md
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
…essages the untranslated report lists
pt-br is in COMPLETE_MESSAGES, but the untranslated report listed 1,851
status and error messages with no pt-br row, so with the interface in
Brazilian Portuguese those messages showed in English.
- pt-br.tsv gains 1,812 `@msg` rows. The translations are VectorCraft's
own, written from the English meaning with the catalog's terms.
Placeholders, backticked names, command ids, parameter names,
double-quoted values and bare true, false and null stay as the English
writes them.
- The 39 messages that open #7211, #7214, #7116 and #7330 add pt-br rows
for are left out. bundled_catalogs_are_consistent rejects a second row
for the same key.
- The Portuguese row of docs/localization-parity.md gives the catalog's
counts: 9,260 rows, 3,445 of them `@msg` and 48 `@msgplural`.
uk is in COMPLETE_MESSAGES, but the untranslated report listed status
and error messages with no Ukrainian row, so with the interface in
Ukrainian those messages showed in English.
- uk.tsv gains 1,811 `@msg` rows. The translations are VectorCraft's
own, written from the English meaning with the catalog's terms and
typography (« » around interface names, ’ inside words).
Placeholders, backticked names, command ids, double-quoted values and
bare true, false and null stay as the English writes them.
- The 41 rows that open #7211, #7214, #7116, #7330 and #817 add to
uk.tsv are left out, so the catalog keeps one row per key when they
merge (bundled_catalogs_are_consistent rejects a second row for a
key).
- The Ukrainian row of docs/localization-parity.md gives the catalog's
counts.
…ort lists
The Polish catalog lacked a row for 1,854 of the status and error
messages that the untranslated report finds in the sources, so the
status bar showed those messages in English.
- pl.tsv gains 1,808 `@msg` rows: the errors of commands and their
parameter checks, scripts, file import and export, printing and
plug-ins. The translations are VectorCraft's own, written from the
English meaning with the catalog's conventions and its existing
feature names. Placeholders, parameter names, command ids and
literal values stay as the English writes them.
- 44 of the others get their Polish rows from open #7211, #7214,
#7116, #7330 and #1931, so this change leaves them out. 2 are
messages that no catalog has a row for yet. Menu strings, `tl!`
literals, undo labels and swatch names are unchanged.
- Two of the messages have two placeholders in a row. The catalog does
not load such a row, so those two are still shown in English.
- The Polish row of docs/localization-parity.md gives the catalog's
current counts.
Docs: docs/target-app-parity.md, docs/localization-parity.md.
… report lists
The Traditional Chinese catalog had no row for 1,852 of the status and
error messages that i18n::tests::untranslated_report finds in the
sources, so the status bar showed them in English.
- zh-hant.tsv gets 1,813 @msg rows under one comment line at the end of
the file, written in Taiwan usage with the catalog's existing terms.
Each row keeps the English key, its placeholders, doubled braces,
parameter names, command ids and quoted values unchanged.
- The 38 rows that open #7211, #7116 and #7330 add, and the row #7214
added, are left out, so the catalogs stay consistent.
- docs/localization-parity.md and docs/target-app-parity.md give the new
counts: 3,438 @msg and 48 @msgplural rows, 40 messages still listed.
The rows for `{name} must be {lo}–{hi}{unit} (got {v})` and
`unknown parameter `{within}{key}`; {why}` are counted by the report, but
Template::parse does not load a template with two placeholders in a row,
so those two messages still show in English. The row for
`{k} has control characters{_1}` does not match the message for a
multi-line File Info field, where its last placeholder is empty.
…ranslated report lists
cs is in COMPLETE_MESSAGES, but the untranslated report listed 1,853
status and error messages with no Czech row, so with the interface in
Czech those messages showed in English.
- cs.tsv gains 1,813 `@msg` rows in one block after the last row. The
translations are VectorCraft's own, written from the English meaning
with the catalog's terms and typography (polite second person, Czech
quotation marks „…“). Placeholders, backticked names, command ids,
double-quoted values and bare true, false and null stay as the
English writes them.
- The 38 messages whose Czech rows open #7211, #7116 and #7330 add are
left out, so the catalog keeps one row per key when they merge.
- Menu strings, `tl!` literals, undo labels and swatch names are
unchanged.
- The Czech row of docs/localization-parity.md gives the catalog's
current counts.
The Korean catalog lacked a row for 1,852 of the status and error
messages that the untranslated report finds in the sources, so the
status bar showed those messages in English. ko.tsv now has an @msg
row for 1,813 of them: the errors of commands and their parameter
checks, scripts, file import and export, printing and plug-ins.
The other 39 are messages that open #7211, #7214, #7116 and #7330
also translate in ko.tsv. They are left out so the catalog has no
duplicate key when those pull requests merge.
The rows keep every placeholder, parameter name, command id and
literal value of the English, and use the feature names the catalog
already has.
The report's Korean menu strings, tl! strings, undo labels and swatch
names are unchanged.
Docs: docs/localization-parity.md, docs/target-app-parity.md.
The Dutch catalog lacked a row for 1,813 of the status and error
messages that the untranslated report finds in the sources, so the
status bar showed those messages in English.
- nl.tsv gains 1,813 `@msg` rows: the errors of commands and their
parameter checks, scripts, file import and export, printing and
plug-ins. The translations are VectorCraft's own, written from the
English meaning with the catalog's conventions and its existing
feature names. Placeholders, parameter names, command ids and
literal values stay as the English writes them.
- The untranslated report lists 41 Dutch messages after the change:
the composite font errors, the justification spacing error and the
`connect` end error, which open #7211, #7116 and #7330 translate,
and three unknown-parameter and range errors that open #7353
rewrites so that a catalog row can match them. Menu strings, `tl!`
literals and undo labels are unchanged.
- The row for `{k} has control characters{_1}` does not match the
message for a multi-line File Info field, where the last
placeholder is empty.
- docs/localization-parity.md's Dutch row and a line in
docs/target-app-parity.md give the new counts.
main's German row for the connect end refusal quotes the values as „start“ and „end“, so the test now looks for each accepted value inside any of the quotation mark pairs the catalogs use.
…report lists
The Hungarian catalog lacked a row for 1,853 of the status and error
messages that the untranslated report finds in the sources, so the
status bar showed those messages in English.
- hu.tsv gains 1,813 `@msg` rows: the errors of commands and their
parameter checks, scripts, file import and export, printing and
plug-ins. The translations are VectorCraft's own, written from the
English meaning with the catalog's conventions and its existing
feature names. Placeholders, parameter names, command ids and
literal values stay as the English writes them.
- The 38 rows that open #7211, #7116 and #7330 add to hu.tsv are left
out, since a second row for the same key fails
bundled_catalogs_are_consistent. The report lists those 38 and two
messages that no catalog has a row for. Menu strings, `tl!`
literals, undo labels and swatch names are unchanged.
- Two of the messages have two placeholders in a row. The catalog does
not load such a row, so those two are still shown in English. The
row for `{k} has control characters{_1}` does not match the File
Info error for a multi-line field, where the last placeholder is
empty.
- The Hungarian row of docs/localization-parity.md gives the catalog's
current message counts.
Docs: docs/target-app-parity.md, docs/localization-parity.md.
…rows
The freehand refusal test now checks only that each catalog shows the
accepted values as written, in its own quotation marks, so the Hindi row
keeps its translated parameter name. No command emits "angle and roundness
must be finite" any more, so its row goes from every catalog.
…e-angle rows
main now has a Japanese row for `connect end must be "start" or "end"`,
so the branch's copy is removed and ja.tsv keeps main's row. Nothing
emits `angle and roundness must be finite` any more, so the rows main
added for it in hu.tsv and ja.tsv are removed.
…port lists
The Turkish catalog had no row for 1,852 of the status and error
messages that the untranslated report finds in the sources, so the
status bar showed those messages in English.
- tr.tsv gains 1,811 `@msg` rows: the errors of commands and their
parameter checks, scripts, file import and export, printing,
plug-ins and Banner Hems and Grommets. The translations are
VectorCraft's own, written from the English meaning with the
catalog's terms and its existing feature names. Placeholders,
parameter names, command ids and literal values stay as the English
writes them.
- The 38 listed messages that open #7211, #7116 and #7330 add tr.tsv
rows for are left out, so the catalog has no duplicate key when they
merge.
- Three messages that open #7353 rewrites are left out: the unknown
parameter error and the PDF setting range error, which have two
placeholders in a row that no row can match, and the unknown
parameter error for a format's settings.
- The row for `{k} has control characters{_1}` does not match the
message for a multi-line File Info field, where the last
placeholder is empty.
- The Turkish row of docs/localization-parity.md gives the catalog's
current counts and what still falls back to English.
Docs: docs/target-app-parity.md, docs/localization-parity.md.
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
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.
What and why
path.blobclamped a suppliedangleto -180 to 180 degrees, aroundnessto 0 to 100% and afidelityto 0.01 to 100 pt, so a call withangle: 200orfidelity: -1painted with a different value from the one it gave. The Blob Brush help page lists Angle, which rotates the brush tip, Roundness, which sets the tip's shape, and Fidelity as tool options, and gives no numeric limits for them (help page); the ranges are VectorCraft's own values, the ones the Tool Options dialog already uses. This changes existing behavior: a value outside its range is now an error instead of being clamped.path.blob(crates/engine/src/cmd/draw2/freehand.rs) returns an invalid-parameters error for anangleoutside -180 to 180, aroundnessoutside 0 to 100 and afidelityoutside 0.01 to 100 pt, before the document changes, and records no undo step. NaN and infinities fail the same checks. A key left out or null still takes its default: 0 degrees, 100% and no fitting. The endpoints are accepted.path.freehandtreated anystyleother than "brush" as "pencil", and anyextendendother than "start" as "end". It now rejects an unknown value with an error, as it already did for theendofconnect. Left out or null, they keep "pencil" and "end".connect end must be "start" or "end", which 22 of the 28 catalogs lack onmain, gets rows in the 18 of those that are complete-message catalogs. The other 4 (bg, bs, gu and id) still lack it. Nothing emits the messageangle and roundness must be finiteany more, and its row is removed from the 15 catalogs that have one onmain.connect end must be "start" or "end"row toes.tsv. Whichever of this pull request and i18n: Spanish translates 1,802 of the status and error messages the untranslated report lists #7316 merges second drops its copy of the row, sincebundled_catalogs_are_consistentrejects a key that appears twice.crates/engine/src/cmd/draw2/commands.rs.path.blobandpath.freehand,docs/mcp.mdand one line indocs/target-app-parity.mddescribe the refusals.How it was tested
New tests, each failing without the change:
tests::draw2::blob_brush_rejects_out_of_range_tip_and_fidelity_without_edit(vectorcraft-engine): values just outside each bound are refused with their message and add no path; the endpoints and omitted or null keys still paint.tests::draw2::pencil_refuses_an_unknown_style_or_extend_end(vectorcraft-engine): an unknownstyleand an unknownextendendare refused and leave the paths as they were, and "pencil", "brush", "end", "start" and null still draw.i18n::tests::blob_brush_range_refusals_translate_with_their_valuesandi18n::tests::freehand_choice_refusals_translate_with_their_values(vectorcraft-ui-egui): every complete-message catalog translates the new messages; thepath.blobmessages keep the parameter names and numbers, and thepath.freehandmessages keep the accepted values as written, in the catalog's own quotation marks.The existing test
tests::draw2::blob_tip_zero_and_invalid_values_are_safepainted with an angle and roundness of1e300and-1e300; it now expects those calls to be refused.Focused checks on Linux, with the branch as one commit on
mainat0b29b2b1f6:cargo fmt --all -- --checkcargo check --workspace --testscargo clippy -p vectorcraft-engine -p vectorcraft-ui-egui --all-targets -- -D warningscargo test -p vectorcraft-engine --libfiltered totests::draw2::pencil,tests::draw2::blob,drawing_commandsandtests::params: 57 passedcargo test -p vectorcraft-ui-egui --lib i18n: 112 passedOn
2ec0af8eb0, withmainatd89f593bfamerged in:cargo fmt --all -- --check,cargo clippy -p vectorcraft-ui-egui --all-targets -- -D warnings, andcargo test -p vectorcraft-ui-egui --lib i18n: 124 passed.On
0f342cd689, withmainat0adbdce6cbmerged in:cargo fmt --all -- --check,cargo check --workspace --tests, andcargo test -p vectorcraft-ui-egui --lib i18n: 124 passed.On
963d38d040, withmainat9ddbf25befmerged in (Danish is now a complete-message language, andda.tsvgets the rows above):cargo fmt --all -- --check,cargo check --workspace --tests, andcargo test -p vectorcraft-ui-egui --lib i18n: 125 passed.On the earlier base
4558196e16:cargo xtask brands,layers(30 crates) andassets(337 files).cargo xtask ciruns in this pull request's CI.cargo xtask cipassed in this pull request's CI run on447e1f898e: https://github.com/storytold/vectorcraft/actions/runs/43531370762/job/132249632122Checklist
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