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
{{ message }}
Repository navigation
i18n: File Info's control-characters error is translated for multi-line fields too - #7399
file.info refuses a text value with control characters. text() in crates/engine/src/cmd/fileinfo.rs built the error as "{k} has control characters{}", with an empty string for Description and Copyright Notice (which take line breaks and tabs) and " or line breaks" for the other fields. The message scan collected that as {k} has control characters{_1}, and the catalog matcher does not match a final placeholder against empty text, so "description has control characters" stayed English in every catalog. The catalogs that had the {_1} row (de, hi, it, pl, pt-br, sv, uk, zh-hans) showed the single-line form with " or line breaks" left in English, and the other complete-message catalogs had no row.
text() now returns one of two whole messages: {k} has control characters and {k} has control characters or line breaks. The English text of both errors is unchanged, and so is which characters each field refuses.
Each of the 24 catalogs that translate all status and error messages has a row for both messages, worded with the terms the catalog already uses for control characters and line breaks. The field's key stays as written. The old {k} has control characters{_1} rows are removed.
catalog.rs is unchanged, so no other message matches differently.
docs/target-app-parity.md has one line for it.
The only change users see is that both forms of the error now show in the interface language.
How it was tested
New tests, on main at 446bf59:
i18n::tests::file_info_control_character_refusals_translate_in_every_complete_catalog (crates/ui-egui/src/i18n/tests.rs): the message scan finds both keys; file.info with a control character in each of the seven text paths (Description, Copyright Notice, Title, Author, Author Title, Copyright URL and a keyword in a list) gives the exact English error; and in every complete-message language the shown text keeps the field's key and has none of the rule's English words. It fails without the change: with the old source line it stops at the message scan.
tests::fileinfo::control_characters_are_refused_with_the_field_rule (crates/engine/src/tests/fileinfo.rs): the exact error for each of the seven paths, and Description and Copyright Notice still take line breaks and tabs. It guards the English text and passes with or without the change.
Focused checks on the tree that contains main at 446bf59:
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
Documentation
Clarified which file information fields allow line breaks and tabs, and which reject control characters or line breaks.
Documented that validation errors are translated separately for each affected field.
Tests
Added coverage for field-specific validation rules and the corresponding error messages.
…ne fields too
file.info refused a control character with one message built from an
optional suffix: "{k} has control characters" for Description and
Copyright Notice, and the same text plus " or line breaks" for the other
text fields and each keyword in a list. A catalog template cannot match an
empty final placeholder, so the multi-line form stayed in English in every
catalog, and the five catalogs with a row showed the single-line suffix in
English.
The two rules are now two complete messages with the same English text,
each with a row in every catalog that translates all status and error
messages. The old `{k} has control characters{_1}` rows in de, hi, it,
pt-br and sv are replaced.
…haracters row
The rows main added for `{k} has control characters{_1}` match no message the
engine now writes; the two complete messages already have rows in these catalogs.
Reviewing files that changed from the base of the PR and between c454a5a and ebe823d.
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/fileinfo.rs
crates/engine/src/tests/fileinfo.rs
crates/ui-egui/src/i18n/tests.rs
docs/target-app-parity.md
No actionable comments were generated in the recent review.
Walkthrough
The change makes the file.info validation message conditional explicit. Tests and documentation describe field-specific control-character and line-break rules, including translated error messages.
Changes: File info validation
Layer / File(s)
Summary
Validation rules and coverage crates/engine/src/cmd/fileinfo.rs, crates/engine/src/tests/fileinfo.rs, crates/ui-egui/src/i18n/tests.rs, docs/target-app-parity.md
The message construction uses an explicit conditional without changing the error wording. Tests cover field-specific refusals, accepted line breaks and tabs, and translated errors. Documentation records the field rules.
Both control-character error forms for File Info now appear in the interface language, and the English messages and field rules are unchanged. No actionable merge-blocking risk remains; the full CI run should be confirmed before merging.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the main change: File Info's control-characters error now gets translated for multi-line fields. It matches the changeset, which splits the error into two complete messages with transl…
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.
Description check
The description follows the template's three sections (What and why, How it was tested, Checklist) and covers the change in detail. It explains the cause (a trailing {_1} placeholder not matching empt…
Full details: Title check
Explanation
The title names the main change: File Info's control-characters error now gets translated for multi-line fields. It matches the changeset, which splits the error into two complete messages with translations in each catalog. The phrase 'multi-line fields' is slightly loose, since the fix covers both forms of the error, but the title is specific and accurate.
Full details: Description check
Explanation
The description follows the template's three sections (What and why, How it was tested, Checklist) and covers the change in detail. It explains the cause (a trailing {_1} placeholder not matching empty text), the fix (two whole messages in place of an optional suffix), the 24 catalog rows, the docs line, and the user-visible effect. It names the new tests with their files and lists the focused checks run, including a post-merge re-run after Danish was added. Checklist items are partly ticked. The first item, cargo xtask ci, is unchecked, and the description says CI runs in the PR but does not confirm it passed. The author ticked the panic, clean-room and ASSETS.md items without stating how they were verified. The description says 24 catalogs, while the PR objectives say 23; the description also says the Danish rows were added after the merge, which the objectives do not mention. These inconsistencies do not block the check, but they should be fixed.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 460d211 and c454a5a.
Files ignored due to path filters: 23
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/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/fileinfo.rs
crates/engine/src/tests/fileinfo.rs
crates/ui-egui/src/i18n/tests.rs
docs/target-app-parity.md
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
Merged, thanks @dotCooCoo! It passed its CI checks.
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
file.inforefuses a text value with control characters.text()incrates/engine/src/cmd/fileinfo.rsbuilt the error as"{k} has control characters{}", with an empty string for Description and Copyright Notice (which take line breaks and tabs) and" or line breaks"for the other fields. The message scan collected that as{k} has control characters{_1}, and the catalog matcher does not match a final placeholder against empty text, so "description has control characters" stayed English in every catalog. The catalogs that had the{_1}row (de, hi, it, pl, pt-br, sv, uk, zh-hans) showed the single-line form with " or line breaks" left in English, and the other complete-message catalogs had no row.text()now returns one of two whole messages:{k} has control charactersand{k} has control characters or line breaks. The English text of both errors is unchanged, and so is which characters each field refuses.{k} has control characters{_1}rows are removed.catalog.rsis unchanged, so no other message matches differently.docs/target-app-parity.mdhas one line for it.The only change users see is that both forms of the error now show in the interface language.
How it was tested
New tests, on
mainat446bf59:i18n::tests::file_info_control_character_refusals_translate_in_every_complete_catalog(crates/ui-egui/src/i18n/tests.rs): the message scan finds both keys;file.infowith a control character in each of the seven text paths (Description, Copyright Notice, Title, Author, Author Title, Copyright URL and a keyword in a list) gives the exact English error; and in every complete-message language the shown text keeps the field's key and has none of the rule's English words. It fails without the change: with the old source line it stops at the message scan.tests::fileinfo::control_characters_are_refused_with_the_field_rule(crates/engine/src/tests/fileinfo.rs): the exact error for each of the seven paths, and Description and Copyright Notice still take line breaks and tabs. It guards the English text and passes with or without the change.Focused checks on the tree that contains
mainat446bf59:cargo fmt --all -- --checkcargo check --workspace --testscargo clippy -p vectorcraft-engine --all-targets -- -D warningsandcargo clippy -p vectorcraft-ui-egui --all-targets -- -D warningscargo test -p vectorcraft-ui-egui --lib i18n: 116 passedcargo test -p vectorcraft-engine --lib fileinfo: 22 passedcargo xtask brands,cargo xtask layers(30 crates) andcargo xtask assets(337 files)After merging
mainat9ddbf25, which makes Danish a complete-message language, the branch adds the two Danish rows (0537975). On that tree:cargo fmt --all -- --checkcargo check --workspace --testscargo test -p vectorcraft-ui-egui --lib i18n: 124 passedcargo xtask ciruns in this pull request's CI.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