-
-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds new documentation for PostCode validator #16
Adds new documentation for PostCode validator #16
Conversation
froschdesign
commented
Mar 14, 2020
Q | A |
---|---|
Documentation | yes |
### Conventions for self defined Formats | ||
|
||
When using self defined formats, you should omit the regex delimiters and | ||
anchors (`'/^'` and `'$/'`). They are attached automatically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I have checked in the code it is true when we skip delimiters, completely, but if we use other delimiters than /
it will break the regexp by attaching additional /^
and $/
. We can provide regexp as: #^AT-\d{3}$#
but internally it will be broken and changed to: /^#^AT-\d{3}$#$/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The question that arises here: is the documentation wrong or the code which handles this behaviour?
In my opinion the simple handling should be in the foreground and the code should realize this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to detect any delimiter used, but the question is if we can do that in any reliable way?
Hm... I would need to think about it.
In general I do not like idea of attaching /^
and $/
(maybe /
is the most used, but is not the only/valid one and there are sometimes good reasons to use another).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have a problem here then a separate issue report is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@froschdesign I've created issue: #18
b227eb9
to
137f468
Compare
Thanks, @froschdesign! |