Multibyte trim support by vjik · Pull Request #131 · yiisoft/hydrator

vjik · 2026-09-15T19:52:22Z

Q	A
Is bugfix?	❌
New feature?	✔️
Breaks BC?	❌
Fix #130

codecov · 2026-09-15T19:53:40Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (90a34d6) to head (7dc5408).

Additional details and impacted files

@@             Coverage Diff             @@
##              master      #131   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity       337       374   +37     
===========================================
  Files             43        44    +1     
  Lines            796       885   +89     
===========================================
+ Hits             796       885   +89

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

🟢 Approval recommended

The opt-in implementation preserves existing behavior and is covered across resolver, hydration, configuration, and Unicode cases.

Pull request overview

Adds opt-in Unicode-aware trimming while preserving existing byte-oriented trim behavior.

Changes:

Adds multibyte trim attributes and resolvers.
Adds multibyte mode to array-string conversion.
Adds tests, documentation, dependency metadata, and changelog entries.

File summaries

File	Description
`src/Attribute/Parameter/MultibyteTrim.php`	Adds two-sided multibyte trim attribute.
`src/Attribute/Parameter/MultibyteTrimResolver.php`	Implements two-sided multibyte trimming.
`src/Attribute/Parameter/MultibyteLeftTrim.php`	Adds left-side multibyte trim attribute.
`src/Attribute/Parameter/MultibyteLeftTrimResolver.php`	Implements left-side multibyte trimming.
`src/Attribute/Parameter/MultibyteRightTrim.php`	Adds right-side multibyte trim attribute.
`src/Attribute/Parameter/MultibyteRightTrimResolver.php`	Implements right-side multibyte trimming.
`src/Attribute/Parameter/Trim.php`	Documents the multibyte alternative.
`src/Attribute/Parameter/LeftTrim.php`	Documents the multibyte alternative.
`src/Attribute/Parameter/RightTrim.php`	Documents the multibyte alternative.
`src/Attribute/Parameter/ToArrayOfStrings.php`	Documents resolver-level multibyte mode.
`src/Attribute/Parameter/ToArrayOfStringsResolver.php`	Adds optional multibyte element trimming.
`tests/Attribute/Parameter/MultibyteTrimTest.php`	Tests two-sided multibyte trimming.
`tests/Attribute/Parameter/MultibyteLeftTrimTest.php`	Tests left-side multibyte trimming.
`tests/Attribute/Parameter/MultibyteRightTrimTest.php`	Tests right-side multibyte trimming.
`tests/Attribute/Parameter/TrimTest.php`	Confirms existing byte-oriented behavior.
`tests/Attribute/Parameter/LeftTrimTest.php`	Confirms existing left-trim behavior.
`tests/Attribute/Parameter/RightTrimTest.php`	Confirms existing right-trim behavior.
`tests/Attribute/Parameter/ToArrayOfStringsTest.php`	Tests default and multibyte array trimming.
`docs/guide/en/typecasting.md`	Documents multibyte trimming usage.
`composer.json`	Adds the development polyfill and runtime suggestions.
`composer-dependency-analyser.php`	Accounts for the optional polyfill dependency.
`CHANGELOG.md`	Records the new functionality.

Review details

Files reviewed: 22/22 changed files
Comments generated: 0
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

samdark · 2026-09-15T22:39:48Z

What's your reasoning for introducing new attributes instead of making existing one unicode-aware?

vjik · 2026-09-16T05:37:27Z

What's your reasoning for introducing new attributes instead of making existing one unicode-aware?

trim() and mb_trim() process "characters" differently. trim supports ranges, but mb_trim() does not. Also mb_trim() supports from PHP 8.4 or requires polyfill.

samdark · 2026-09-16T16:33:48Z

Yes but is there a reason to use the one with no unicode support?

vjik · 2026-09-20T06:13:41Z

Yes but is there a reason to use the one with no unicode support?

Why not? I think such situations can happen currently.

For mb_tim() using user should use PHP 8.4 or install polyfill, it’s not always possible or meaningful.

I suggest to merge this PR, but in future, after drop PHP 8.3 support, use mb_* always (revert changes from this PR and just replace trim() to mb_trim())

samdark · 2026-09-20T08:22:28Z

