Fix and reinstate repl/errmsgs

[dwijnand]

[test_windows_full]

Fixes #13444

[griggt]

Might want [test_windows_full] ?

[dwijnand]

Ideally someone can approve, this passes CI and we can get a clean nightly run.

[smarter]

Is the BufferedWriter useful for something here?

[dwijnand]

I was wondering the same, but it already took me n hours today to figure out what was actually different (need to toInt chars to get real differences) and how it possibly broke, that I'm done. This combination comes from what the constructor in PrintWriter does when it's given a PrintStream (out).

[smarter]

I guess it doesn't matter since there's autoFlush=true, but I'll note that the repl reporter in scala 2 doesn't use BufferedWriter.

[smarter]

Besides the CI run, someone should open cmd.exe on Windows and check that the output is actually correct.

[dwijnand]

I would hope that's what we had the tests running on Windows for... Either way, I'd prefer to have the test reinstated even without that check. Worst case I'll inline the call to doReport and avoid using a PrintWriter.

[smarter]

Tests are great but encoding and Windows are weird enough that this isn't enough in general to know what the user will see with their own eyes :).

[griggt]

I'll try to take a look soon. I have a Windows machine I can test on.

[griggt]

Real-life results don't look great.

Tested with Windows 10 Console:

with this PR:

3.0.2 (looks good):

Same results using the new Windows Terminal instead of Windows Console.

The output on Windows 7 is a mess (but it seems to be so on 3.0.2 also):

[smarter]

Could you try using ctx.settings.encoding.value instead of UTF-8? Presumably that shouldn't impact the tests since they're always run with UTF-8 encoding.

[griggt]

Could you try using ctx.settings.encoding.value instead of UTF-8?

No change -- same bad output on the console.

[griggt]

FWIW the console output looks good if I run chcp 65001 at the cmd.exe prompt before launching the REPL. But 3.0.2 does not need this and I doubt that many users would even think to try this.

[griggt]

If we can discover the encoding used by the current console out: PrintStream and re-use it when constructing the OutputStreamWriter for the reporter, we seem to get the proper output.

For example, this hack "works" on JDK < 16

   /** Like ConsoleReporter, but without file paths or real -Xprompt'ing */
   private object ReplConsoleReporter extends ConsoleReporter(
     reader = null, // this short-circuits the -Xprompt display from waiting for an input
-    writer = new PrintWriter(new BufferedWriter(new OutputStreamWriter(out, StandardCharsets.UTF_8)), /* autoFlush = */ true), // write to out, not Console.err
+    writer = new PrintWriter(new BufferedWriter(new OutputStreamWriter(out, encodingOfPrintStream(out))), /* autoFlush = */ true), // write to out, not Console.err
   ) {
     override def posFileStr(pos: SourcePosition) = "" // omit file paths
   }
 
+  private def encodingOfPrintStream(ps: PrintStream): String = {
+    val f = ps.getClass.getDeclaredField("charOut")
+    f.setAccessible(true)
+    f.get(ps).asInstanceOf[OutputStreamWriter].getEncoding
+  }
+

and gives the expected output on both my Windows machine (where it apparently uses encoding "Cp437") and Linux (where it uses "UTF8").

But can we get at this without the reflection hackery?

[griggt]

Worst case I'll inline the call to doReport and avoid using a PrintWriter.

I've pushed a commit 1632cb9 that takes that sort of approach, since there doesn't seem to be an API to get the encoding out of an existing PrintStream.

I verified with that commit the output looks good again in Windows Console / Windows Terminal.

[dwijnand]

Could you try using ctx.settings.encoding.value instead of UTF-8?

No change -- same bad output on the console.

It's weird this didn't work. What encoding was being used? Was it not Cp437?

[griggt]

Could you try using ctx.settings.encoding.value instead of UTF-8?

It's weird this didn't work. What encoding was being used? Was it not Cp437?

Just checked. UTF-8 in that case.

[dwijnand]

Ah, so that's the encoding you're using for compiling the files but not for the console.. Hmm.

[griggt]

Yes, I believe so. There seems to be API to ask for the former but not the latter (but the Console.out PrintStream automagically chooses the appropriate encoder, just doesn't want to share that tidbit)

[dwijnand]

Everyone happy to land this now?

[griggt]

I'll have time later today to look it over in detail if desired but at a glance LGTM.

[philwalk]

I'm seeing what looks like this problem for the first time, when I run errmsgs test in a cygwin/mintty environment. Same thing in Windows Terminal.

Is this unexpected? Or is there some configuration step I need to take? Here's the output:

-- [E007] Type Mismatch Error: -------------------------------------------------
1 | abstract class C { type T; val x: T; val s: Unit = { type T = String; var y: T = x; locally { def f() = { type T = Int; val z: T = y }; f() } }; }
  |                                                                                  ^
  |Found:    (C.this.x : C.this.T)
  |Required: T▓
  |
  |where:    T  is a type in class C
  |          T▓ is a type in the initializer of value s which is an alias of String
longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | abstract class C { type T; val x: T; val s: Unit = { type T = String; var y: T = x; locally { def f() = { type T = Int; val z: T = y }; f() } }; }
  |                                                                                                                                    ^
  |Found:    (y : T)
  |Required: T▓
  |
  |where:    T  is a type in the initializer of value s which is an alias of String
  |          T▓ is a type in method f which is an alias of Int
longer explanation available when compiling with `-explain`

and here's the expected output:

-- [E007] Type Mismatch Error: -------------------------------------------------
1 | abstract class C { type T; val x: T; val s: Unit = { type T = String; var y: T = x; locally { def f() = { type T = Int; val z: T = y }; f() } }; }
  |                                                                                  ^
  |Found:    (C.this.x : C.this.T)
  |Required: T┬▓
  |
  |where:    T  is a type in class C
  |          T┬▓ is a type in the initializer of value s which is an alias of String
longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | abstract class C { type T; val x: T; val s: Unit = { type T = String; var y: T = x; locally { def f() = { type T = Int; val z: T = y }; f() } }; }
  |                                                                                                                                    ^
  |Found:    (y : T)
  |Required: T┬▓
  |
  |where:    T  is a type in the initializer of value s which is an alias of String
  |          T┬▓ is a type in method f which is an alias of Int
longer explanation available when compiling with `-explain`