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.
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
Add Criteria-Based Filtering #894
Add Criteria-Based Filtering #894
Changes from 41 commits
bbe78ea
e48ca60
a5bdaa3
113d0c2
592a100
4cb44d3
8b8898f
8ceb3a5
ef4fd7b
ee8b4fd
076062f
7697efd
4be722f
08de315
391ab6f
0a2a63a
d4bc32c
596f49b
22b0f4a
34c00e5
785abd6
d51a97e
0265f04
91f7e28
bd8b5b8
e5bdf6e
d6010ca
3c4369b
d0837ad
5f0d857
97e2dbe
d332f7e
1fff489
1b072f6
5d429a2
e37834d
11e048a
6b9d102
225a806
e3a79b5
e3c8d87
97586f0
b5b61f0
f0f551a
da4cfdf
a17d2ed
6d2916b
ca058c4
13bd83d
d44817d
d973ec7
d8ada64
0897e47
3c64b07
216877a
5515485
f0b8a5b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
You should not have ignoreCase as a field of Pattern. I understand logically it could make sense, but it is already a field of Restriction, and it is confusing to have it in both places where users will wonder which takes precedence,
It would also be confusing in places like this:
Omit it from Pattern and none of the above will even be possible.
Also,
value
is confusing. Specifically, this is a pattern for LIKE. If someone does,it won't actually behave as a prefix. We are going to need to make this very clear so it isn't misused (any maybe we don't even want to use a record with a public constructor)
I'm not sure what best to call it. Here is something I thought of quickly,
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 don't agree with this. One of the main nice things about
Pattern
is that it encapsulates case-sensitivity.By original proposal was:
Where, the idea was that users would not usually call the constructor to obtain a
Pattern
.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.
Irrespective of the discussion on ignoringCase, the automatic escaping that you proposed above seems very useful, and helps users avoid unintentional wildcards in their prefix/suffix/substring, which can be literals now instead of patterns. I like that better than what is under this PR.
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.
Nice, I agreed.
I moved this class to our PR.