[
https://issues.apache.org/jira/browse/GROOVY-7465?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14997818#comment-14997818
]
ASF GitHub Bot commented on GROOVY-7465:
----------------------------------------
Github user keeganwitt commented on a diff in the pull request:
https://github.com/apache/incubator-groovy/pull/176#discussion_r44361018
--- Diff: src/main/org/codehaus/groovy/runtime/ResourceGroovyMethods.java
---
@@ -974,11 +1021,30 @@ public static void append(File self, InputStream
stream) throws IOException {
* @since 1.0
*/
public static void append(File file, Object text, String charset)
throws IOException {
+ append(file, text, charset, false);
+ }
+
+ /**
+ * Append the text at the end of the File, using a specified encoding.
If
+ * the given charset is "UTF-16BE" or "UTF-16LE" (or an equivalent
alias),
+ * <code>writeBom</code> is <code>true</code>, and the file doesn't
already
+ * exist, the requisite byte order mark is written to the file before
the
+ * text is appended.
+ *
+ * @param file a File
+ * @param text the text to append at the end of the File
+ * @param charset the charset used
+ * @param writeBom whether to write a BOM
+ * @throws IOException if an IOException occurs.
+ * @since 2.5.0
+ */
+ public static void append(File file, Object text, String charset,
boolean writeBom) throws IOException {
Writer writer = null;
try {
+ boolean shouldWriteBom = writeBom && !file.exists();
FileOutputStream out = new FileOutputStream(file, true);
- if (!file.exists()) {
- writeUTF16BomIfRequired(charset, out);
+ if (shouldWriteBom) {
--- End diff --
The check is still there, I just pulled it out into a separate variable
(`shouldWriteBom`). I could do it without a variable, I only use it once so
it's not really necessary.
> ResourceGroovyMethods/NioGroovyMethods BOM behavior is inconsistent
> -------------------------------------------------------------------
>
> Key: GROOVY-7465
> URL: https://issues.apache.org/jira/browse/GROOVY-7465
> Project: Groovy
> Issue Type: Bug
> Components: groovy-jdk
> Reporter: Keegan Witt
> Assignee: Keegan Witt
> Labels: breaking_change
> Fix For: 2.5.0-beta-1
>
>
> Most users would expect {{withPrintWriter}} to be a convenience method that
> behaves the same as if they had called {{new PrintWriter}} themselves, but
> this is not the current behavior:
> {code:java}
> File file = new File("tmp.txt")
> try {
> String text = " "
> String charset = "UTF-16LE"
> file.withPrintWriter(charset) { it << text }
> println "withPrintWriter"
> file.getBytes().each { System.out.format("%02x ", it) }
> PrintWriter w = new PrintWriter(file, charset)
> w.print(text)
> w.close()
> println "\n\nnew PrintWriter"
> file.getBytes().each { System.out.format("%02x ", it) }
> } finally {
> file.delete()
> }
> {code}
> Outputs
> {noformat}
> withPrintWriter
> ff fe 20 00
> new PrintWriter
> 20 00
> {noformat}
> Additionally most users would expect that there's no difference in behavior
> between NIO and traditional methods, but this is also not the case
> {code:java}
> import java.nio.file.Files
> import java.nio.file.FileSystems
> import java.nio.file.Path
> File file = new File("tmp1.txt")
> Path path = FileSystems.getDefault().getPath("tmp2.txt")
> try {
> String text = " "
> String charset = "UTF-16LE"
> file.withPrintWriter(charset) { it << text }
> println "withPrintWriter"
> file.getBytes().each { System.out.format("%02x ", it) }
> path.withPrintWriter(charset) { it << text }
> println "\n\nnio withPrintWriter"
> path.getBytes().each { System.out.format("%02x ", it) }
> } finally {
> file.delete()
> Files.delete(path)
> }
> {code}
> outputs
> {noformat}
> withPrintWriter
> ff fe 20 00
> nio withPrintWriter
> 20 00
> {noformat}
> This is because {{ResourceGroovyMethods}} have a {{writeUTF16BomIfRequired}}
> method that {{NioGroovyMethods}} don't.
> Most likely we'd want to change {{ResourceGroovyMethods}} to not add the BOM
> by default, or at least allow the user to opt out of that behavior by doing
> something like adding a boolean argument to the existing methods that would
> control that behavior. The other option would be to make the NIO methods
> consistent by having them also use {{writeUTF16BomIfRequired}}.
> This began as a
> [discussion|http://mail-archives.apache.org/mod_mbox/incubator-groovy-users/201506.mbox/browser]
> on the user mailing list.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
