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); }