[GitHub] [lucene-solr] jimczi commented on a change in pull request #1541: RegExp - add case insensitive matching option

[GitBox <gi...@apache.org>]

jimczi commented on a change in pull request #1541:
URL: https://github.com/apache/lucene-solr/pull/1541#discussion_r431877760



##########
File path: lucene/core/src/test/org/apache/lucene/util/automaton/TestRegExp.java
##########
@@ -215,11 +238,13 @@ protected String checkRandomExpression(String docValue) {
 
     String regexPattern = result.toString();
     // Assert our randomly generated regex actually matches the provided raw input using java's expression matcher
-    Pattern pattern = Pattern.compile(regexPattern);
+    Pattern pattern = caseSensitiveQuery ? Pattern.compile(regexPattern): 
+                                           Pattern.compile(regexPattern, Pattern.CASE_INSENSITIVE); 
+                                             ;
     Matcher matcher = pattern.matcher(docValue);
     assertTrue("Java regex " + regexPattern + " did not match doc value " + docValue, matcher.matches());
 
-    RegExp regex = new RegExp(regexPattern);
+    RegExp regex = caseSensitiveQuery ? new RegExp(regexPattern) : new RegExp(regexPattern, RegExp.CASE_INSENSITIVE);

Review comment:
       Should it be `new RegExp(regexPattern, RegExp.ALL | RegExp.CASE_INSENSITIVE)` ?

##########
File path: lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java
##########
@@ -437,9 +437,15 @@
   public static final int INTERVAL = 0x0020;
   
   /**
-   * Syntax flag, enables all optional regexp syntax.
+   * Allows case insensitive matching.
    */
-  public static final int ALL = 0xffff;
+  public static final int CASE_INSENSITIVE = 0x0040;  
+  
+  /**
+   * Syntax flag, enables all optional regexp syntax
+   * but preserves default setting of case sensitive matching. 
+   */
+  public static final int ALL = 0xffff ^ CASE_INSENSITIVE ;

Review comment:
       Maybe add a `ALL_AND_CASE_INSENSITIVE` ?

##########
File path: lucene/core/src/test/org/apache/lucene/util/automaton/TestRegExp.java
##########
@@ -88,10 +88,14 @@ public void testRepeatWithEmptyLanguage() throws Exception {
     assertTrue(a.toString().length() > 0);
   }
   
+  
+  boolean caseSensitiveQuery = true;
+  
   public void testCoreJavaParity() {
     // Generate random doc values and random regular expressions
     // and check for same matching behaviour as Java's Pattern class.
     for (int i = 0; i < 1000; i++) {
+      caseSensitiveQuery = true;      

Review comment:
       should use randomization ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org