garydgregory commented on a change in pull request #295:
URL: https://github.com/apache/commons-io/pull/295#discussion_r741915051
##########
File path:
src/main/java/org/apache/commons/io/output/FileWriterWithEncoding.java
##########
@@ -81,20 +94,46 @@ private static Writer initWriter(final File file, final
Object encoding, final b
if (!fileExistedAlready) {
FileUtils.deleteQuietly(file);
}
- throw ex;
+ if (ex instanceof UnsupportedCharsetException) {
+ throw new IOException(ex);
Review comment:
Why make this more complicated?
##########
File path:
src/main/java/org/apache/commons/io/output/FileWriterWithEncoding.java
##########
@@ -53,25 +57,34 @@
/**
* Initializes the wrapped file writer. Ensure that a cleanup occurs if
the writer creation fails.
*
- * @param file the file to be accessed
- * @param encoding the encoding to use - may be Charset, CharsetEncoder or
String, null uses the default Charset.
- * @param append true to append
- * @return the initialized writer
- * @throws IOException if an error occurs
+ * @param file the file to be accessed.
+ * @param encoding the encoding to use - may be Charset, CharsetEncoder or
String, null or unrecognized uses the default Charset.
Review comment:
"null or unrecognized uses the default Charset": What does
"unrecognized" mean? This change is confusing to me.
##########
File path:
src/main/java/org/apache/commons/io/output/FileWriterWithEncoding.java
##########
@@ -39,9 +43,9 @@
* By default, the file will be overwritten, but this may be changed to append.
* </p>
* <p>
- * The encoding must be specified using either the name of the {@link
Charset}, the {@link Charset}, or a
- * {@link CharsetEncoder}. If the default encoding is required then use the
{@link java.io.FileWriter} directly, rather
- * than this implementation.
+ * The encoding should be specified using either the name of the {@link
Charset}, the {@link Charset}, or a
+ * {@link CharsetEncoder}. Otherwise, the default {@link Charset} will be
used, but if this is required then
Review comment:
" but if this is required then" when I read this got confused as to what
"this" is and it is only by reading the original text that I understood. So
either make it clear or make it extra clear: Either use FileWriter or
Charset.defaultCharset().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
