search

quchen / prettyprinter

A modern, extensible and well-documented prettyprinter.
BSD 2-Clause "Simplified" License
293 stars 34 forks source link

Support Windows line endings #216

Closed MatthewDaggitt closed 2 years ago

MatthewDaggitt commented 2 years ago

Currently Prettyprinter line endings assume a Linux LF format, e,g. https://github.com/quchen/prettyprinter/blob/d155e79127aea05b4eafa4f00fe331b2c349fc03/prettyprinter/src/Prettyprinter/Render/Text.hs#L66 and this is causing issues when I'm trying to use files generated with Prettyprinter on Windows.

One way around this to replace the new line character "\n" as follows:

import System.Info (os)
...
eol = if os == "mingw32" then "\n" else "\r\n"

Are there any objections to me making a PR doing so? Does anyone know of a more principled solution?

sjakobi commented 2 years ago

What kind of issues are you referring to? My understanding is that text I/O functions generally include platform-dependent conversions from LF to CRLF and vice versa. See e.g. https://hackage.haskell.org/package/text-1.2.5.0/docs/src/Data.Text.IO.html#hPutStr. Why should prettyprinter include this kind of logic too?!

MatthewDaggitt commented 2 years ago

Your understanding is correct. I've done a little digging and I've found that our problems stem from writing out the file contents as ByteStrings rather than Text or String, which don't include the platform-dependent conversions. I'm investigating whether we can avoid doing so and will report back in the next couple of days.

MatthewDaggitt commented 2 years ago

Hmm okay, so I've reworked our code to avoid using ByteString (we were using it again to solve unicode compatibility problems with Windows, but I've found a different solution). I think on balance it makes sense not change anything in Prettyprinter as if people want to use ByteString then, unlike I was, they should probably be aware of the implications... I'm going to close this issue. Feel free to reopen if you disagree with my conclusion :smile:

sjakobi commented 2 years ago

Sounds good to me.

BTW, AFAIK even with ByteStrings you can get the desired newline conversion by configuring the file Handle. See https://hackage.haskell.org/package/base-4.16.0.0/docs/GHC-IO-Handle.html#v:hSetNewlineMode.