Parse models of > 3 namespaces #16704
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fryguy
reviewed
Dec 20, 2017
lib/miq_expression/tag.rb
Outdated
| @@ -1,6 +1,6 @@ | |||
| class MiqExpression::Tag < MiqExpression::Target | |||
| REGEX = / | |||
| (?<model_name>([[:alnum:]]*(::)?){4}) | |||
| (?<model_name>([[:upper:]][[:alnum:]]*(::[[:upper:]][[:alnum:]]*)*)?) | |||
There was a problem hiding this comment.
You might want to make the inner captures into non-capturing groups
It was found that the regex for parsing tags had edge cases that would cause it to never finish evaluating. The solution to that at the time was to limit models to an arbitrary number (3) of namespaces. The bug below describes a tag that was created with a model more deeply nested than that (which I have used as a test case). Since there is no number that can be guaranteed to always work, we need a more sophisticated regex that won't loop infinitely while also supporting arbitrarily deep nesting. I've attempted to do that here, while fixing some poorly formed tests that failed to match the new pattern. I will only add that this regex has probably reached the point where it's a burden to maintain, and that we should look into reimplenting a formal parser for fields/tags in the near future. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1524889
imtayadeway
force-pushed
the
bug/miq-expression-model-namespaces
branch
from
92cb827
to
e0fa9a8
Compare
|
Checked commit imtayadeway@e0fa9a8 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
simaishi
pushed a commit
that referenced
this pull request
Jan 3, 2018
…amespaces Parse models of > 3 namespaces (cherry picked from commit faeac8b) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1530648
|
Gaprindashvili backport details: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It was found that the regex for parsing tags had edge cases that would
cause it to never finish evaluating. The solution to that at the time
was to limit models to an arbitrary number (3) of namespaces. The bug
below describes a tag that was created with a model more deeply nested
than that (which I have used as a test case). Since there is no number
that can be guaranteed to always work, we need a more sophisticated
regex that won't loop infinitely while also supporting arbitrarily
deep nesting.
I've attempted to do that here, while fixing some poorly formed tests
that failed to match the new pattern.
I will only add that this regex has probably reached the point where
it's a burden to maintain, and that we should look into reimplenting a
formal parser for fields/tags in the near future.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1524889
/cc @lgalis