+        }
+
+        return Result::success(
+            mb_ltrim($resolvedValue, $attribute->characters ?? $this->characters),


The ability to specify encoding is missing. It should be UTF-8 by default, but users should be able to specify it as well.

samdark · 2026-09-20T08:24:05Z

+        }
+
+        return Result::success(
+            mb_rtrim($resolvedValue, $attribute->characters ?? $this->characters),


The ability to specify encoding is missing. It should be UTF-8 by default, but users should be able to specify it as well.

samdark · 2026-09-20T08:24:47Z

+        }
+
+        return Result::success(
+            mb_trim($resolvedValue, $attribute->characters ?? $this->characters),


The ability to specify encoding is missing. It should be UTF-8 by default, but users should be able to specify it as well.

samdark · 2026-09-20T08:31:26Z

@vjik

Having both Trim and MultibyteTrim is confusing. I'd use

#[Trim(multibyte: true)]

While the current PR solution makes sense from the backwards compatibility standpoint, I think in the modern web the majority of input will be UTF-8. So for current release I'd keep multibyte false but change the default in next release.

samdark · 2026-09-20T08:38:54Z

We can introduce support for ranges for the multibyte version. At least for ASCII ones.

vjik · 2026-09-20T10:10:09Z

#[Trim(multibyte: true)]

$characters in trim() and mb_trim() are different parameter:

trim() supports ranges, e.g. a..z
mb_trim() doesn't support ranges

So, we don't can to use one parameter for multibyte and non-multibyte modes.

I add multibyte parameter to ToArrayOfStrings because it doesn't contain $characters parameter.

vjik · 2026-09-20T10:10:44Z

We can introduce support for ranges for the multibyte version. At least for ASCII ones.

I think is overhead in this case. Better drop support of ranges)

Copilot

Copilot review overview

🟡 Changes recommended

Automatic multibyte selection changes existing default behavior despite the stated backward-compatibility guarantee.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 4

Open (4)

Preserve byte-oriented LeftTrim by default · New
Keep multibyte RightTrim behavior opt-in · New
Require opt-in for multibyte array trimming · New
Keep multibyte Trim behavior opt-in · New

Copilot

Copilot review overview

🟢 Approval recommended

The implementation preserves existing defaults, handles optional dependencies, and provides comprehensive cross-version coverage.

Review effort: Balanced
Findings: None

Resolved since last review (4)

Keep multibyte Trim behavior opt-in
Require opt-in for multibyte array trimming
Keep multibyte RightTrim behavior opt-in
Preserve byte-oriented LeftTrim by default

Multibyte trim support

29f496b

vjik requested a balanced review from Copilot September 15, 2026 19:53

Copilot started reviewing on behalf of vjik September 15, 2026 19:53 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Merge branch 'master' into mb-trim

0dce301

vjik requested a review from a team September 15, 2026 20:25

vjik added the status:code review The pull request needs review. label Sep 15, 2026

vjik added 3 commits September 15, 2026 23:28

fix dependency analyser

b69ae6f

fix code coverage

c18fdbb

Merge branch 'master' into mb-trim

95b3fec

vjik mentioned this pull request Sep 20, 2026

TrimResolver doesn't respect UTF-8 #130

Open

samdark reviewed Sep 20, 2026

View reviewed changes

improve

aa521b1

vjik added status:under development Someone is working on a pull request. and removed status:code review The pull request needs review. labels Sep 20, 2026

vjik added 4 commits September 20, 2026 13:54

fix

2aa4e6c

fix

f127f62

improve

31a8464

improve

66fe5dd

vjik requested a balanced review from Copilot September 20, 2026 15:46

Copilot started reviewing on behalf of vjik September 20, 2026 15:47 View session

Copilot AI reviewed Sep 20, 2026

View reviewed changes

Comment thread src/Attribute/Parameter/LeftTrimResolver.php Outdated

Comment thread src/Attribute/Parameter/RightTrimResolver.php Outdated

Comment thread src/Attribute/Parameter/ToArrayOfStringsResolver.php Outdated

Comment thread src/Attribute/Parameter/TrimResolver.php Outdated

fix

0a0fba9

vjik requested a balanced review from Copilot September 20, 2026 15:54

Copilot started reviewing on behalf of vjik September 20, 2026 15:55 View session

Copilot AI reviewed Sep 20, 2026

View reviewed changes

vjik added status:code review The pull request needs review. and removed status:under development Someone is working on a pull request. labels Sep 20, 2026

vjik requested a review from samdark September 20, 2026 15:58

Merge branch 'master' into mb-trim

7dc5408

Uh oh!

Conversation

vjik commented Sep 15, 2026

Uh oh!

codecov Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

samdark commented Sep 15, 2026

Uh oh!

vjik commented Sep 16, 2026

Uh oh!

samdark commented Sep 16, 2026

Uh oh!

vjik commented Sep 20, 2026

Uh oh!

samdark Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

vjik Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

samdark Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

vjik Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

samdark Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

vjik Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

samdark commented Sep 20, 2026

Uh oh!

samdark commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vjik commented Sep 20, 2026

Uh oh!

vjik commented Sep 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Sep 15, 2026 •

edited

Loading

samdark commented Sep 20, 2026 •

edited

Loading