pde.internal.build.Utils: use some java.nio convenience #637
Conversation
Test Results 246 files + 6 246 suites +6 1h 1m 29s ⏱️ + 21m 8s For more details on these errors, see this check. Results for commit 8f96e69. ± Comparison against base commit 7dd409f. This pull request removes 1 test.♻️ This comment has been updated with latest results. |
| try { | ||
| InputStream fragmentXML = BundleHelper.getDefault().getBundle().getEntry(TEMPLATE + "/30/fragment/fragment.xml").openStream(); //$NON-NLS-1$ | ||
| Utils.transferStreams(fragmentXML, new FileOutputStream(sourceFragmentDirURL.append(Constants.FRAGMENT_FILENAME_DESCRIPTOR).toOSString())); | ||
| Path dest = new File(sourceFragmentDirURL.append(Constants.FRAGMENT_FILENAME_DESCRIPTOR).toOSString()).toPath(); |
There was a problem hiding this comment.
| Path dest = new File(sourceFragmentDirURL.append(Constants.FRAGMENT_FILENAME_DESCRIPTOR).toOSString()).toPath(); | |
| Path dest = sourceFragmentDirURL.append(Constants.FRAGMENT_FILENAME_DESCRIPTOR).toPath(); |
build/org.eclipse.pde.build/src/org/eclipse/pde/internal/build/builder/SourceGenerator.java
Outdated
Show resolved
Hide resolved
| String destName = sourcePluginDirURL.append(Constants.BUNDLE_FILENAME_DESCRIPTOR).toOSString(); | ||
| try { | ||
| Utils.transferStreams(new ByteArrayInputStream(buffer.toString().getBytes()), new FileOutputStream(sourcePluginDirURL.append(Constants.BUNDLE_FILENAME_DESCRIPTOR).toOSString())); | ||
| Files.writeString(new File(destName).toPath(), buffer.toString()); |
There was a problem hiding this comment.
Better use:
Path destFile = sourcePluginDirURL.append(Constants.BUNDLE_FILENAME_DESCRIPTOR).toPath();
try {
Files.writeString(destFile, buffer.toString());
build/org.eclipse.pde.build/src/org/eclipse/pde/internal/build/builder/SourceGenerator.java
Outdated
Show resolved
Hide resolved
build/org.eclipse.pde.build/src/org/eclipse/pde/internal/build/Utils.java
Outdated
Show resolved
Hide resolved
| @@ -934,21 +874,11 @@ static public int scan(StringBuffer buf, int start, String[] targets) { | |||
| * @throws IOException | |||
| */ | |||
| static public StringBuffer readFile(File target) throws IOException { | |||
| return readFile(new FileInputStream(target)); | |||
| return new StringBuffer(new String(Files.readAllBytes(target.toPath()))); | |||
There was a problem hiding this comment.
| return new StringBuffer(new String(Files.readAllBytes(target.toPath()))); | |
| return new StringBuffer(Files.readString(target.toPath())); |
In general I think it would be better to just return the String and let the caller add it to a StringBuffer if necessary.
Then this method is sufficenitly simple to just inline it into all callers.
Additionally the caller should use StringBuilder instead of StringBuffer.
IIRC there is even an automated JDT-clean up to use StringBuilder instead of StringBuffer. However this is probably better done in a follow up.
There was a problem hiding this comment.
Files.readString uses UTF8 instead of std charset. Probably UTF8 is meant anyway, but this change is not about changing the charset.
There was a problem hiding this comment.
That's right that the charset does not match, but then the default charset can be passed as second argument.
This would have to advantages, it saves one string-array copy (Files.readString() re-uses the byte array in the string) and it makes more clear that a 'special' charset is used here and future readers of the code are less likely to over-see it.
Furthermore a second argument is not that much more code and then we can replace this extra method with standard Java.
| } | ||
| } | ||
| return result; | ||
| return new StringBuffer(new String(stream.readAllBytes())); |
There was a problem hiding this comment.
I think this method is sufficiently simple to just inline it into the callers.
The callers of this method should also use StringBuilder instead of StringBuffer.
There was a problem hiding this comment.
That would duplicate the flaw that a std charset is used.
|
ignoring random fail PlainJavaProjectTest #555 |
|
@jukzi please be so kind and give participants of a discussion time to respond in case of a disagreement before merging. |
No description provided.