validators: add option for non-ECMA regex validation#103
validators: add option for non-ECMA regex validation#103omriarad wants to merge 1 commit intojazzband:masterfrom
Conversation
this allows more flexibility with exporting the generated schema with other implementations. according to the JSON schema the pattern SHOULD be ECMA but not necessarily. Signed-off-by: Omri Arad <omriarad3@gmail.com>
| flags.update( | ||
| {key: self.ECMA_FLAGS[key] in result.flags}) | ||
|
|
||
| self.flags = [ |
There was a problem hiding this comment.
This breaks compatibility with current code, and contradicts the comment at the head of the function:
Note, that if given pattern is ECMA regex, given flags will be
completely ignored and taken from given regex.
There was a problem hiding this comment.
Well, it appears that actually the code comment is incorrect :(
Ignore my comment.
| person.name = 'Jimmy' | ||
|
|
||
|
|
||
| def test_regex_modify_schema(): |
There was a problem hiding this comment.
There are some missing test cases:
- Invalid flags
- Flags provided directly in an ECMA262 pattern
- Flags passed with
**flags
|
Nice, but actually since this is going to be some 'generic' validator - why not create other class for it? I mean (*Actually since this is generic regex you could/should remove actual validation too - if you don't know what language it is in - you cannot validate it. And if you know (that this is ie PCRE or python expression then such class wouldn't be generic either.) |
|
In that case maybe |
|
Yeah, this would make sense actually, but alongside |
|
Will work on that soon. In any case the lack of this validator is not a big deal since the library enables anyone to write such validator themselves |
|
I think this PR should be closed since it is followed by #108 which fixes the issues with this PR. |
3 participants
this allows more flexibility with exporting the generated schema with other implementations.
according to the JSON schema the pattern SHOULD be ECMA but not necessarily.
Implements #102