Clojure

newline should output platform-specific newline sequence

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Release 1.3
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

(newline), and (println) currently output \n as the newline sequence, even on Windows, where \r\n is the newline sequence.
These functions should output the platform specific newline sequence.

  1. 0300-newline-local-var-6-commits.patch
    17/Dec/10 10:02 AM
    27 kB
    Stuart Halloway
  2. 0300-newline-resolved.patch
    29/Oct/10 1:13 PM
    0.9 kB
    Stuart Halloway
  3. 0300-newline-with-pprint-and-tests.patch
    04/Nov/10 11:39 PM
    15 kB
    Tom Faulhaber

Activity

Hide
Assembla Importer added a comment -

rnewman said: It might be nice, then, to introduce 'newln' and 'newline', with the former doing platform-specific line breaks, and the latter printing the newline character.

Show
Assembla Importer added a comment - rnewman said: It might be nice, then, to introduce 'newln' and 'newline', with the former doing platform-specific line breaks, and the latter printing the newline character.
Hide
Assembla Importer added a comment -

djpowell said: If you want an explicit line feed character, then you're probably best just printing \newline explicitly. In Java out.newline(), and out.println() always use platform specific newlines. Also, in C, printf("\n") is defined to output the platform specific newline to text streams.

println and friends operate on characters rather than bytes, so I think it makes sense for you to get newline translation in addition to character encoding.

Also the patch attached allows an alternative line-separator to be dynamically bound, should you need to write files for other platforms.

Show
Assembla Importer added a comment - djpowell said: If you want an explicit line feed character, then you're probably best just printing \newline explicitly. In Java out.newline(), and out.println() always use platform specific newlines. Also, in C, printf("\n") is defined to output the platform specific newline to text streams. println and friends operate on characters rather than bytes, so I think it makes sense for you to get newline translation in addition to character encoding. Also the patch attached allows an alternative line-separator to be dynamically bound, should you need to write files for other platforms.
Hide
Assembla Importer added a comment -

stu said: This seems reasonable, but I don't live on Windows. Can you add a pointer to list or IRC discussion and re-set this ticket to test?

In particular I would like to see discussion of (1) whether any existing code might depend on the current behavior and (2) dynamic binding vs. explicit argument to bypass platform behavior.

Show
Assembla Importer added a comment - stu said: This seems reasonable, but I don't live on Windows. Can you add a pointer to list or IRC discussion and re-set this ticket to test? In particular I would like to see discussion of (1) whether any existing code might depend on the current behavior and (2) dynamic binding vs. explicit argument to bypass platform behavior.
Hide
Assembla Importer added a comment -

djpowell said: Started discussion here:
http://groups.google.com/group/clojure/browse_thread/thread/70f2945110f209e1#

I don't think there is much point in an explicit parameter to newline - if you want to be that specific, you may as well use (print "\r\n")?

But it is probably reasonable to want non-platform newlines occasionally by dynamic binding, eg to interact with swank (which prefers \n), or HTTP and MIME which prefer \r\n.

Show
Assembla Importer added a comment - djpowell said: Started discussion here: http://groups.google.com/group/clojure/browse_thread/thread/70f2945110f209e1# I don't think there is much point in an explicit parameter to newline - if you want to be that specific, you may as well use (print "\r\n")? But it is probably reasonable to want non-platform newlines occasionally by dynamic binding, eg to interact with swank (which prefers \n), or HTTP and MIME which prefer \r\n.
Hide
Assembla Importer added a comment -

stu said: 1. Agree that a specific override parameter for newline is bad – if you want to do something specific, just do it.

2. But by the same token, I would rather not introduce a new bindable var. I would just change newline to always print the result of (System/getProperty "line.separator"). Not totally sure about this, though. What do others think?

Show
Assembla Importer added a comment - stu said: 1. Agree that a specific override parameter for newline is bad – if you want to do something specific, just do it. 2. But by the same token, I would rather not introduce a new bindable var. I would just change newline to always print the result of (System/getProperty "line.separator"). Not totally sure about this, though. What do others think?
Hide
Assembla Importer added a comment -
Show
Assembla Importer added a comment - stu said: [file:bn-GzURhKr34gXeJe5cbCb]
Hide
Assembla Importer added a comment -

stu said: Second patch wins, per unanimous agreement on dev list at http://groups.google.com/group/clojure-dev/browse_thread/thread/4f1812a1f96a3347.

Show
Assembla Importer added a comment - stu said: Second patch wins, per unanimous agreement on dev list at http://groups.google.com/group/clojure-dev/browse_thread/thread/4f1812a1f96a3347.
Hide
Assembla Importer added a comment -

richhickey said: I don't think we should be looking up a system property on every call to newline

Show
Assembla Importer added a comment - richhickey said: I don't think we should be looking up a system property on every call to newline
Hide
Assembla Importer added a comment -

richhickey said: Also, what is the relationship between (newline) and \newline ?

Show
Assembla Importer added a comment - richhickey said: Also, what is the relationship between (newline) and \newline ?
Hide
Assembla Importer added a comment -

djpowell said: Re: calling a system property on every call

  • thinking back, that is probably one of the reasons that I made this a dynamic var in the original version of my patch. I agree, it doesn't seem elegant to keep checking a system property. We might be better using a dynamic var, even if we just keep the var private for now if there is any concern about not wanting to support rebindng. I assume that a dynamic var is faster than a system property?

Re: the relationship between newline and \newline

  • (newline) outputs the platform dependent newline sequence. It works like System.out.println() in Java, or Console.WriteLine() in C#. Note that (println) and similar clojure functions call newline to output line endings.
  • \newline is simply the constant value of the ASCII 0x0a linefeed character. This is the same semantics as "\n" in Java and C#. Note that this is different to stdio in C where "\n" has different semantics depending on whether the stream that it is written to was opened in text or binary mode.
Show
Assembla Importer added a comment - djpowell said: Re: calling a system property on every call
  • thinking back, that is probably one of the reasons that I made this a dynamic var in the original version of my patch. I agree, it doesn't seem elegant to keep checking a system property. We might be better using a dynamic var, even if we just keep the var private for now if there is any concern about not wanting to support rebindng. I assume that a dynamic var is faster than a system property?
Re: the relationship between newline and \newline
  • (newline) outputs the platform dependent newline sequence. It works like System.out.println() in Java, or Console.WriteLine() in C#. Note that (println) and similar clojure functions call newline to output line endings.
  • \newline is simply the constant value of the ASCII 0x0a linefeed character. This is the same semantics as "\n" in Java and C#. Note that this is different to stdio in C where "\n" has different semantics depending on whether the stream that it is written to was opened in text or binary mode.
Hide
Assembla Importer added a comment -

richhickey said: We can cache the property without a dynamic var and all that that implies

Show
Assembla Importer added a comment - richhickey said: We can cache the property without a dynamic var and all that that implies
Hide
Stuart Halloway added a comment -

After sleeping on it (in a hammock), I still think that respecting the Java default is the way to go. If you want something different, it is easy enough to do:

  • if you want only a single newline, print \newline
  • if you want something else, then there is no precedent for it in Java or Clojure, so roll-your-own.

The most recent patch (0300-newline-resolved) does this, but breaks some tests of cl_format. Soliciting feeback from Tom Faulhaber before pushing this to screened.

Show
Stuart Halloway added a comment - After sleeping on it (in a hammock), I still think that respecting the Java default is the way to go. If you want something different, it is easy enough to do:
  • if you want only a single newline, print \newline
  • if you want something else, then there is no precedent for it in Java or Clojure, so roll-your-own.
The most recent patch (0300-newline-resolved) does this, but breaks some tests of cl_format. Soliciting feeback from Tom Faulhaber before pushing this to screened.
Hide
Tom Faulhaber added a comment -

From my email to Stu:

I think that cl-format should emit platform specific newlines (this is consistent with the recommendation in CLtL2, section 2.2.2) when confronted the ~% directive. I suspect that that's where the tests are failing. CLtL2 also specifies that \newline will do the same thing, but I assume that we're not going there.

I'll try to get you an extended patch with improved tests over the weekend.

Show
Tom Faulhaber added a comment - From my email to Stu: I think that cl-format should emit platform specific newlines (this is consistent with the recommendation in CLtL2, section 2.2.2) when confronted the ~% directive. I suspect that that's where the tests are failing. CLtL2 also specifies that \newline will do the same thing, but I assume that we're not going there. I'll try to get you an extended patch with improved tests over the weekend.
Hide
Tom Faulhaber added a comment -

This patch includes pprint/cl-format in the change and fixes all the tests so they can run on either Unix or Windows.It replaces (and includes/corrects) previous patches.

Show
Tom Faulhaber added a comment - This patch includes pprint/cl-format in the change and fixes all the tests so they can run on either Unix or Windows.It replaces (and includes/corrects) previous patches.
Hide
Tom Faulhaber added a comment -

OK, this should be ready to go.

Show
Tom Faulhaber added a comment - OK, this should be ready to go.
Hide
David Powell added a comment -

I think it would be worth caching the value of the system property in a private def. System.getProperties invokes synchronisation, security managers, and other heavy-weight things.

Show
David Powell added a comment - I think it would be worth caching the value of the system property in a private def. System.getProperties invokes synchronisation, security managers, and other heavy-weight things.
Hide
Stuart Halloway added a comment -

Two questions about avoiding the system property lookup:

  1. Should a cached value be a public def, a private def, or just a closure?
  2. The JDK implementation caches in a private field, but they use a privileged action and a sun-specific class to request the property! Can we just do a simple getProperty?
Show
Stuart Halloway added a comment - Two questions about avoiding the system property lookup:
  1. Should a cached value be a public def, a private def, or just a closure?
  2. The JDK implementation caches in a private field, but they use a privileged action and a sun-specific class to request the property! Can we just do a simple getProperty?
Hide
Rich Hickey added a comment -

1) private def
2) Yes

Show
Rich Hickey added a comment - 1) private def 2) Yes
Hide
Stuart Halloway added a comment -

Dec 17 patch addresses all comments, and includes cleanup of test helpers (coalesce to top of test hieararchy so that committers can find them)

Show
Stuart Halloway added a comment - Dec 17 patch addresses all comments, and includes cleanup of test helpers (coalesce to top of test hieararchy so that committers can find them)

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: