dweiss commented on code in PR #12973:
URL: https://github.com/apache/lucene/pull/12973#discussion_r1437830890
##########
lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/EscapeQuerySyntaxImpl.java:
##########
@@ -184,7 +186,7 @@ public CharSequence escape(CharSequence text, Locale
locale, Type type) {
* Returns a String where the escape char has been removed, or kept only
once if there was a
* double escape.
*
- * <p>Supports escaped unicode characters, e. g. translates <code>A</code>
to <code>A</code>.
+ * <p>Supports escaped Unicode characters, e.g. translates <code>A</code> to
<code>A</code>.
Review Comment:
Seems like the comment is trying to say unicode escape sequences are
replaced into their characters? Right now it says A->A, which doesn't make
sense to me.
##########
lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/EscapeQuerySyntaxImpl.java:
##########
@@ -40,105 +40,109 @@ public class EscapeQuerySyntaxImpl implements
EscapeQuerySyntax {
"AND", "OR", "NOT", "TO", "WITHIN", "SENTENCE", "PARAGRAPH", "INORDER"
};
- private static final CharSequence escapeChar(CharSequence str, Locale
locale) {
- if (str == null || str.length() == 0) return str;
+ private static CharSequence escapeChar(CharSequence str, Locale locale) {
+ if (str == null || str.isEmpty()) return str;
CharSequence buffer = str;
- // regular escapable Char for terms
- for (int i = 0; i < escapableTermChars.length; i++) {
- buffer = replaceIgnoreCase(buffer,
escapableTermChars[i].toLowerCase(locale), "\\", locale);
+ // regular escapable char for terms
+ for (String escapableTermChar : escapableTermChars) {
+ buffer = escapeIgnoringCase(buffer,
escapableTermChar.toLowerCase(locale), "\\", locale);
}
- // First Character of a term as more escaping chars
- for (int i = 0; i < escapableTermExtraFirstChars.length; i++) {
- if (buffer.charAt(0) == escapableTermExtraFirstChars[i].charAt(0)) {
- buffer = "\\" + buffer.charAt(0) + buffer.subSequence(1,
buffer.length());
+ // first char of a term as more escaping chars
+ for (String escapableTermExtraFirstChar : escapableTermExtraFirstChars) {
+ if (buffer.charAt(0) == escapableTermExtraFirstChar.charAt(0)) {
+ buffer = "\\" + buffer;
break;
}
}
return buffer;
}
- private final CharSequence escapeQuoted(CharSequence str, Locale locale) {
- if (str == null || str.length() == 0) return str;
+ private static CharSequence escapeQuoted(CharSequence str, Locale locale) {
+ if (str == null || str.isEmpty()) return str;
CharSequence buffer = str;
- for (int i = 0; i < escapableQuotedChars.length; i++) {
- buffer = replaceIgnoreCase(buffer,
escapableTermChars[i].toLowerCase(locale), "\\", locale);
+ for (String escapableQuotedChar : escapableQuotedChars) {
+ buffer = escapeIgnoringCase(buffer,
escapableQuotedChar.toLowerCase(locale), "\\", locale);
}
return buffer;
}
- private static final CharSequence escapeTerm(CharSequence term, Locale
locale) {
- if (term == null) return term;
+ private static CharSequence escapeTerm(CharSequence term, Locale locale) {
+ if (term == null || term.isEmpty()) return term;
- // Escape single Chars
+ // escape single chars
term = escapeChar(term, locale);
term = escapeWhiteChar(term, locale);
- // Escape Parser Words
- for (int i = 0; i < escapableWordTokens.length; i++) {
- if (escapableWordTokens[i].equalsIgnoreCase(term.toString())) return
"\\" + term;
+ // escape parser words
+ for (String escapableWordToken : escapableWordTokens) {
+ if (escapableWordToken.equalsIgnoreCase(term.toString())) return "\\" +
term;
}
return term;
}
/**
- * replace with ignore case
+ * Prepend every case-insensitive occurrence of the {@code sequence1} in the
{@code string} with
+ * the {@code escapeChar}. When the {@code sequence1} is empty, every
character in the {@code
+ * string} is escaped.
*
- * @param string string to get replaced
+ * @param string string to apply escaping to
* @param sequence1 the old character sequence in lowercase
- * @param escapeChar the new character to prefix sequence1 in return string.
- * @return the new String
+ * @param escapeChar the escape character to prefix sequence1 in the
returned string
+ * @return CharSequence with every occurrence of {@code sequence1} prepended
with {@code
+ * escapeChar}
*/
- private static CharSequence replaceIgnoreCase(
+ private static CharSequence escapeIgnoringCase(
CharSequence string, CharSequence sequence1, CharSequence escapeChar,
Locale locale) {
if (escapeChar == null || sequence1 == null || string == null) throw new
NullPointerException();
- // empty string case
int count = string.length();
int sequence1Length = sequence1.length();
+
+ // empty search string - escape every character
if (sequence1Length == 0) {
- StringBuilder result = new StringBuilder((count + 1) *
escapeChar.length());
- result.append(escapeChar);
+ StringBuilder result = new StringBuilder(count * (1 +
escapeChar.length()));
for (int i = 0; i < count; i++) {
- result.append(string.charAt(i));
Review Comment:
Would it be possible to add a test for this, since you've found what looks
like a bug? Thanks!
--
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.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
