Skip to content
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

Performance improvement for find & replace all in Batch Mode. #2268

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,23 @@
* @param text the substitution text
*/
void replaceSelection(String text);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is dangerous to do as many other projects implement the IFindReplaceTarget (EMF implements this too, for example) @merks

The current approach taken is to implement a new IFindReplaceTargetExtension. We are at target extension 4, it's time to break the tradition of counting up, please give it a meaningful name (eg. IFindReplaceTargetBatchCountExtension)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads up.

Copy link
Author

@mehmet-karaman mehmet-karaman Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, This gives me an idea.. Maybe I can fix the IMarker blocker with creating this new extension

/**
* @return true, if its able to handle batch replacements.
*/

Check warning on line 106 in bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/IFindReplaceTarget.java

View check run for this annotation

Jenkins - Eclipse Platform / Compiler and API Tools

Other

ERROR: The default method org.eclipse.jface.text.IFindReplaceTarget.canBatchReplace() in an interface has been added

Check warning on line 106 in bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/IFindReplaceTarget.java

View check run for this annotation

Jenkins - Eclipse Platform / Compiler and API Tools

Other

ERROR: Missing @SInCE tag on canBatchReplace()
default boolean canBatchReplace() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not required after extracting a new FindReplaceTargetExtension. Instead, we assume that any extender of FindReplaceTargetX can perform all of it's operations

return false;
}

/**
* @param findString the string to find.
* @param replaceString the string to replace found occurrences.
* @param wholeWordSearch search for whole words.
* @param caseSensitiveSearch case sensitive search.
* @param regexSearch RegEex search.
* @param incrementalSearch search in selected lines.
*/

Check warning on line 118 in bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/IFindReplaceTarget.java

View check run for this annotation

Jenkins - Eclipse Platform / Compiler and API Tools

Other

ERROR: The default method org.eclipse.jface.text.IFindReplaceTarget.batchReplace(String, String, boolean, boolean, boolean, boolean) in an interface has been added

Check warning on line 118 in bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/IFindReplaceTarget.java

View check run for this annotation

Jenkins - Eclipse Platform / Compiler and API Tools

Other

ERROR: Missing @SInCE tag on batchReplace(String, String, boolean, boolean, boolean, boolean)
default int batchReplace(String findString, String replaceString, boolean wholeWordSearch, boolean caseSensitiveSearch, boolean regexSearch, boolean incrementalSearch) {
Copy link
Contributor

@Wittmaxi Wittmaxi Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why a method declaration in an interface has a method body and why that even is legal...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats because it shouldn't break any other implementers of this interface. By default I've disabled the batch replace in the interface to avoid breaking / compilation errors in all other implementers..

return -1;
}
}
1 change: 1 addition & 0 deletions bundles/org.eclipse.text/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Export-Package:
org.eclipse.jface.text; text="split"; mandatory:="text",
org.eclipse.jface.text.link; text="split"; mandatory:="text",
org.eclipse.jface.text.projection,
org.eclipse.jface.text.internal,
org.eclipse.jface.text.rules; text="split"; mandatory:="text",
org.eclipse.jface.text.source; text="split"; mandatory:="text",
org.eclipse.jface.text.templates; text="split"; mandatory:="text",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

import org.eclipse.core.runtime.Assert;

import org.eclipse.jface.text.internal.RegExUtils;


/**
* Provides search and replace operations on
Expand Down Expand Up @@ -163,14 +165,14 @@ private IRegion findReplace(final FindReplaceOperationCode operationCode, int st

if (regExSearch) {
patternFlags |= Pattern.MULTILINE;
findString= substituteLinebreak(findString);
findString= RegExUtils.substituteLinebreak(findString);
}

if (!caseSensitive)
patternFlags |= Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE;

if (!regExSearch)
findString= asRegPattern(findString);
findString= RegExUtils.asRegPattern(findString);

if (wholeWord)
findString= "\\b" + findString + "\\b"; //$NON-NLS-1$ //$NON-NLS-2$
Expand Down Expand Up @@ -262,85 +264,6 @@ private IRegion findReplace(final FindReplaceOperationCode operationCode, int st
return null;
}

/**
* Substitutes \R in a regex find pattern with {@code (?>\r\n?|\n)}
*
* @param findString the original find pattern
* @return the transformed find pattern
* @throws PatternSyntaxException if \R is added at an illegal position (e.g. in a character set)
* @since 3.4
*/
private String substituteLinebreak(String findString) throws PatternSyntaxException {
int length= findString.length();
StringBuilder buf= new StringBuilder(length);

int inCharGroup= 0;
int inBraces= 0;
boolean inQuote= false;
for (int i= 0; i < length; i++) {
char ch= findString.charAt(i);
switch (ch) {
case '[':
buf.append(ch);
if (! inQuote)
inCharGroup++;
break;

case ']':
buf.append(ch);
if (! inQuote)
inCharGroup--;
break;

case '{':
buf.append(ch);
if (! inQuote && inCharGroup == 0)
inBraces++;
break;

case '}':
buf.append(ch);
if (! inQuote && inCharGroup == 0)
inBraces--;
break;

case '\\':
if (i + 1 < length) {
char ch1= findString.charAt(i + 1);
if (inQuote) {
if (ch1 == 'E')
inQuote= false;
buf.append(ch).append(ch1);
i++;

} else if (ch1 == 'R') {
if (inCharGroup > 0 || inBraces > 0) {
String msg= TextMessages.getString("FindReplaceDocumentAdapter.illegalLinebreak"); //$NON-NLS-1$
throw new PatternSyntaxException(msg, findString, i);
}
buf.append("(?>\\r\\n?|\\n)"); //$NON-NLS-1$
i++;

} else {
if (ch1 == 'Q') {
inQuote= true;
}
buf.append(ch).append(ch1);
i++;
}
} else {
buf.append(ch);
}
break;

default:
buf.append(ch);
break;
}

}
return buf.toString();
}

/**
* Interprets current Retain Case mode (all upper-case,all lower-case,capitalized or mixed)
Expand Down Expand Up @@ -556,39 +479,6 @@ else if(Character.isUpperCase(foundText.charAt(0))) // is first character upper-
return i;
}

/**
* Converts a non-regex string to a pattern
* that can be used with the regex search engine.
*
* @param string the non-regex pattern
* @return the string converted to a regex pattern
*/
private String asRegPattern(String string) {
StringBuilder out= new StringBuilder(string.length());
boolean quoting= false;

for (int i= 0, length= string.length(); i < length; i++) {
char ch= string.charAt(i);
if (ch == '\\') {
if (quoting) {
out.append("\\E"); //$NON-NLS-1$
quoting= false;
}
out.append("\\\\"); //$NON-NLS-1$
continue;
}
if (!quoting) {
out.append("\\Q"); //$NON-NLS-1$
quoting= true;
}
out.append(ch);
}
if (quoting)
out.append("\\E"); //$NON-NLS-1$

return out.toString();
}

/**
* Substitutes the previous match with the given text.
* Sends a <code>DocumentEvent</code> to all registered <code>IDocumentListener</code>.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.eclipse.jface.text.internal;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a copyright-header (in all other files too)


import org.eclipse.osgi.util.NLS;

Check warning on line 4 in bundles/org.eclipse.text/src/org/eclipse/jface/text/internal/Messages.java

View check run for this annotation

Jenkins - Eclipse Platform / Compiler and API Tools

Other

ERROR: Missing @SInCE tag on org.eclipse.jface.text.internal.Messages
public class Messages extends NLS {
Copy link
Contributor

@Wittmaxi Wittmaxi Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Messages is not a helpful name - why are we even extracting this into a separate class?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this pattern is how most of the nls support is provided by the wizard.

private static final String BUNDLE_NAME= Messages.class.getPackageName() + ".messages"; //$NON-NLS-1$

public static String RegExUtils_0;

public static String RegExUtils_IllegalPositionForRegEx;
static {
// initialize resource bundle
NLS.initializeMessages(BUNDLE_NAME, Messages.class);
}

private Messages() {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
package org.eclipse.jface.text.internal;

import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;

Check warning on line 5 in bundles/org.eclipse.text/src/org/eclipse/jface/text/internal/RegExUtils.java

View check run for this annotation

Jenkins - Eclipse Platform / Compiler and API Tools

Other

ERROR: Missing @SInCE tag on org.eclipse.jface.text.internal.RegExUtils
public class RegExUtils {

/**
* Converts a non-regex string to a pattern
* that can be used with the regex search engine.
*
* @param string the non-regex pattern
* @return the string converted to a regex pattern
*/
public static String asRegPattern(String string) {
StringBuilder out= new StringBuilder(string.length());
boolean quoting= false;

for (int i= 0, length= string.length(); i < length; i++) {
char ch= string.charAt(i);
if (ch == '\\') {
if (quoting) {
out.append("\\E"); //$NON-NLS-1$
quoting= false;
}
out.append("\\\\"); //$NON-NLS-1$
continue;
}
if (!quoting) {
out.append("\\Q"); //$NON-NLS-1$
quoting= true;
}
out.append(ch);
}
if (quoting)
out.append("\\E"); //$NON-NLS-1$

return out.toString();
}

/**
* Substitutes \R in a regex find pattern with {@code (?>\r\n?|\n)}
*
* @param findString the original find pattern
* @return the transformed find pattern
* @throws PatternSyntaxException if \R is added at an illegal position (e.g. in a character set)
* @since 3.4
*/
public static String substituteLinebreak(String findString) throws PatternSyntaxException {
int length= findString.length();
StringBuilder buf= new StringBuilder(length);

int inCharGroup= 0;
int inBraces= 0;
boolean inQuote= false;
for (int i= 0; i < length; i++) {
char ch= findString.charAt(i);
switch (ch) {
case '[':
buf.append(ch);
if (! inQuote)
inCharGroup++;
break;

case ']':
buf.append(ch);
if (! inQuote)
inCharGroup--;
break;

case '{':
buf.append(ch);
if (! inQuote && inCharGroup == 0)
inBraces++;
break;

case '}':
buf.append(ch);
if (! inQuote && inCharGroup == 0)
inBraces--;
break;

case '\\':
if (i + 1 < length) {
char ch1= findString.charAt(i + 1);
if (inQuote) {
if (ch1 == 'E')
inQuote= false;
buf.append(ch).append(ch1);
i++;

} else if (ch1 == 'R') {
if (inCharGroup > 0 || inBraces > 0) {
String msg= Messages.RegExUtils_IllegalPositionForRegEx;
throw new PatternSyntaxException(msg, findString, i);
}
buf.append("(?>\\r\\n?|\\n)"); //$NON-NLS-1$
i++;

} else {
if (ch1 == 'Q') {
inQuote= true;
}
buf.append(ch).append(ch1);
i++;
}
} else {
buf.append(ch);
}
break;

default:
buf.append(ch);
break;
}

}
return buf.toString();
}

/**
* Creates the Pattern according to the flags.
* @param findString the find string.
* @param wholeWord find whole words.
* @param caseSensitive search case sensitive.
* @param regExSearch is RegEx search
* @return a Pattern which can directly be used.
*/
public static Pattern createRegexSearchPattern(String findString, boolean wholeWord, boolean caseSensitive, boolean regExSearch) {
int patternFlags = 0;
if (regExSearch) {
patternFlags |= Pattern.MULTILINE;
findString = RegExUtils.substituteLinebreak(findString);
}

if (!caseSensitive)
patternFlags |= Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE;

if (!regExSearch)
findString = RegExUtils.asRegPattern(findString);

if (wholeWord)
findString = "\\b" + findString + "\\b"; //$NON-NLS-1$ //$NON-NLS-2$

return Pattern.compile(findString, patternFlags);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
RegExUtils_IllegalPositionForRegEx=Illegal position for \\\\R
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,7 @@ private OverlayPreferenceStore createOverlayStore() {
overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.STRING, AbstractTextEditor.PREFERENCE_COLOR_FIND_SCOPE));

overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.STRING, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_CURRENT_LINE_COLOR));
overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.BOOLEAN, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_BATCH_REPLACE));
overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.BOOLEAN, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_USE_FIND_REPLACE_OVERLAY));
overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.BOOLEAN, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_FIND_REPLACE_OVERLAY_AT_BOTTOM));
overlayKeys.add(new OverlayPreferenceStore.OverlayKey(OverlayPreferenceStore.BOOLEAN, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_CURRENT_LINE));
Expand Down Expand Up @@ -866,6 +867,10 @@ public void widgetSelected(SelectionEvent e) {
IntegerDomain lineSpaceDomain= new IntegerDomain(0, 1000);
addTextField(appearanceComposite, lineSpacing, lineSpaceDomain, 15, 0);

label= TextEditorMessages.TextEditorDefaultsPreferencePage_batchReplace;
Preference batchReplace= new Preference(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_BATCH_REPLACE, label, null);
addCheckBox(appearanceComposite, batchReplace, new BooleanDomain(), 0);

label= TextEditorMessages.TextEditorPreferencePage_useFindReplaceOverlay;
Preference useFindReplaceOverlay= new Preference(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_USE_FIND_REPLACE_OVERLAY, label, null);
final Button useOverlay= addCheckBox(appearanceComposite, useFindReplaceOverlay, new BooleanDomain(), 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ private TextEditorMessages() {
NLS.initializeMessages(BUNDLE_NAME, TextEditorMessages.class);
}

public static String TextEditorDefaultsPreferencePage_batchReplace;
public static String TextEditorDefaultsPreferencePage_carriageReturn;
public static String TextEditorDefaultsPreferencePage_transparencyLevel;
public static String TextEditorDefaultsPreferencePage_codeMinings_description;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ TextEditorPreferencePage_findScopeColor=Find scope
TextEditorPreferencePage_accessibility_disableCustomCarets= Use &custom caret
TextEditorPreferencePage_accessibility_wideCaret= &Enable thick caret
TextEditorPreferencePage_accessibility_useSaturatedColorsInOverviewRuler=U&se saturated colors in overview ruler
TextEditorDefaultsPreferencePage_batchReplace=Enable batch replace for search and replace all.
TextEditorDefaultsPreferencePage_carriageReturn=Carriage Return ( \u00a4 )
TextEditorDefaultsPreferencePage_transparencyLevel=&Transparency level (0 is transparent and 255 is opaque):
TextEditorDefaultsPreferencePage_codeMinings_description=How annotations should be shown in-line in text editors which support code minings
Expand Down
Loading
Loading