Support for not closing the output stream opened by user #941
Conversation
| @@ -61,7 +61,7 @@ public void write(Table table, CsvWriteOptions options) { | |||
| } finally { | |||
| if (csvWriter != null) { | |||
| csvWriter.flush(); | |||
| csvWriter.close(); | |||
| if (options.autoClose()) csvWriter.close(); | |||
There was a problem hiding this comment.
FixedWidthWriter would need to be updated as well
There was a problem hiding this comment.
FixedWidthWriterwould need to be updated as well
Fixed. But I have some problem with FixedWidthWriter when I try to output the table.
ByteArrayOutputStream baos = new ByteArrayOutputStream();
FixedWidthWriteOptions options =
new FixedWidthWriteOptions.Builder(baos)
.header(new FixedWidthFields(10,10))
.build();
FixedWidthWriter writer = new FixedWidthWriter();
writer.write(table, options);The last line of code got IllegalStateException. When FixedWidthWriterSettings is constructed with FixedWidthWriteOptions, fieldLengths is neved assigned. I think it will be fixed in another issue.
assign FixedWidthWriterSettings.fieldLengths (jtablesaw#943)
|
Does this need to be an option or could the output stream always be left open for writer and stream, but closed for everything else? |
I think the output stream should always be left open for writer and stream, but closed for others. |
|
Yes, since it is essentially a constant, it doesn't need to be in the
options class. That's really for user configurable things.
Is there some advantage to putting it there, rather than the writer classes
themselves?
…On Sat, May 22, 2021 at 12:17 PM Changzhen Zhang ***@***.***> wrote:
the output stream always be left open for writer and stream, but closed
for everything else
I think the output stream should always be left open for writer and
stream, but closed for others.
Do you mean that this change should be put in other places, not in
WriteOptions?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#941 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA2FPASEMF35XFVPOL6REMTTO7KK3ANCNFSM45GVOVNQ>
.
|
|
ok, I will change it later. |
|
@lwhite1, I reviewed the code, and I found that I put the boolean value in |
|
i don't know, actually. I recall from the last time i looked at it that it
was difficult to find the right place to close the streams, so I'm not
surprised.
You can leave it in WriteOptions. Just document in the code why it's there,
and that is not exposed as an actual option.
(BTW, for the documentation I assume you meant "distribute" rather than
"distinguish")
thanks
…On Sun, May 23, 2021 at 2:51 AM Changzhen Zhang ***@***.***> wrote:
@lwhite1 <https://github.com/lwhite1>, I reviewed the code, and I found
that I put the boolean value in WriteOptions only because it can be
distinguished here according to the different builders. Is there any other
place can distinguish the builder with different output stream? I will try
to put this value there later.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#941 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA2FPAQXYSVC4QZR3QASYWLTPCQY3ANCNFSM45GVOVNQ>
.
|
Please check if the document is appropriate. Thanks! |
Thanks for contributing.
Description
Add a boolean attribute
autocloseinWriteOptionsandBuilder, whose default value is true. WhenBuilderis constructed with OutputStream or Writer, setautocloseas false.Corresponding issue is #750
Testing
Yes. 2 unit tests for testing whether FileOutputStream and OutputStreamWriter close.