pre-RFR (s): 8049847: Enhance PrintWriter line.separator handling

[Claes Redestad]

For the record I would be very happy to update the implementation to use
System.lineSeparator() in PrintWriter/BufferedWriter rather than reading
line.separator, thus removing the unspecified behavior to be able to 
hack the
current implementation via manipulating system properties. This might break
some applications, but, as you imply, any such use case might already be
better accommodated by overriding the PrintWriter#println/printf/format
methods.

How about closing this bug as WNF and filing a new RFE to remove the
PrintWriter/BufferedWriter constructor dependencies on line.separator?

/Claes

On 2015-01-04 20:18, Xueming Shen wrote:
>
> I'm very concerned every time when I see someone tries to achieve 
> something by
> manipulating the system property, given the lesson we have learned 
> from the past,
> (property file.encoding, for example). In that particular 
> stackoverflow sample, I
> would assume the better suggestion is to override the "println", 
> instead of trying
> to overwrite the system property. I might go further to update the 
> implementation
> to use the new System.lineSeparator() to disencourage this kind of 
> workaround.
>
> I agree with Alan that the general interface "Appendable" probably 
> should not get
> into the specific "formatting" functionality here. And I also think 
> the "line.separator"
> should not be in the "Writer" as well, writer/reader does not deal 
> with "formatting",
> only couple "special" writers need to work with "line"
>
> Personally I would not be too concerned about the PrintWriter's 
> format("%n") does
> not work the same way as PrintWriter.println(), when the "println()" 
> is overrode to write
> a line separator that is different to the system default (again, it 
> should not be achieved
> by modifying the system property), given the purpose of  defining the 
> "%n" is to
> provide the functionality of  formatting a line separator the same as 
> the underlying
> platform (otherwise a \n or \r\n probably should be used explicitly 
> ?).  If you really
> want to make such a customized version of PrintWriter to "format" a 
> different line
> separator for "%n",  the workaround is to override the format() to 
> replace the %n.
>
> Btw, Formatter is not "tightly" coupled with Appendable, even though 
> we are creating
> Appendable underneath, a kind of implementation detail. So from 
> specification point
> of view, it might not be appropriate to specifically specify "%n" to a 
> particular "Appendable".
>
> My 2 cents.
>
> On 1/4/15 6:10 AM, Claes Redestad wrote:
>>
>> On 2015-01-04 11:06, Alan Bateman wrote:
>>> On 02/01/2015 15:38, Claes Redestad wrote:
>>>> Hi,
>>>>
>>>>  this is a proposal to resolve concerns that 
>>>> PrintWriter/BufferedWriter behave
>>>> inconsistently with j.u.Formatter when setting the line.separator 
>>>> property to override
>>>> system-appropriate line breaks.
>>>>
>>>>  Instead of providing a set of new constructors, I propose to 
>>>> simply add a new default
>>>> method j.l.Appendable#lineSeparator, which by default delegates to 
>>>> System.lineSeparator().
>>>> This allows classes such as PrintWriter/BufferedWriter to provide 
>>>> overrides which
>>>> communicate to j.u.Formatter their intended behavior.
>>> In the bug report then I assume the inconsistency only arises 
>>> because the test case attempts to change the line separator in the 
>>> main method, which is too late. It should work if set on the 
>>> command-line but this of course sets it system-wide.
>>
>> Right, the PrintWriter inconsistency detailed:
>> - methods format/printf uses j.u.Formatter, which uses 
>> System.lineSeparator() internally if it
>> encounters an "%n" in the format string. Only responds to when 
>> changing line.separator on
>> command-line or similar.
>> - methods println and newLine use the PrintWriter internal 
>> lineSeparator value evaluated at
>> object creation time:
>>
>>     public PrintWriter(Writer out,
>>                        boolean autoFlush) {
>>         super(out);
>>         this.out = out;
>>         this.autoFlush = autoFlush;
>>         lineSeparator = java.security.AccessController.doPrivileged(
>>             new 
>> sun.security.action.GetPropertyAction("line.separator"));
>>     }
>>
>> This leads to hacks existing in the wild which sets line.separator 
>> before creating the PrintWriter,
>> as exemplified here: 
>> http://stackoverflow.com/questions/1014287/is-there-a-way-to-make-printwriter-output-to-unix-format
>>
>> Removing this functionality could break compatibility with some 
>> applications where developers
>> are intentionally changing the line.separator, while it'd be nice if 
>> there was a better way to facilitate
>> this, which I think my proposal would allow with minimal
>>
>>>
>>> Your proposal to add a lineSeperator() method to Appendable would 
>>> allow for customization but I'm not sure that it's the right place 
>>> (as Appendable is a very general interface and would require every 
>>> Appendable implementation that writes lines to be updated to make 
>>> use of the new method).
>>
>> Yes, this can seem an awkward place to add this method, but the only 
>> place where we'd be able
>> to as easily make java.util.Formatter use the custom lineSeparator, 
>> without which we wouldn't
>> be able to resolve the inconsistency above nor provide a more 
>> convenient way for developers
>> to avoid the line.separator hack.
>>
>> However, since Appendable is defined rather strictly-coupled with 
>> java.util.Formatter ("The Appendable
>> interface must be implemented by any class whose instances are 
>> intended to receive formatted
>> output from a java.util.Formatter.") and Formatter provides a 
>> higher-order mechanism to print new-lines
>> ("%n") to the Appendable, it doesn't feel entirely arbitrary that the 
>> lineSeparator behavior should be
>> accessible via Appendable.
>>
>>>
>>> An alternative to consider is adding the notion of lines to 
>>> java.io.Writer and update it to define newLine() and lineSeperator() 
>>> methods (i.e. move the BufferWriter::newLine method up to Writer). 
>>> It's a bit of extra work to update BufferedWriter's spec, adjust a 
>>> few things in PrintWriter/PrintStream, and special-case Writers in 
>>> Formatter but something to consider.
>>
>> I need to think about what that would mean, and especially how to 
>> solve it on the Formatter side.
>> I worry shoehorning Writer or upcasts into Formatter might be more 
>> complex that it's worth.
>> Any ideas?
>>
>> /Claes
>>
>>>
>>> -Alan
>>
>