You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/11/02 06:30:41 UTC
[GitHub] [commons-io] wodencafe opened a new pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
wodencafe opened a new pull request #295:
URL: https://github.com/apache/commons-io/pull/295
This pull request is to make org.apache.commons.io.output.FileWriterWithEncoding more precisely match its JavaDoc contract, for example throwing a java.lang.NullPointerException when the contract specifies that the exception will be thrown.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] data443-cboyd commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
data443-cboyd commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-958689368
Hi @garydgregory , the pull request has been updated. Thank you.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #295:
URL: https://github.com/apache/commons-io/pull/295#discussion_r795216962
##########
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:
Closing, no reply since November of last year.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-957253070
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] data443-cboyd removed a comment on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
data443-cboyd removed a comment on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-958689368
Hi @garydgregory , the pull request has been updated. Thank you.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] data443-cboyd commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
data443-cboyd commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-958689368
Hi @garydgregory , the pull request has been updated. Thank you.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] wodencafe commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
wodencafe commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-957637714
Hi @garydgregory , if this is true then the JavaDoc should be updated instead to not suggest that a java.lang.NullPointerException is thrown when charset is null for certain method calls. Right now the class is not following the contract as specified by the JavaDoc.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] wodencafe commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
wodencafe commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-957637714
Hi @garydgregory , if this is true then the JavaDoc should be updated instead to not suggest that a java.lang.NullPointerException is thrown when charset is null for certain method calls. Right now the class is not following the contract as specified by the JavaDoc.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] wodencafe commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
wodencafe commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-960343378
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] wodencafe commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
wodencafe commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-957637714
Hi @garydgregory , if this is true then the JavaDoc should be updated instead to not suggest that a java.lang.NullPointerException is thrown when charset is null for certain method calls. Right now the class is not following the contract as specified by the JavaDoc.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] wodencafe commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
wodencafe commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-958183537
@garydgregory This is a fair point, I will update the PR later tonight with an approach of correcting the JavaDoc based on the existing behavior, rather than the other way around (changing the behavior to match the existing JavaDoc).
Thank you for the feedback.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-957253070
This breaks the existing feature where the platform charset is used when null is passed in.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-957253070
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] data443-cboyd removed a comment on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
data443-cboyd removed a comment on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-958689368
Hi @garydgregory , the pull request has been updated. Thank you.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory closed pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
garydgregory closed pull request #295:
URL: https://github.com/apache/commons-io/pull/295
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-957914236
Updating the Javadoc is better in this case IMO.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] data443-cboyd commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
data443-cboyd commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-958689368
Hi @garydgregory , the pull request has been updated. Thank you.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] data443-cboyd commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
data443-cboyd commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-958689368
Hi @garydgregory , the pull request has been updated. Thank you.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #295: Make FileWriterWithEncoding more precisely match its JavaDoc contract
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #295:
URL: https://github.com/apache/commons-io/pull/295#issuecomment-1025188890
Closing, no reply since November of last year.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org