Skip to content

Conversation

@samdark
Copy link
Member

@samdark samdark commented Jan 8, 2026

Q A
Is bugfix?
New feature? ✔️
Breaks BC?

@samdark samdark requested review from a team and Copilot January 8, 2026 20:24
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new NumericHelper::trimDecimalZeros() method that removes trailing zeros from the decimal part of numeric strings. The implementation handles various edge cases including null values, empty strings, negative numbers, and numbers without decimal parts.

  • Adds trimDecimalZeros() static method to NumericHelper class
  • Includes comprehensive test coverage with 10 test cases
  • Updates documentation comment style for existing method

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/NumericHelper.php Adds the new trimDecimalZeros() method with documentation, also fixes hyphenation in existing docblock
tests/NumericHelperTest.php Adds test data provider and test method for the new functionality
CHANGELOG.md Documents the new feature in the 2.7.1 release notes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI commented Jan 8, 2026

@samdark I've opened a new pull request, #157, to work on those changes. Once the pull request is ready, I'll request review from you.

Copy link
Contributor

Copilot AI commented Jan 8, 2026

@samdark I've opened a new pull request, #158, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 2 commits January 9, 2026 01:03
Co-authored-by: samdark <47294+samdark@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: samdark <47294+samdark@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
samdark and others added 6 commits January 11, 2026 14:10
Co-authored-by: Sergei Tigrov <rrr-r@ya.ru>
diff --git c/src/NumericHelper.php i/src/NumericHelper.php
index c602ede..2506fde 100644
--- c/src/NumericHelper.php
+++ i/src/NumericHelper.php
@@ -16,9 +16,6 @@ use function is_numeric;
 use function is_scalar;
 use function preg_match;
 use function preg_replace;
-use function rtrim;
-use function str_contains;
-use function str_ends_with;
 use function str_replace;
 use function substr;

@@ -176,7 +173,7 @@ final class NumericHelper
      * containing a decimal part.
      *
      * @return string|null The input string with trailing decimal zeros (and a trailing decimal
-     * separator, if any) removed, or `null` if the input was `null`.
+     * separator, if any) removed, or `null` if the input was `null` or an empty string.
      */
     public static function trimDecimalZeros(?string $value): ?string
     {
diff --git c/src/NumericHelper.php i/src/NumericHelper.php
index 2506fde..d1c388b 100644
--- c/src/NumericHelper.php
+++ i/src/NumericHelper.php
@@ -186,16 +186,26 @@ final class NumericHelper
             return null;
         }

-        if (!str_contains($value, '.')) {
-            return $value;
-        }
-        /** @psalm-suppress PossiblyNullArgument */
-        $value = rtrim($value, '0');
-
-        if (!str_ends_with($value, '.')) {
+        $decimalPosition = strpos($value, '.');
+        if ($decimalPosition === false) {
             return $value;
         }

-        return substr($value, 0, -1);
+        $length = strlen($value);
+        $index = $length - 1;
+
+        while ($index > $decimalPosition && $value[$index] === '0') {
+            $index--;
+        }
+
+        if ($index === $length - 1) {
+            return $value;
+        }
+
+        if ($index === $decimalPosition) {
+            return substr($value, 0, $decimalPosition);
+        }
+
+        return substr($value, 0, $index + 1);
     }
 }
@samdark samdark requested review from Tigrov, Copilot and vjik January 13, 2026 00:25
@samdark samdark added the status:code review The pull request needs review. label Jan 13, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

return $value;
}

return substr($value, 0, -1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check on empty string, and return null when it is empty string.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems, for string ".000" result will be empty string.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be in some cases 0, instead of null for empty result?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I don't know.

empty string => "0" or null ?
string with spaces only => "0" or null ?
"." => "0" or null?

I think we should choose between two options:

null => null
empty string => null
string with spaces only => null
"." => null
".00" => null
null => null
empty string => "0"
string with spaces only => "0"
"." => "0"
".00" => "0"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the following would be expected:

null => null
empty string => null
string with spaces only => null
"." => null
".00" => '0'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented.

samdark and others added 2 commits January 14, 2026 10:34
Co-authored-by: Sergei Predvoditelev <sergei@predvoditelev.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:code review The pull request needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants