Skip to content

Commit 3c12155

Browse files
HeikoKlareHeikoKlare
committed
Move busy indicator handling for find/replace from business logic to UI
The IFindReplaceLogic and its implementation currently accept a Display instance in the `performReplaceAll()` and `performSelectAll()` methods. This is used to show a busy indicator in case the operation takes too long. Showing a busy indicator is, however, the responsibility of the UI. This change removes the Display instance from the logic interface and moves the responsibility of showing a busy indicator for the two operations to the UI consumer, i.e., the FindReplaceDialog class. It also ensures that in case the active shell of the dialog is null (e.g., because setting the shell has not been processed by the GTK event queue), no exception is thrown.
1 parent 66847ac commit 3c12155

File tree

4 files changed

+36
-75
lines changed

4 files changed

+36
-75
lines changed

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogic.java

+4-36
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@
2121
import java.util.regex.Pattern;
2222
import java.util.regex.PatternSyntaxException;
2323

24-
import org.eclipse.swt.custom.BusyIndicator;
2524
import org.eclipse.swt.graphics.Point;
26-
import org.eclipse.swt.widgets.Display;
2725

2826
import org.eclipse.jface.text.IFindReplaceTarget;
2927
import org.eclipse.jface.text.IFindReplaceTargetExtension;
@@ -219,27 +217,12 @@ private IEditorStatusLine getStatusLineManager() {
219217
}
220218

221219
@Override
222-
public void performReplaceAll(String findString, String replaceString, Display display) {
220+
public void performReplaceAll(String findString, String replaceString) {
223221
resetStatus();
224222

225-
int replaceCount = 0;
226-
227223
if (findString != null && !findString.isEmpty()) {
228-
229-
class ReplaceAllRunnable implements Runnable {
230-
public int numberOfOccurrences;
231-
232-
@Override
233-
public void run() {
234-
numberOfOccurrences = replaceAll(findString, replaceString == null ? "" : replaceString); //$NON-NLS-1$
235-
}
236-
}
237-
238224
try {
239-
ReplaceAllRunnable runnable = new ReplaceAllRunnable();
240-
BusyIndicator.showWhile(display, runnable);
241-
replaceCount = runnable.numberOfOccurrences;
242-
225+
int replaceCount = replaceAll(findString, replaceString == null ? "" : replaceString); //$NON-NLS-1$
243226
if (replaceCount != 0) {
244227
if (replaceCount == 1) { // not plural
245228
statusLineMessage(FindReplaceMessages.FindReplace_Status_replacement_label);
@@ -264,27 +247,12 @@ public void run() {
264247
}
265248

266249
@Override
267-
public void performSelectAll(String findString, Display display) {
250+
public void performSelectAll(String findString) {
268251
resetStatus();
269252

270-
int selectCount = 0;
271-
272253
if (findString != null && !findString.isEmpty()) {
273-
274-
class SelectAllRunnable implements Runnable {
275-
public int numberOfOccurrences;
276-
277-
@Override
278-
public void run() {
279-
numberOfOccurrences = selectAll(findString);
280-
}
281-
}
282-
283254
try {
284-
SelectAllRunnable runnable = new SelectAllRunnable();
285-
BusyIndicator.showWhile(display, runnable);
286-
selectCount = runnable.numberOfOccurrences;
287-
255+
int selectCount = selectAll(findString);
288256
if (selectCount != 0) {
289257
if (selectCount == 1) { // not plural
290258
statusLineMessage(FindReplaceMessages.FindReplace_Status_selection_label);

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java

+2-10
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
*******************************************************************************/
1414
package org.eclipse.ui.internal.findandreplace;
1515

16-
import org.eclipse.swt.widgets.Display;
17-
1816
import org.eclipse.jface.text.IFindReplaceTarget;
1917

2018
import org.eclipse.ui.internal.findandreplace.status.IFindReplaceStatus;
@@ -93,21 +91,15 @@ public interface IFindReplaceLogic {
9391
*
9492
* @param findString The string that will be replaced
9593
* @param replaceString The string that will replace the findString
96-
* @param display the display on which the busy feedback should be
97-
* displayed. If the display is null, the Display for the
98-
* current thread will be used. If there is no Display for
99-
* the current thread,the runnable code will be executed
100-
* and no busy feedback will be displayed.y
10194
*/
102-
public void performReplaceAll(String findString, String replaceString, Display display);
95+
public void performReplaceAll(String findString, String replaceString);
10396

10497
/**
10598
* Selects all occurrences of findString.
10699
*
107100
* @param findString The String to find and select
108-
* @param display The UI's Display The UI's Display
109101
*/
110-
public void performSelectAll(String findString, Display display);
102+
public void performSelectAll(String findString);
111103

112104
/**
113105
* Locates the user's findString in the target

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.osgi.framework.FrameworkUtil;
2121

2222
import org.eclipse.swt.SWT;
23+
import org.eclipse.swt.custom.BusyIndicator;
2324
import org.eclipse.swt.custom.ScrolledComposite;
2425
import org.eclipse.swt.events.ControlAdapter;
2526
import org.eclipse.swt.events.ControlEvent;
@@ -38,6 +39,7 @@
3839
import org.eclipse.swt.widgets.Combo;
3940
import org.eclipse.swt.widgets.Composite;
4041
import org.eclipse.swt.widgets.Control;
42+
import org.eclipse.swt.widgets.Display;
4143
import org.eclipse.swt.widgets.Event;
4244
import org.eclipse.swt.widgets.Group;
4345
import org.eclipse.swt.widgets.Label;
@@ -307,7 +309,8 @@ public void widgetSelected(SelectionEvent e) {
307309
new SelectionAdapter() {
308310
@Override
309311
public void widgetSelected(SelectionEvent e) {
310-
findReplaceLogic.performSelectAll(getFindString(), fActiveShell.getDisplay());
312+
BusyIndicator.showWhile(fActiveShell != null ? fActiveShell.getDisplay() : Display.getCurrent(),
313+
() -> findReplaceLogic.performSelectAll(getFindString()));
311314
writeSelection();
312315
updateButtonState();
313316
updateFindAndReplaceHistory();
@@ -351,8 +354,8 @@ public void widgetSelected(SelectionEvent e) {
351354
new SelectionAdapter() {
352355
@Override
353356
public void widgetSelected(SelectionEvent e) {
354-
findReplaceLogic.performReplaceAll(getFindString(), getReplaceString(),
355-
fActiveShell.getDisplay());
357+
BusyIndicator.showWhile(fActiveShell != null ? fActiveShell.getDisplay() : Display.getCurrent(),
358+
() -> findReplaceLogic.performReplaceAll(getFindString(), getReplaceString()));
356359
writeSelection();
357360
updateButtonState();
358361
updateFindAndReplaceHistory();

‎tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogicTest.java

+24-26
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.mockito.Mockito;
2828

2929
import org.eclipse.swt.SWT;
30-
import org.eclipse.swt.widgets.Display;
3130
import org.eclipse.swt.widgets.Shell;
3231

3332
import org.eclipse.jface.text.Document;
@@ -98,31 +97,30 @@ public void testPerformReplaceAllForwards() {
9897
* operates
9998
*/
10099
private void performReplaceAllBaseTestcases(IFindReplaceLogic findReplaceLogic, TextViewer textViewer) {
101-
Display display= parentShell.getDisplay();
102100
textViewer.setDocument(new Document("aaaa"));
103101

104-
findReplaceLogic.performReplaceAll("a", "b", display);
102+
findReplaceLogic.performReplaceAll("a", "b");
105103
assertThat(textViewer.getDocument().get(), equalTo("bbbb"));
106104
expectStatusIsReplaceAllWithCount(findReplaceLogic, 4);
107105

108-
findReplaceLogic.performReplaceAll("b", "aa", display);
106+
findReplaceLogic.performReplaceAll("b", "aa");
109107
assertThat(textViewer.getDocument().get(), equalTo("aaaaaaaa"));
110108
expectStatusIsReplaceAllWithCount(findReplaceLogic, 4);
111109

112-
findReplaceLogic.performReplaceAll("b", "c", display);
110+
findReplaceLogic.performReplaceAll("b", "c");
113111
assertThat(textViewer.getDocument().get(), equalTo("aaaaaaaa"));
114112
expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH);
115113

116-
findReplaceLogic.performReplaceAll("aaaaaaaa", "d", display); // https://github.com/eclipse-platform/eclipse.platform.ui/issues/1203
114+
findReplaceLogic.performReplaceAll("aaaaaaaa", "d"); // https://github.com/eclipse-platform/eclipse.platform.ui/issues/1203
117115
assertThat(textViewer.getDocument().get(), equalTo("d"));
118116
expectStatusIsReplaceAllWithCount(findReplaceLogic, 1);
119117

120-
findReplaceLogic.performReplaceAll("d", null, display);
118+
findReplaceLogic.performReplaceAll("d", null);
121119
assertThat(textViewer.getDocument().get(), equalTo(""));
122120
expectStatusIsReplaceAllWithCount(findReplaceLogic, 1);
123121

124122
textViewer.getDocument().set("f");
125-
findReplaceLogic.performReplaceAll("f", "", display);
123+
findReplaceLogic.performReplaceAll("f", "");
126124
assertThat(textViewer.getDocument().get(), equalTo(""));
127125
expectStatusIsReplaceAllWithCount(findReplaceLogic, 1);
128126

@@ -131,7 +129,7 @@ private void performReplaceAllBaseTestcases(IFindReplaceLogic findReplaceLogic,
131129
Mockito.when(mockFindReplaceTarget.isEditable()).thenReturn(false);
132130

133131
findReplaceLogic.updateTarget(mockFindReplaceTarget, false);
134-
findReplaceLogic.performReplaceAll("a", "b", display);
132+
findReplaceLogic.performReplaceAll("a", "b");
135133
expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH);
136134
}
137135

@@ -142,15 +140,15 @@ public void testPerformReplaceAllForwardRegEx() {
142140
findReplaceLogic.activate(SearchOptions.REGEX);
143141
findReplaceLogic.activate(SearchOptions.FORWARD);
144142

145-
findReplaceLogic.performReplaceAll(".+\\@.+\\.com", "", parentShell.getDisplay());
143+
findReplaceLogic.performReplaceAll(".+\\@.+\\.com", "");
146144
assertThat(textViewer.getDocument().get(), equalTo(" looks.almost@like_an_email"));
147145
expectStatusIsReplaceAllWithCount(findReplaceLogic, 1);
148146

149-
findReplaceLogic.performReplaceAll("( looks.)|(like_)", "", parentShell.getDisplay());
147+
findReplaceLogic.performReplaceAll("( looks.)|(like_)", "");
150148
assertThat(textViewer.getDocument().get(), equalTo("almost@an_email"));
151149
expectStatusIsReplaceAllWithCount(findReplaceLogic, 2);
152150

153-
findReplaceLogic.performReplaceAll("[", "", parentShell.getDisplay());
151+
findReplaceLogic.performReplaceAll("[", "");
154152
assertThat(textViewer.getDocument().get(), equalTo("almost@an_email"));
155153
expectStatusIsMessageWithString(findReplaceLogic, "Unclosed character class near index 0" + System.lineSeparator()
156154
+ "[" + System.lineSeparator()
@@ -165,15 +163,15 @@ public void testPerformReplaceAllForward() {
165163
findReplaceLogic.activate(SearchOptions.REGEX);
166164
findReplaceLogic.activate(SearchOptions.FORWARD);
167165

168-
findReplaceLogic.performReplaceAll(".+\\@.+\\.com", "", parentShell.getDisplay());
166+
findReplaceLogic.performReplaceAll(".+\\@.+\\.com", "");
169167
assertThat(textViewer.getDocument().get(), equalTo(" looks.almost@like_an_email"));
170168
expectStatusIsReplaceAllWithCount(findReplaceLogic, 1);
171169

172-
findReplaceLogic.performReplaceAll("( looks.)|(like_)", "", parentShell.getDisplay());
170+
findReplaceLogic.performReplaceAll("( looks.)|(like_)", "");
173171
assertThat(textViewer.getDocument().get(), equalTo("almost@an_email"));
174172
expectStatusIsReplaceAllWithCount(findReplaceLogic, 2);
175173

176-
findReplaceLogic.performReplaceAll("[", "", parentShell.getDisplay());
174+
findReplaceLogic.performReplaceAll("[", "");
177175
assertThat(textViewer.getDocument().get(), equalTo("almost@an_email"));
178176
expectStatusIsMessageWithString(findReplaceLogic, "Unclosed character class near index 0" + System.lineSeparator()
179177
+ "[" + System.lineSeparator()
@@ -364,15 +362,15 @@ public void testPerformSelectAllForward() {
364362
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);
365363
findReplaceLogic.activate(SearchOptions.FORWARD);
366364

367-
findReplaceLogic.performSelectAll("c", parentShell.getDisplay());
365+
findReplaceLogic.performSelectAll("c");
368366
expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH);
369367

370-
findReplaceLogic.performSelectAll("b", parentShell.getDisplay());
368+
findReplaceLogic.performSelectAll("b");
371369
expectStatusIsFindAllWithCount(findReplaceLogic, 4);
372370
// I don't have access to getAllSelectionPoints or similar (not yet implemented), so I cannot really test for correct behavior
373371
// related to https://github.com/eclipse-platform/eclipse.platform.ui/issues/1047
374372

375-
findReplaceLogic.performSelectAll("AbAbAbAb", parentShell.getDisplay());
373+
findReplaceLogic.performSelectAll("AbAbAbAb");
376374
expectStatusIsFindAllWithCount(findReplaceLogic, 1);
377375
}
378376

@@ -383,13 +381,13 @@ public void testPerformSelectAllRegEx() {
383381
findReplaceLogic.activate(SearchOptions.FORWARD);
384382
findReplaceLogic.activate(SearchOptions.REGEX);
385383

386-
findReplaceLogic.performSelectAll("c.*", parentShell.getDisplay());
384+
findReplaceLogic.performSelectAll("c.*");
387385
expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH);
388386

389-
findReplaceLogic.performSelectAll("(Ab)*", parentShell.getDisplay());
387+
findReplaceLogic.performSelectAll("(Ab)*");
390388
expectStatusIsFindAllWithCount(findReplaceLogic, 1);
391389

392-
findReplaceLogic.performSelectAll("Ab(Ab)+Ab(Ab)+(Ab)+", parentShell.getDisplay());
390+
findReplaceLogic.performSelectAll("Ab(Ab)+Ab(Ab)+(Ab)+");
393391
expectStatusIsCode(findReplaceLogic, FindStatus.StatusCode.NO_MATCH);
394392
}
395393

@@ -400,12 +398,12 @@ public void testPerformSelectAllBackward() {
400398
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);
401399
findReplaceLogic.deactivate(SearchOptions.FORWARD);
402400

403-
findReplaceLogic.performSelectAll("b", parentShell.getDisplay());
401+
findReplaceLogic.performSelectAll("b");
404402
expectStatusIsFindAllWithCount(findReplaceLogic, 4);
405403
// I don't have access to getAllSelectionPoints or similar (not yet implemented), so I cannot really test for correct behavior
406404
// related to https://github.com/eclipse-platform/eclipse.platform.ui/issues/1047
407405

408-
findReplaceLogic.performSelectAll("AbAbAbAb", parentShell.getDisplay());
406+
findReplaceLogic.performSelectAll("AbAbAbAb");
409407
expectStatusIsFindAllWithCount(findReplaceLogic, 1);
410408
}
411409

@@ -414,7 +412,7 @@ public void testPerformSelectAllOnReadonlyTarget() {
414412
TextViewer textViewer= setupTextViewer("Ab Ab");
415413
textViewer.setEditable(false);
416414
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);
417-
findReplaceLogic.performSelectAll("Ab", Display.getCurrent());
415+
findReplaceLogic.performSelectAll("Ab");
418416
expectStatusIsFindAllWithCount(findReplaceLogic, 2);
419417
}
420418

@@ -437,14 +435,14 @@ public void testSelectInSearchScope() {
437435
IFindReplaceLogic findReplaceLogic= setupFindReplaceLogicObject(textViewer);
438436
textViewer.setSelection(new TextSelection(6, 11));
439437
findReplaceLogic.deactivate(SearchOptions.GLOBAL);
440-
findReplaceLogic.performReplaceAll("l", "", Display.getCurrent());
438+
findReplaceLogic.performReplaceAll("l", "");
441439
expectStatusIsReplaceAllWithCount(findReplaceLogic, 2);
442440

443441
findReplaceLogic.activate(SearchOptions.GLOBAL);
444442
textViewer.setSelection(new TextSelection(0, 5));
445443
findReplaceLogic.deactivate(SearchOptions.GLOBAL);
446444

447-
findReplaceLogic.performReplaceAll("n", "", Display.getCurrent());
445+
findReplaceLogic.performReplaceAll("n", "");
448446
expectStatusIsReplaceAllWithCount(findReplaceLogic, 1);
449447

450448
assertThat(textViewer.getTextWidget().getText(), is("lie1\nine2\nine3"));

0 commit comments

Comments
 (0)