Bug 578413 - improve ConvertLineDelemiter performance
Avoid UI events for every line replaced. Instead construct a replacement
for the whole document content.
Because this is instantaneous there is no progress report within each
file anymore.
+ improve error msg if encoding is wrong to see the affected file upon
multi file convert.
Change-Id: I889e94687815658da46db1592bf2a5b73840b85f
Signed-off-by: Joerg Kubitz <jkubitz-eclipse@gmx.de>
Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.text/+/190155
Tested-by: Platform Bot <platform-bot@eclipse.org>
diff --git a/org.eclipse.core.filebuffers.tests/META-INF/MANIFEST.MF b/org.eclipse.core.filebuffers.tests/META-INF/MANIFEST.MF
index b7f97e6..e519001 100644
--- a/org.eclipse.core.filebuffers.tests/META-INF/MANIFEST.MF
+++ b/org.eclipse.core.filebuffers.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@
Bundle-ManifestVersion: 2
Bundle-Name: %Plugin.name
Bundle-SymbolicName: org.eclipse.core.filebuffers.tests;singleton:=true
-Bundle-Version: 3.12.100.qualifier
+Bundle-Version: 3.12.200.qualifier
Bundle-Activator: org.eclipse.core.filebuffers.tests.FileBuffersTestPlugin
Bundle-ActivationPolicy: lazy
Bundle-Vendor: %Plugin.providerName
diff --git a/org.eclipse.core.filebuffers.tests/pom.xml b/org.eclipse.core.filebuffers.tests/pom.xml
index 35ffef0..6c552e2 100644
--- a/org.eclipse.core.filebuffers.tests/pom.xml
+++ b/org.eclipse.core.filebuffers.tests/pom.xml
@@ -18,7 +18,7 @@
<relativePath>../tests-pom/</relativePath>
</parent>
<artifactId>org.eclipse.core.filebuffers.tests</artifactId>
- <version>3.12.100-SNAPSHOT</version>
+ <version>3.12.200-SNAPSHOT</version>
<packaging>eclipse-test-plugin</packaging>
<properties>
<testSuite>${project.artifactId}</testSuite>
diff --git a/org.eclipse.core.filebuffers.tests/src/org/eclipse/core/filebuffers/tests/ConvertLineDelemiterTest.java b/org.eclipse.core.filebuffers.tests/src/org/eclipse/core/filebuffers/tests/ConvertLineDelemiterTest.java
new file mode 100644
index 0000000..c3cad09
--- /dev/null
+++ b/org.eclipse.core.filebuffers.tests/src/org/eclipse/core/filebuffers/tests/ConvertLineDelemiterTest.java
@@ -0,0 +1,93 @@
+/*******************************************************************************
+ * Copyright (c) 2022 Joerg Kubitz and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Joerg Kubitz - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.core.filebuffers.tests;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.function.Function;
+
+import org.junit.Test;
+
+
+import org.eclipse.core.runtime.IPath;
+import org.eclipse.core.runtime.Path;
+
+import org.eclipse.core.resources.IFile;
+import org.eclipse.core.resources.IProject;
+import org.eclipse.core.resources.ResourcesPlugin;
+
+import org.eclipse.core.filebuffers.FileBuffers;
+import org.eclipse.core.filebuffers.manipulation.ConvertLineDelimitersOperation;
+import org.eclipse.core.filebuffers.manipulation.FileBufferOperationRunner;
+
+public class ConvertLineDelemiterTest {
+
+ private static final String[] DELIMS= new String[] {
+ "\r",
+ "\n",
+ "\r\n",
+ };
+
+ @Test
+ public void testWithDelimnAtEnd() throws Exception {
+ test(delim -> delim + "line1" + delim + "line2" + delim + delim + "line3" + delim);
+ }
+
+ @Test
+ public void testWithoutDelimnAtEnd() throws Exception {
+ test(delim -> "line1" + delim + "line2" + delim + delim + "line3");
+ }
+
+ void test(Function<String, String> testFile) throws Exception {
+ IProject p= ResourcesPlugin.getWorkspace().getRoot().getProject("ConvertLineDelemiterTest");
+ p.create(null);
+ p.open(null);
+ try {
+ for (String outputDelim : DELIMS) {
+ IFile[] files= new IFile[DELIMS.length];
+ int i= 0;
+ for (String inputDelim : DELIMS) {
+ String input= testFile.apply(inputDelim);
+ IFile file= ResourcesPlugin.getWorkspace().getRoot().getFile(new Path("/ConvertLineDelemiterTest/test" + i + ".txt"));
+ InputStream s= new ByteArrayInputStream(input.getBytes());
+ file.create(s, true, null);
+ files[i++]= file;
+ }
+ FileBufferOperationRunner runner= new FileBufferOperationRunner(FileBuffers.getTextFileBufferManager(), null);
+ ConvertLineDelimitersOperation op= new ConvertLineDelimitersOperation(outputDelim);
+ runner.execute(Arrays.stream(files).map(f -> f.getFullPath()).toArray(IPath[]::new), op, null);
+ for (IFile file : files) {
+ String actual= Files.readString(file.getLocation().toFile().toPath());
+ String expected= testFile.apply(outputDelim);
+ assertEquals(readable(expected), readable(actual));
+ }
+ for (IFile file : files) {
+ file.delete(true, null);
+ }
+ }
+ } finally {
+ p.delete(true, null);
+ }
+ }
+
+ private String readable(String s) {
+ s= s.replace("\r", "\\r");
+ s= s.replace("\n", "\\n");
+ return s;
+ }
+}
diff --git a/org.eclipse.core.filebuffers/META-INF/MANIFEST.MF b/org.eclipse.core.filebuffers/META-INF/MANIFEST.MF
index c2f2287..a288957 100644
--- a/org.eclipse.core.filebuffers/META-INF/MANIFEST.MF
+++ b/org.eclipse.core.filebuffers/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.core.filebuffers; singleton:=true
-Bundle-Version: 3.7.100.qualifier
+Bundle-Version: 3.7.200.qualifier
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Export-Package:
diff --git a/org.eclipse.core.filebuffers/src/org/eclipse/core/filebuffers/manipulation/ConvertLineDelimitersOperation.java b/org.eclipse.core.filebuffers/src/org/eclipse/core/filebuffers/manipulation/ConvertLineDelimitersOperation.java
index 32779c3..54ba2d9 100644
--- a/org.eclipse.core.filebuffers/src/org/eclipse/core/filebuffers/manipulation/ConvertLineDelimitersOperation.java
+++ b/org.eclipse.core.filebuffers/src/org/eclipse/core/filebuffers/manipulation/ConvertLineDelimitersOperation.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2000, 2008 IBM Corporation and others.
+ * Copyright (c) 2000, 2022 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -10,16 +10,18 @@
*
* Contributors:
* IBM Corporation - initial API and implementation
+ * Joerg Kubitz - rewrite implementation
*******************************************************************************/
package org.eclipse.core.filebuffers.manipulation;
+import java.util.Objects;
+
import org.eclipse.core.internal.filebuffers.FileBuffersPlugin;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Status;
-import org.eclipse.core.runtime.SubMonitor;
import org.eclipse.core.filebuffers.IFileBufferStatusCodes;
import org.eclipse.core.filebuffers.ITextFileBuffer;
@@ -29,11 +31,9 @@
import org.eclipse.jface.text.BadLocationException;
import org.eclipse.jface.text.DocumentRewriteSessionType;
import org.eclipse.jface.text.IDocument;
-import org.eclipse.jface.text.IRegion;
/**
- * A text file buffer operation that changes the line delimiters to a specified
- * line delimiter.
+ * A text file buffer operation that changes the line delimiters to a specified line delimiter.
*
* @since 3.1
*/
@@ -42,8 +42,7 @@
private String fLineDelimiter;
/**
- * Creates a new line delimiter conversion operation for the given target
- * delimiter.
+ * Creates a new line delimiter conversion operation for the given target delimiter.
*
* @param lineDelimiter the target line delimiter
*/
@@ -56,24 +55,24 @@
protected MultiTextEditWithProgress computeTextEdit(ITextFileBuffer fileBuffer, IProgressMonitor progressMonitor) throws CoreException {
IDocument document= fileBuffer.getDocument();
int lineCount= document.getNumberOfLines();
-
- SubMonitor subMonitor= SubMonitor.convert(progressMonitor, FileBuffersMessages.ConvertLineDelimitersOperation_task_generatingChanges, lineCount);
try {
-
- MultiTextEditWithProgress multiEdit= new MultiTextEditWithProgress(FileBuffersMessages.ConvertLineDelimitersOperation_task_applyingChanges);
-
+ String original= document.get();
+ int newLengthGuess= original.length() + (fLineDelimiter.length() - 1) * lineCount;
+ StringBuilder sb= new StringBuilder(newLengthGuess);
for (int i= 0; i < lineCount; i++) {
- final String delimiter= document.getLineDelimiter(i);
- if (delimiter != null && !delimiter.isEmpty() && !delimiter.equals(fLineDelimiter)) {
- IRegion region= document.getLineInformation(i);
- multiEdit.addChild(new ReplaceEdit(region.getOffset() + region.getLength(), delimiter.length(), fLineDelimiter));
+ int offset= document.getLineOffset(i);
+ int length= document.getLineLength(i);
+ String delim= document.getLineDelimiter(i);
+ sb.append(original, offset, offset + length - (delim == null ? 0 : delim.length()));
+ if (delim != null) {
+ sb.append(fLineDelimiter);
}
- subMonitor.split(1);
}
-
- return multiEdit.getChildrenSize() <= 0 ? null : multiEdit;
-
- } catch (BadLocationException x) {
+ String replaced= sb.toString();
+ MultiTextEditWithProgress multiEdit= new MultiTextEditWithProgress(FileBuffersMessages.ConvertLineDelimitersOperation_task_applyingChanges);
+ multiEdit.addChild(new ReplaceEdit(0, document.getLength(), replaced));
+ return Objects.equals(replaced, original) ? null : multiEdit;
+ } catch (BadLocationException | IndexOutOfBoundsException x) {
throw new CoreException(new Status(IStatus.ERROR, FileBuffersPlugin.PLUGIN_ID, IFileBufferStatusCodes.CONTENT_CHANGE_FAILED, "", x)); //$NON-NLS-1$
}
}
diff --git a/org.eclipse.core.filebuffers/src/org/eclipse/core/internal/filebuffers/FileBuffersMessages.properties b/org.eclipse.core.filebuffers/src/org/eclipse/core/internal/filebuffers/FileBuffersMessages.properties
index 8e17fda..8acfe38 100644
--- a/org.eclipse.core.filebuffers/src/org/eclipse/core/internal/filebuffers/FileBuffersMessages.properties
+++ b/org.eclipse.core.filebuffers/src/org/eclipse/core/internal/filebuffers/FileBuffersMessages.properties
@@ -28,7 +28,7 @@
ResourceTextFileBuffer_error_illegal_encoding_message_arg= Character encoding "{0}" is not a legal character encoding.
ResourceTextFileBuffer_error_unsupported_encoding_message_arg= Character encoding "{0}" is not supported by this platform.
-ResourceTextFileBuffer_error_charset_mapping_failed_message_arg=Some characters cannot be mapped using "{0}" character encoding.\nEither change the encoding or remove the characters which are not supported by the "{0}" character encoding.
+ResourceTextFileBuffer_error_charset_mapping_failed_message_arg=Some characters cannot be mapped using "{0}" character encoding for file "{1}".\nEither change the encoding or remove the characters which are not supported by the "{0}" character encoding.
ResourceTextFileBuffer_task_saving= Saving
ResourceTextFileBuffer_oom_on_file_read=OutOfMemoryError occurred while reading file "{0}".
diff --git a/org.eclipse.core.filebuffers/src/org/eclipse/core/internal/filebuffers/ResourceTextFileBuffer.java b/org.eclipse.core.filebuffers/src/org/eclipse/core/internal/filebuffers/ResourceTextFileBuffer.java
index 8b903e4..fc287de 100644
--- a/org.eclipse.core.filebuffers/src/org/eclipse/core/internal/filebuffers/ResourceTextFileBuffer.java
+++ b/org.eclipse.core.filebuffers/src/org/eclipse/core/internal/filebuffers/ResourceTextFileBuffer.java
@@ -334,7 +334,7 @@
stream= new ByteArrayInputStream(bytes, 0, byteBuffer.limit());
} catch (CharacterCodingException ex) {
Assert.isTrue(ex instanceof UnmappableCharacterException);
- String message= NLSUtility.format(FileBuffersMessages.ResourceTextFileBuffer_error_charset_mapping_failed_message_arg, encoding);
+ String message= NLSUtility.format(FileBuffersMessages.ResourceTextFileBuffer_error_charset_mapping_failed_message_arg, new Object[] {encoding,getLocation().toString()});
IStatus s= new Status(IStatus.ERROR, FileBuffersPlugin.PLUGIN_ID, IFileBufferStatusCodes.CHARSET_MAPPING_FAILED, message, ex);
throw new CoreException(s);
}
