Fixed a bug where a newline was treated as a valid value even if it was included at the end. #4094
Conversation
|
It took me a minute but I think I understand this now. Basically, because we used Would implementing the |
|
@ytetsuro Please squash this to a single commit. Alternatively, whoever merges it please be sure to squash. |
test: Add a test case where the last string is a newline code when alpha_numeric_punct rule. fix: treated as false if there is a newline at the end. test: Add a test case where the last string is a newline code when alpha_numeric_space rule. fix: treated as false if there is a newline at the end. test: Add a test case where the last string is a newline code when decimal rule. fix: treated as false if there is a newline at the end. fix: treated as false if there is a newline at the end for alpha_numeric_space. test: Add a test case where the last string is a newline code when numeric rule. fix: treated as false if there is a newline at the end for numeric rule. test: Add a test case where the last string is a newline code when decimal rule. fix: treated as false if there is a newline at the end for alpha_dash rule. test: Add a test case where the last string is a newline code when integer rule. fix: treated as false if there is a newline at the end for integer rule. fix: typo.
ytetsuro
force-pushed
the
fix/NewlinesAtTheEndAreIncludInThePatternMatch
branch
from
5bbb713
to
108a73f
Compare
fixed. |
It is I think a bad solution. <?php
$foo = <<<EOT
0123
234
567
EOT;
preg_match('/^[0-9]+$/m', $foo); // true |
|
Gotcha, thanks for clarifying! I'm good with this solution then, we'll see if anyone with more regex experience else weighs in. |
| @@ -44,7 +44,7 @@ public function alpha_space(?string $value = null): bool | |||
| return true; | |||
| } | |||
|
|
|||
| return (bool) preg_match('/^[A-Z ]+$/i', $value); | |||
| return (bool) preg_match('/\A[A-Z ]+\z/i', $value); | |||
There was a problem hiding this comment.
please add @see https://regex101.com/ link comment so it will be easier to verify in the future if bug/improvement needed
There was a problem hiding this comment.
please add @see https://regex101.com/ link comment so it will be easier to verify in the future if bug/improvement needed
Do we write this link in the code where all the regular expressions are written?
I don't see the need to add a link.
I think the attached URL is maintained by an individual, but how long will it be valid?
There was a problem hiding this comment.
You can check these:
CodeIgniter4/system/View/Parser.php
Line 576 in 3119fc8
CodeIgniter4/system/View/Parser.php
Line 742 in 3119fc8
Yes, not everwhere yet, but I think it will be better to add it whenever possible for better future.
I've no information about how long it will be valid, but it seems it long enough ,for example, this is from 5 years ago https://regex101.com/library/wL5mP7 .
There was a problem hiding this comment.
Will this link really be easy to maintain?
Every time a regular expression is edited, does the URL also need to be maintained, and will all the maintainers review and manage it?
The system also has the ability to delete the URLs that are created.
After the merge, if the link is deleted, the information will be lost.
I think test code is more constructive than a link that may or may not work.
What do other MEMBERS think about this?
I don't want to add this URL as my commit, but I will reluctantly add it if many members agree with this.
There was a problem hiding this comment.
I vote for adding the link.
As it is positioned directly above the code I would think it is implicit to maintain it (or at least see what it does) when changing the regex itself.
Also, while of course the link might point to a dead end some future day, I consider it to be better to provide information that might get lost instead of not providing info at all in the first place.
There was a problem hiding this comment.
It would be nice to have a link. It will make our life easier in the future.
There was a problem hiding this comment.
Thanks for giving me your input.
I have added your comment.
Description
I want it to be false when
However, the regular expression validation implemented in CodeIgniter4 allowed a trailing newline code.
Checklist: