Pretty-printing uses CRLF on Windows. Closes #14808.

[tamird]

No description provided.

[rust-highfive]

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

[tamird]

r? @alexcrichton

[alexcrichton]

Could you use cfg! here instead of #[cfg] as well? (helps weed out compile errors on all platforms at once)

[tamird]

Do you mean:

diff --git a/src/libsyntax/print/pp.rs b/src/libsyntax/print/pp.rs
index 15aaf9c..c0d68ef 100644
--- a/src/libsyntax/print/pp.rs
+++ b/src/libsyntax/print/pp.rs
@@ -507,8 +507,13 @@ impl<'a> Printer<'a> {
         }
     }
     pub fn print_newline(&mut self, amount: isize) -> io::Result<()> {
+        let newline = if cfg!(windows) {
+            "\r\n"
+        } else {
+            "\n"
+        };
         debug!("NEWLINE {}", amount);
-        let ret = write!(self.out, "\n");
+        let ret = self.out.write_all(newline.as_bytes());
         self.pending_indentation = 0;
         self.indent(amount);
         return ret;

?

[alexcrichton]

Yeah that'd do the trick! You could also use b"\n" to avoid the call to .as_bytes()

[tamird]

Done!

[tamird]

Gah, that doesn't typecheck. Sec

[alexcrichton]

@bors: r+ 7dcb1bd

[tamird]

OK, this should work.

[alexcrichton]

@bors: r+ bad285c

[bors]

⌛ Testing commit bad285c with merge a025675...

[bors]

💔 Test failed - auto-win-64-nopt-t

[klutzy]

I'm against this change. If the output depends on OS, it leads to subtle bugs which can't be fixed by non-Windows users.
Also, we already have backlog of various CRLF bugs: e.g. #7723, #8731, and #12471. These bugs are sometimes hard to fix (or even hard to figure out what's wrong).
Actually the test failed during run-make due to extra \r.

[azurespace]

I also think this change must not be merged without enough concern. If this feature is needed for some situation, it should not be solved such a way. It would be more suitable solution to add an environmental variable EVERY platform as a flag to enable/disable.

[tamird]

I'm fine with this not going in - I was just looking to close #14808. Perhaps that issue should be closed in favour of an RFC?

[alexcrichton]

Sure, sounds like there's not consensus on this issue. I don't think this necessarily needs an RFC, but it would probably be best to reach consensus on the issue first before pursuing this. Thanks regardless though @tamird!