Bug 575242 - fixed progress reporting and cancellation on replace
For many elements to be changed, initial preparation phase caused UI to
hang without any progress and any chance to cancel the long running
operation. Added progress monitors checks and reporting through the
relevant parts of code, so that operation is not blocking anymore and
shows some decent progress.
Change-Id: Icb5c8beb59ddeea39c59901b655fea59997dbde6
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.text/+/183747
Tested-by: Platform Bot <platform-bot@eclipse.org>
Reviewed-by: Simeon Andreev <simeon.danailov.andreev@gmail.com>
diff --git a/org.eclipse.search/search/org/eclipse/search/internal/ui/text/ReplaceRefactoring.java b/org.eclipse.search/search/org/eclipse/search/internal/ui/text/ReplaceRefactoring.java
index ca3b092..6a5ea1b 100644
--- a/org.eclipse.search/search/org/eclipse/search/internal/ui/text/ReplaceRefactoring.java
+++ b/org.eclipse.search/search/org/eclipse/search/internal/ui/text/ReplaceRefactoring.java
@@ -34,6 +34,7 @@
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.OperationCanceledException;
+import org.eclipse.core.runtime.SubMonitor;
import org.eclipse.core.resources.IContainer;
import org.eclipse.core.resources.IFile;
@@ -211,13 +212,17 @@
fMatches.clear();
if (fSelection != null) {
+ SubMonitor progress = SubMonitor.convert(pm);
+ // "Unknown" progress, because selected elements can be containers
+ progress.setWorkRemaining(100_000);
for (Object element : fSelection) {
- collectMatches(element);
+ collectMatches(element, progress);
}
} else {
Object[] elements= fResult.getElements();
+ SubMonitor progress = SubMonitor.convert(pm, elements.length);
for (Object element : elements) {
- collectMatches(element);
+ collectMatches(element, progress.split(1));
}
}
if (!hasMatches()) {
@@ -226,7 +231,8 @@
return new RefactoringStatus();
}
- private void collectMatches(Object object) throws CoreException {
+ private void collectMatches(Object object, SubMonitor progress) throws CoreException {
+ progress.checkCanceled();
if (object instanceof LineElement) {
LineElement lineElement= (LineElement) object;
FileMatch[] matches= lineElement.getMatches(fResult);
@@ -239,7 +245,7 @@
IContainer container= (IContainer) object;
IResource[] members= container.members();
for (IResource member : members) {
- collectMatches(member);
+ collectMatches(member, progress);
}
} else if (object instanceof IFile) {
Match[] matches= fResult.getMatches(object);
@@ -256,6 +262,7 @@
}
}
}
+ progress.worked(1);
}
public int getNumberOfFiles() {
@@ -345,11 +352,13 @@
return fCollator.compare(p1, p2);
}
});
- checkFilesToBeChanged(allFiles, resultingStatus);
+ int workSize = allFiles.length;
+ SubMonitor progress = SubMonitor.convert(pm, workSize * 2);
+ checkFilesToBeChanged(allFiles, resultingStatus, progress.split(workSize));
if (resultingStatus.hasFatalError()) {
return resultingStatus;
}
-
+ progress.setWorkRemaining(workSize);
CompositeChange compositeChange= new CompositeChange(SearchMessages.ReplaceRefactoring_composite_change_name);
compositeChange.markAsSynthetic();
@@ -357,10 +366,12 @@
boolean hasChanges= false;
try {
for (IFile file : allFiles) {
+ progress.checkCanceled();
Set<FileMatch> bucket= fMatches.get(file);
if (!bucket.isEmpty()) {
try {
- TextChange change= createFileChange(file, pattern, bucket, resultingStatus, matchGroups);
+ TextChange change = createFileChange(file, pattern, bucket, resultingStatus, matchGroups,
+ progress);
if (change != null) {
compositeChange.add(change);
hasChanges= true;
@@ -370,6 +381,7 @@
return RefactoringStatus.createFatalErrorStatus(message);
}
}
+ progress.worked(1);
}
} catch (PatternSyntaxException e) {
String message= Messages.format(SearchMessages.ReplaceRefactoring_error_replacement_expression, e.getLocalizedMessage());
@@ -385,9 +397,11 @@
return resultingStatus;
}
- private void checkFilesToBeChanged(IFile[] filesToBeChanged, RefactoringStatus resultingStatus) throws CoreException {
+ private void checkFilesToBeChanged(IFile[] filesToBeChanged, RefactoringStatus resultingStatus, SubMonitor pm)
+ throws CoreException {
ArrayList<IFile> readOnly= new ArrayList<>();
for (IFile file : filesToBeChanged) {
+ pm.checkCanceled();
if (file.isReadOnly())
readOnly.add(file);
}
@@ -404,7 +418,9 @@
resultingStatus.merge(ResourceChangeChecker.checkFilesToBeChanged(filesToBeChanged, null));
}
- private TextChange createFileChange(IFile file, Pattern pattern, Set<FileMatch> matches, RefactoringStatus resultingStatus, Collection<MatchGroup> matchGroups) throws PatternSyntaxException, CoreException {
+ private TextChange createFileChange(IFile file, Pattern pattern, Set<FileMatch> matches,
+ RefactoringStatus resultingStatus, Collection<MatchGroup> matchGroups, SubMonitor pm)
+ throws PatternSyntaxException, CoreException {
PositionTracker tracker= InternalSearchUI.getInstance().getPositionTracker();
TextFileChange change= new TextFileChange(Messages.format(SearchMessages.ReplaceRefactoring_group_label_change_for_file, file.getName()), file);
@@ -422,6 +438,7 @@
String lineDelimiter= TextUtilities.getDefaultLineDelimiter(document);
for (FileMatch match : matches) {
+ pm.checkCanceled();
int offset= match.getOffset();
int length= match.getLength();
Position currentPosition= tracker.getCurrentPosition(match);
